feat: autofocus first search result#89
Conversation
- Auto-select first result when search query returns results - Add controlled value state to Command component - Improves keyboard navigation and UX when searching Fixes kahdri feedback: first match now comes selected by default
|
Someone is attempting to deploy a commit to the joyco Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| React.useEffect(() => { | ||
| if (hasResults && results.length > 0) { | ||
| const firstUrl = results[0].url.split('#')[0] | ||
| setSelectedValue(firstUrl) | ||
| } else { | ||
| setSelectedValue(undefined) | ||
| } | ||
| }, [hasResults, results]) |
There was a problem hiding this comment.
Redundant results.length > 0 guard
hasResults is derived as isSearching && hasLoadedResults && results.length > 0 in use-search.ts, so the inner results.length > 0 check is always true when hasResults is true. The condition can be simplified to just if (hasResults).
| React.useEffect(() => { | |
| if (hasResults && results.length > 0) { | |
| const firstUrl = results[0].url.split('#')[0] | |
| setSelectedValue(firstUrl) | |
| } else { | |
| setSelectedValue(undefined) | |
| } | |
| }, [hasResults, results]) | |
| React.useEffect(() => { | |
| if (hasResults) { | |
| const firstUrl = results[0].url.split('#')[0] | |
| setSelectedValue(firstUrl) | |
| } else { | |
| setSelectedValue(undefined) | |
| } | |
| }, [hasResults, results]) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: components/layout/sidebar/index.tsx
Line: 36:43
Comment:
**Redundant `results.length > 0` guard**
`hasResults` is derived as `isSearching && hasLoadedResults && results.length > 0` in `use-search.ts`, so the inner `results.length > 0` check is always `true` when `hasResults` is `true`. The condition can be simplified to just `if (hasResults)`.
```suggestion
React.useEffect(() => {
if (hasResults) {
const firstUrl = results[0].url.split('#')[0]
setSelectedValue(firstUrl)
} else {
setSelectedValue(undefined)
}
}, [hasResults, results])
```
How can I resolve this? If you propose a fix, please make it concise.| React.useEffect(() => { | ||
| if (hasResults && results.length > 0) { | ||
| const firstUrl = results[0].url.split('#')[0] | ||
| setSelectedValue(firstUrl) | ||
| } else { | ||
| setSelectedValue(undefined) | ||
| } | ||
| }, [hasResults, results]) |
There was a problem hiding this comment.
Keyboard navigation resets on each API response
When the user types a query, the effect fires once per API response to re-select the first result. If the user has already moved focus down the list with arrow keys and is mid-selection, the next debounced fetch (150 ms after each keystroke) will reset selectedValue back to the first item. For fast typists this is fine because the query keeps changing, but for any scenario where results update while the user is navigating (e.g. network latency), the selection jumps back to the top unexpectedly.
Consider only setting selectedValue when the query itself changes (i.e. also tracking query in the dependency array and resetting only on query changes, not on result updates for the same query).
Prompt To Fix With AI
This is a comment left during a code review.
Path: components/layout/sidebar/index.tsx
Line: 36:43
Comment:
**Keyboard navigation resets on each API response**
When the user types a query, the effect fires once per API response to re-select the first result. If the user has already moved focus down the list with arrow keys and is mid-selection, the next debounced fetch (150 ms after each keystroke) will reset `selectedValue` back to the first item. For fast typists this is fine because the query keeps changing, but for any scenario where results update while the user is navigating (e.g. network latency), the selection jumps back to the top unexpectedly.
Consider only setting `selectedValue` when the query itself changes (i.e. also tracking `query` in the dependency array and resetting only on query changes, not on result updates for the same query).
How can I resolve this? If you propose a fix, please make it concise.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude take over here and make this actually work, see the greptile comments and make sure we always have the first result selected even after searching |
|
Claude Code is working… I'll analyze this and get back to you. |
|
@claude take over here and make this actually work, see the greptile comments and make sure we always have the first result selected even after searching |
|
Claude Code is working… I'll analyze this and get back to you. |
2 participants
Cuando buscás algo en el registry, ahora el primer resultado viene selected automáticamente.
Changes:
Fixes feedback de kahdri: el primer match ahora viene selected por default.
Greptile Summary
This PR adds auto-selection of the first search result in the registry sidebar. When search results arrive, a new controlled
value/onValueChangepair is wired to theCommandcomponent, and auseEffectsetsselectedValueto the URL of the first result. This enables immediate keyboard-driven navigation (pressing Enter after a search navigates to the top result) without requiring the user to press the down-arrow first.The implementation is clean and well-scoped. Two minor points worth considering:
results.length > 0guard inside the effect is redundant —hasResultsalready encodes that invariant fromuse-search.ts.resultsis in the effect's dependency array,selectedValueis reset to the first item on every API response. If the user is mid-navigation with arrow keys and a new response arrives (e.g. due to network jitter with the same query), their selection is silently reset to the top. This is a subtle UX edge case rather than a functional bug.Confidence Score: 4/5
Commandcomponent's controlled-value API. The two issues found are both style-level: a redundant guard condition and a minor UX edge case where in-flight keyboard navigation resets on each API response. Neither affects correctness or causes runtime errors.Important Files Changed
value/onValueChangeto theCommandcomponent and an effect to auto-select the first search result; logic is mostly sound but the redundantresults.length > 0guard inside the effect (whenhasResultsalready guarantees it) is a minor style issue, and there's a subtle UX concern where in-flight keyboard navigation resets on each new API response.Last reviewed commit: 6b86c76
(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!