feat(web): add typeahead keyboard selection to Select (Alpha) - #870
Conversation
✅ Heimdall Review Status
✅
|
| Code Owner | Status | Calculation | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| ui-systems-eng-team |
✅
1/1
|
Denominator calculation
|
64c2e69 to
2fa6852
Compare
cb-ekuersch
left a comment
There was a problem hiding this comment.
There is a lot going on here - i think it may help to review together. We should potentailly consider the possibility of abstracting this entire typeahead behavior into a single hook that can be called in the component render instead of inlining all this complex react logc in an already compex/long component render body
| type TypeaheadKeyEvent = Pick<KeyboardEvent, 'key' | 'ctrlKey' | 'metaKey' | 'altKey'>; | ||
|
|
||
| // Bare printable key, no modifier (shared by closed and open paths). | ||
| export function isTypeaheadKeyEvent(event: TypeaheadKeyEvent): boolean { |
There was a problem hiding this comment.
nit: why not make this KeyboardEvent?
There was a problem hiding this comment.
Switched this to the explicit React.KeyboardEvent | KeyboardEvent union for readability — it accepts both React's synthetic event from the control onKeyDown and the native event from the window keydown listener, without the structural Pick.
|
|
||
| useEffect( | ||
| () => () => { | ||
| if (typeaheadResetTimeoutRef.current) clearTimeout(typeaheadResetTimeoutRef.current); |
There was a problem hiding this comment.
we should clear this in the cleanup of the effect that creates the timeout
There was a problem hiding this comment.
i see that the timeout is crated in a user interaction so i dont think this effect just to do cleanup in even necessary
There was a problem hiding this comment.
Done — I removed the standalone cleanup-only effect and folded the clearTimeout into the return of the window-listener effect (the effect that actually drives the open typeahead interaction). A reset timeout is only ever pending as part of an open interaction, so that effect's cleanup clears it both on the open→closed transition and on unmount. See the reply on your follow-up comment for the reasoning about why the timer is safe.
There was a problem hiding this comment.
Agreed — the dedicated cleanup-only effect is gone. Two things worth noting: (1) the reset timeout's callback only assigns "" to a ref (typeaheadBufferRef), so it never calls setState or focus; a late fire on an unmounted tree is a harmless no-op, not a stray-timer bug. (2) Even so, I kept an explicit clear by folding it into the window-listener effect's cleanup, which runs on the open→closed transition and on unmount — so no timer lingers. Net: no separate effect just for cleanup, and no stray timer.
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (!open || !pendingTypeAheadKeyRef.current) return; |
There was a problem hiding this comment.
can we label what these different effects are for? Maybe there is value in giving them names and pulling them into custom hooks just for oganization's sake (e.g. useFocusInPortaledDropdown etc.)
There was a problem hiding this comment.
This is largely addressed: all the typeahead effects were extracted out of Select.tsx into a dedicated useTypeahead hook (useTypeahead.ts), which is the "pull them into a custom hook for organization" direction you suggested. Inside the hook I also added terse labels — the type-to-open focus effect and the window-listener effect each have a one-line comment describing their purpose. I intentionally kept it as a single cohesive hook rather than splitting into several micro-hooks (e.g. useFocusInPortaledDropdown), since the effects share the buffer/timeout/focus refs and splitting would add indirection without real clarity. Happy to split further if you feel strongly.
The web Select (Alpha) previously only matched the first letter of an option and only when opening the listbox. Add native <select>-style typeahead: a multi-character search buffer that resets ~500ms after the last keystroke, repeated-key cycling through options that share a first letter, and matching that works both when the listbox is open and closed. Buffer/matching logic is extracted into a pure typeahead module with focused unit tests. Refs CDS-2505 Co-authored-by: Cursor <cursoragent@cursor.com>
88513d5 to
d56c27f
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
…peahead-keyboard-selection
What changed? Why?
Adds native
<select>-style typeahead keyboard selection to the webSelect (Alpha):<select>semantics.keydownlistener so keystrokes are handled consistently even after focus moves into the portaled dropdown.typeahead.tsmodule (isPrintableTypeaheadKey,normalizeOptionText,getTypeaheadMatchIndex) with focused unit tests.The previous implementation only matched the first letter of an option and only at open time, so users could not type multiple characters or cycle between same-prefixed options.
Testing
How has it been tested?
Testing instructions
Focus a web
Select (Alpha)and type characters: with the listbox closed, typing opens it and focuses the first match; with it open, continue typing to refine the prefix match, or repeat a single letter to cycle through same-prefixed options. Seetypeahead.test.tsfor the pure matching logic and the addedSelect.test.tsxcases for integration behavior.Linear: https://linear.app/coinbase/issue/CDS-2505
Change management
type=routine
risk=low
impact=sev5
automerge=false
Made with Cursor