fix: search input initialization and re-focus bug#2148
fix: search input initialization and re-focus bug#2148nnecec wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis PR updates app/composables/useGlobalSearch.ts to preserve client-typed input across hydration and route changes. It adds a client-only helper to read the focused search input value, uses that value to initialise searchQuery on the client, adjusts the URL→state watcher to observe [route.name, route.query.q] and only apply changes when route.name === 'search' (skipping updates if a search input is focused), normalises assignment to avoid redundant sets, clears and focuses the homepage search on return to index after navigation, and adds an onMounted recovery that restores searchQuery from a focused input. Lines changed: +71/−5. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/composables/useGlobalSearch.ts (1)
60-67: Consider extracting focused input detection into a reusable helper.The logic for detecting whether a search input is focused (lines 60-67) duplicates the pattern from
getFocusedSearchInputValue(lines 22-24). Extracting a smallisSearchInputFocused()helper would improve maintainability.♻️ Proposed refactor
+const isSearchInputFocused = (): boolean => { + if (!import.meta.client) return false + const active = document.activeElement + if (!(active instanceof HTMLInputElement)) return false + return active.type === 'search' || active.name === 'q' +} + const getFocusedSearchInputValue = () => { - if (!import.meta.client) return '' - - const active = document.activeElement - if (!(active instanceof HTMLInputElement)) return '' - if (active.type !== 'search' && active.name !== 'q') return '' - return active.value + if (!isSearchInputFocused()) return '' + return (document.activeElement as HTMLInputElement).value }Then in the watcher:
- if (import.meta.client) { - const active = document.activeElement - if ( - active instanceof HTMLInputElement && - (active.type === 'search' || active.name === 'q') - ) { - return - } - } + if (isSearchInputFocused()) return
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ecd4a617-b5c7-4a12-93b5-29da7a115b93
📒 Files selected for processing (1)
app/composables/useGlobalSearch.ts
nnecec
force-pushed
the
fix/home-search-input
branch
from
399f570 to
4636625
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6ac65a2-c31a-42f4-80dc-a8f836ca6980
📒 Files selected for processing (1)
app/composables/useGlobalSearch.ts
| () => [route.name, route.query.q] as const, | ||
| ([routeName, urlQuery]) => { | ||
| if (routeName !== 'search') return | ||
|
|
||
| // Never clobber in-progress typing while any search input is focused. | ||
| if (import.meta.client) { | ||
| const active = document.activeElement | ||
| if ( | ||
| active instanceof HTMLInputElement && | ||
| (active.type === 'search' || active.name === 'q') | ||
| ) { | ||
| return | ||
| } | ||
| } | ||
|
|
||
| const value = normalizeSearchParam(urlQuery) | ||
| if (!value) searchQuery.value = '' | ||
| if (!searchQuery.value) searchQuery.value = value | ||
| if (searchQuery.value !== value) { | ||
| searchQuery.value = value | ||
| } |
There was a problem hiding this comment.
Do not suppress every URL synchronisation while a search box is focused.
This also skips legitimate /search?q=... history or programmatic changes. If focus stays in the input, searchQuery never updates, committedSearchQuery stays stale too, and there is no later resynchronisation on blur.
💡 Safer guard
- // Never clobber in-progress typing while any search input is focused.
- if (import.meta.client) {
- const active = document.activeElement
- if (
- active instanceof HTMLInputElement &&
- (active.type === 'search' || active.name === 'q')
- ) {
- return
- }
- }
-
const value = normalizeSearchParam(urlQuery)
+ // Only skip when the focused input already reflects this URL value.
+ if (import.meta.client) {
+ const activeValue = getFocusedSearchInputValue()
+ if (activeValue && activeValue === value) {
+ return
+ }
+ }
if (searchQuery.value !== value) {
searchQuery.value = value
}| if (import.meta.client) { | ||
| watch( | ||
| () => route.name, | ||
| name => { | ||
| if (name !== 'index') return | ||
| searchQuery.value = '' | ||
| committedSearchQuery.value = '' | ||
| // Use nextTick so we run after the homepage has rendered. | ||
| nextTick(() => { | ||
| const homeInput = document.getElementById('home-search') | ||
| if (homeInput instanceof HTMLInputElement) { | ||
| homeInput.focus() | ||
| homeInput.select() | ||
| } | ||
| }) | ||
| }, | ||
| { flush: 'post' }, | ||
| ) | ||
| } | ||
|
|
||
| // On hydration, useState can reuse SSR payload (often empty), skipping initializer. | ||
| // Recover fast-typed value from the focused input once on client mount. | ||
| if (import.meta.client) { | ||
| onMounted(() => { | ||
| const focusedInputValue = getFocusedSearchInputValue() | ||
| if (!focusedInputValue) return | ||
| if (searchQuery.value) return | ||
|
|
||
| // Use model setter path to preserve instant-search behavior. | ||
| searchQueryValue.value = focusedInputValue | ||
| }) |
There was a problem hiding this comment.
Guard the mounted recovery after an intentional home reset.
These blocks can fight each other on the /search → / logo-click path. The index watcher clears the shared state, but the next mount can still read the previously focused header input via getFocusedSearchInputValue(); writing that back through searchQueryValue restores the old query and, with instant search enabled, can immediately navigate back to /search. A one-shot shared flag is needed so the mounted recovery is skipped for the same navigation that intentionally resets the home field.
2 participants
🔗 Linked issue
no issue
🧭 Context
no context
📚 Description
3.19.1.mp4
Fixed two issues: