Consolidate user search combobox behaviour - #1670
Conversation
Amp-Thread-ID: https://ampcode.com/threads/T-01a05ac9-3763-74fe-8ebd-b4cd88c93554 Co-authored-by: Amp <amp@ampcode.com>
Greptile SummaryThe PR extracts shared debounced user-search behavior into a headless Svelte combobox while preserving the distinct UserPicker and Admin Timeline presentations.
Confidence Score: 5/5The PR appears safe to merge because the previously reported search dismissal and Enter-selection failures are addressed with corresponding regression coverage. No blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "Handle Enter while user search is pendin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (event.key === "Escape") { | ||
| open = false; | ||
| return; | ||
| } |
There was a problem hiding this comment.
When an Admin Timeline user presses Escape while a search is in flight, this branch only closes the list without clearing the query, aborting the request, or advancing its sequence. The response therefore remains current and sets open back to true when it completes, causing the dismissed results to reopen with the old query still visible.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/javascript/components/UserSearchCombobox.svelte
Line: 137-140
Comment:
**Escape does not invalidate search**
When an Admin Timeline user presses Escape while a search is in flight, this branch only closes the list without clearing the query, aborting the request, or advancing its sequence. The response therefore remains current and sets `open` back to true when it completes, causing the dismissed results to reopen with the old query still visible.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| else if (event.key === "ArrowUp") highlight = Math.max(highlight - 1, 0); | ||
| else if (highlight >= 0 && highlight < results.length) | ||
| select(results[highlight]); |
There was a problem hiding this comment.
When Timeline results are displayed and the user presses Enter without first pressing an arrow key, highlight is still -1, so this handler does nothing. The previous Timeline handler selected results[0] in this state, so direct Enter selection now requires an extra ArrowDown keystroke.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/javascript/components/UserSearchCombobox.svelte
Line: 153-155
Comment:
**Enter drops first-result selection**
When Timeline results are displayed and the user presses Enter without first pressing an arrow key, `highlight` is still `-1`, so this handler does nothing. The previous Timeline handler selected `results[0]` in this state, so direct Enter selection now requires an extra ArrowDown keystroke.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Amp-Thread-ID: https://ampcode.com/threads/T-01a05ac9-3763-74fe-8ebd-b4cd88c93554 Co-authored-by: Amp <amp@ampcode.com>
| onkeydown={(event) => { | ||
| if ( | ||
| event.key === "Enter" && | ||
| open && | ||
| highlight === -1 && | ||
| results[0] | ||
| ) { | ||
| event.preventDefault(); | ||
| select(results[0]); | ||
| } else { | ||
| handleKeydown(event); | ||
| } | ||
| }} |
There was a problem hiding this comment.
When a user enters a valid query and presses Enter during the debounce interval or before the request completes, this wrapper delegates to handleKeydown because open is false. That handler prevents the default action but has no highlighted result to select, so no immediate request occurs and no user is added.
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/javascript/pages/Admin/Timeline.svelte
Line: 322-334
Comment:
**Early Enter drops selection**
When a user enters a valid query and presses Enter during the debounce interval or before the request completes, this wrapper delegates to `handleKeydown` because `open` is false. That handler prevents the default action but has no highlighted result to select, so no immediate request occurs and no user is added.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Amp-Thread-ID: https://ampcode.com/threads/T-01a05ac9-3763-74fe-8ebd-b4cd88c93554 Co-authored-by: Amp <amp@ampcode.com>
1 participant
Summary of the problem
UserPicker had robust user search behaviour, while Admin Timeline duplicated it without request cancellation and could send a second search when Enter was pressed before debounced results arrived.
Describe your changes
Extracted a headless UserSearchCombobox that owns debouncing, cancellation, race sequencing and keyboard state while callers keep their own presentation. UserPicker retains its selected-user card and Timeline retains its multi-user chips, presets and compact search results. Added system coverage for stale responses, Enter request behaviour, listbox semantics and keyboard selection.
Screenshots / Media
No visual changes.