fix: correct search condition for routing#1574
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughSearchBox.vue now calls useGlobalSearch with an explicit scope argument 'header'. The useGlobalSearch composable signature was changed to accept a place parameter ('header' | 'content', default 'content'), and its local-filter bypass conditional was refined to require place === 'content' in addition to the existing route-based check. No other public signatures were changed. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/composables/useGlobalSearch.ts (1)
33-36: Fix is correct — one narrow edge case worth noting.The revised condition correctly bypasses the local-filter short-circuit when
place === 'header', which directly resolves the routing breakage on org/user pages.One minor theoretical gap:
isSameQuery(line 33) checksroute.query.p === provider. On a local-filter page that somehow carries?q=<term>&p=algoliain the URL, a header search for that exact term would hit theisSameQuerybranch and silently skip navigation even thoughplace === 'header'. In practice this is essentially unreachable (local-filter pages don't normally carry?p=), but the intent could be made explicit:🔍 Optional guard to fully close the edge case
- const isSameQuery = route.query.q === value && route.query.p === provider + const isOnLocalFilterPage = pagesWithLocalFilter.has(route.name as string) + const isSameQuery = !isOnLocalFilterPage && route.query.q === value && route.query.p === provider - if ((pagesWithLocalFilter.has(route.name as string) && place === 'content') || isSameQuery) { + if ((isOnLocalFilterPage && place === 'content') || isSameQuery) {
🔗 Linked issue
resolves #1555
🧭 Context
Routing was disabled for pages with internal search
📚 Description
Updated condition to detect the place where
useGlobalSearchwas used and if it was in header search field - it will always navigate to search page