Enable typing to open and search select dropdowns#164
Conversation
- Add keydown handler to select trigger that opens popover when typing printable characters - Automatically focus search input when popover opens - Set initial search query to the typed character - Clear search query when popover closes - Maintains normal keyboard navigation (Enter, Space, Arrow keys, etc.)
|
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughAdds keyboard-driven opening and searchable focus/clear behavior to Select: captures printable keystrokes on trigger to open and prefill search, focuses the search input on open, clears search on close, and scrolls the selected item into view. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Trigger
participant SelectComponent as Component
participant SearchInput
participant Popover
User->>Trigger: Type printable character
Trigger->>Component: onKeyDown
Component->>Component: handleTriggerKeyDown (detect printable, set searchQuery)
Component->>Popover: open()
Popover->>Component: onOpenChange(open=true)
Component->>SearchInput: focus via searchInputRef
Component->>Component: scroll selected item into view
User->>Popover: close
Popover->>Component: onOpenChange(open=false)
Component->>Component: clear searchQuery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/components/src/ui/select.tsx`:
- Around line 128-154: The early return in handleTriggerKeyDown stops consumer
buttonProps.onKeyDown from being invoked for navigation keys; update
handleTriggerKeyDown to always call buttonProps.onKeyDown(e) for all events
(including Enter/Space/ArrowUp/ArrowDown/Escape/Tab) before returning, and keep
the existing searchable behavior (preventDefault, open popover via
popoverState.open(), and setSearchQuery) only for printable characters;
reference handleTriggerKeyDown, buttonProps.onKeyDown, popoverState.open,
setSearchQuery and searchable when making the change.
- Around line 199-201: DefaultSearchInput currently does not forward the ref so
SearchInput receives a ref that never becomes searchInputRef.current, breaking
auto-focus; update DefaultSearchInput to accept and forward refs (use
forwardRef) and pass the incoming ref to the underlying input element, ensuring
the SearchInput usage with ref={searchInputRef} receives the real DOM node;
locate DefaultSearchInput and replace its function/constant with
React.forwardRef((props, ref) => ...) and attach ref to the actual input
element, leaving SearchInput and the useEffect that calls
searchInputRef.current.focus() unchanged.
| // Handle keydown on trigger to open popover and start typing | ||
| const handleTriggerKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => { | ||
| // Allow normal keyboard navigation (Enter, Space, Arrow keys, etc.) | ||
| if ( | ||
| e.key === 'Enter' || | ||
| e.key === ' ' || | ||
| e.key === 'ArrowDown' || | ||
| e.key === 'ArrowUp' || | ||
| e.key === 'Escape' || | ||
| e.key === 'Tab' | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| // If it's a printable character and searchable is enabled, open the popover and start typing | ||
| if (searchable && e.key.length === 1 && !e.ctrlKey && !e.metaKey && !e.altKey) { | ||
| e.preventDefault(); | ||
| if (!popoverState.isOpen) { | ||
| popoverState.open(); | ||
| } | ||
| // Set the initial search query | ||
| setSearchQuery(e.key); | ||
| } | ||
|
|
||
| // Call the original onKeyDown if provided | ||
| buttonProps.onKeyDown?.(e); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/components/src/ui/select.tsx | sed -n '120,160p'Repository: lambda-curry/forms
Length of output: 1683
🏁 Script executed:
cat -n packages/components/src/ui/select.tsx | head -130Repository: lambda-curry/forms
Length of output: 5790
🏁 Script executed:
cat -n packages/components/src/ui/select.tsx | sed -n '156,200p'Repository: lambda-curry/forms
Length of output: 2569
🏁 Script executed:
fd select -e test -e spec | head -10Repository: lambda-curry/forms
Length of output: 44
🏁 Script executed:
rg "handleTriggerKeyDown" packages/components/src/ui/select.tsx -B 5 -A 15Repository: lambda-curry/forms
Length of output: 1811
Preserve consumer onKeyDown for navigation keys.
The early return at line 139 prevents buttonProps.onKeyDown from firing for Enter/Space/Arrow/Escape/Tab, breaking the prop delegation contract. Consumers passing an onKeyDown handler expect it to be called for all keyboard events.
🛠️ Proposed fix
const handleTriggerKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => {
// Allow normal keyboard navigation (Enter, Space, Arrow keys, etc.)
if (
e.key === 'Enter' ||
e.key === ' ' ||
e.key === 'ArrowDown' ||
e.key === 'ArrowUp' ||
e.key === 'Escape' ||
e.key === 'Tab'
) {
+ buttonProps.onKeyDown?.(e);
return;
}
// If it's a printable character and searchable is enabled, open the popover and start typing
if (searchable && e.key.length === 1 && !e.ctrlKey && !e.metaKey && !e.altKey) {
e.preventDefault();
if (!popoverState.isOpen) {
popoverState.open();
}
// Set the initial search query
setSearchQuery(e.key);
}
// Call the original onKeyDown if provided
buttonProps.onKeyDown?.(e);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle keydown on trigger to open popover and start typing | |
| const handleTriggerKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => { | |
| // Allow normal keyboard navigation (Enter, Space, Arrow keys, etc.) | |
| if ( | |
| e.key === 'Enter' || | |
| e.key === ' ' || | |
| e.key === 'ArrowDown' || | |
| e.key === 'ArrowUp' || | |
| e.key === 'Escape' || | |
| e.key === 'Tab' | |
| ) { | |
| return; | |
| } | |
| // If it's a printable character and searchable is enabled, open the popover and start typing | |
| if (searchable && e.key.length === 1 && !e.ctrlKey && !e.metaKey && !e.altKey) { | |
| e.preventDefault(); | |
| if (!popoverState.isOpen) { | |
| popoverState.open(); | |
| } | |
| // Set the initial search query | |
| setSearchQuery(e.key); | |
| } | |
| // Call the original onKeyDown if provided | |
| buttonProps.onKeyDown?.(e); | |
| }; | |
| // Handle keydown on trigger to open popover and start typing | |
| const handleTriggerKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => { | |
| // Allow normal keyboard navigation (Enter, Space, Arrow keys, etc.) | |
| if ( | |
| e.key === 'Enter' || | |
| e.key === ' ' || | |
| e.key === 'ArrowDown' || | |
| e.key === 'ArrowUp' || | |
| e.key === 'Escape' || | |
| e.key === 'Tab' | |
| ) { | |
| buttonProps.onKeyDown?.(e); | |
| return; | |
| } | |
| // If it's a printable character and searchable is enabled, open the popover and start typing | |
| if (searchable && e.key.length === 1 && !e.ctrlKey && !e.metaKey && !e.altKey) { | |
| e.preventDefault(); | |
| if (!popoverState.isOpen) { | |
| popoverState.open(); | |
| } | |
| // Set the initial search query | |
| setSearchQuery(e.key); | |
| } | |
| // Call the original onKeyDown if provided | |
| buttonProps.onKeyDown?.(e); | |
| }; |
🤖 Prompt for AI Agents
In `@packages/components/src/ui/select.tsx` around lines 128 - 154, The early
return in handleTriggerKeyDown stops consumer buttonProps.onKeyDown from being
invoked for navigation keys; update handleTriggerKeyDown to always call
buttonProps.onKeyDown(e) for all events (including
Enter/Space/ArrowUp/ArrowDown/Escape/Tab) before returning, and keep the
existing searchable behavior (preventDefault, open popover via
popoverState.open(), and setSearchQuery) only for printable characters;
reference handleTriggerKeyDown, buttonProps.onKeyDown, popoverState.open,
setSearchQuery and searchable when making the change.
| <SearchInput | ||
| ref={searchInputRef} | ||
| placeholder="Search..." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/components/src/ui/select.tsx | head -250 | tail -100Repository: lambda-curry/forms
Length of output: 5451
🏁 Script executed:
wc -l packages/components/src/ui/select.tsxRepository: lambda-curry/forms
Length of output: 103
🏁 Script executed:
grep -n "DefaultSearchInput\|searchInputRef" packages/components/src/ui/select.tsx | head -30Repository: lambda-curry/forms
Length of output: 505
🏁 Script executed:
sed -n '59,65p' packages/components/src/ui/select.tsxRepository: lambda-curry/forms
Length of output: 343
🏁 Script executed:
sed -n '85,105p' packages/components/src/ui/select.tsxRepository: lambda-curry/forms
Length of output: 942
Forward the ref through DefaultSearchInput so auto-focus works.
DefaultSearchInput ignores the ref passed to it, leaving searchInputRef.current null. This breaks the auto-focus functionality in the useEffect that attempts to focus the search input when the popover opens.
🛠️ Proposed fix
-const DefaultSearchInput = forwardRef<HTMLInputElement, React.ComponentPropsWithoutRef<typeof CommandInput>>(
- (props, _ref) => <CommandInput {...props} />,
-);
+const DefaultSearchInput = forwardRef<HTMLInputElement, React.ComponentPropsWithoutRef<typeof CommandInput>>(
+ (props, ref) => <CommandInput ref={ref} {...props} />,
+);🤖 Prompt for AI Agents
In `@packages/components/src/ui/select.tsx` around lines 199 - 201,
DefaultSearchInput currently does not forward the ref so SearchInput receives a
ref that never becomes searchInputRef.current, breaking auto-focus; update
DefaultSearchInput to accept and forward refs (use forwardRef) and pass the
incoming ref to the underlying input element, ensuring the SearchInput usage
with ref={searchInputRef} receives the real DOM node; locate DefaultSearchInput
and replace its function/constant with React.forwardRef((props, ref) => ...) and
attach ref to the actual input element, leaving SearchInput and the useEffect
that calls searchInputRef.current.focus() unchanged.
|
@coderabbitai working on the select component, I wanted to be able to type in that component when it was focused and have it start searching. This almost works but currently when we input our first character it focuses the input and selects the first character so naturally if we keep typing it overwrites the first character. Can you review our approach here and determine what's going on and how we should fix this. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 13400
Root Cause
SolutionAfter focusing the input, explicitly place the cursor at the end of the text instead of letting the browser select it all: useEffect(() => {
if (!popoverState.isOpen) {
// Clear search query when closing
setSearchQuery('');
return;
}
requestAnimationFrame(() => {
if (searchable && searchInputRef.current) {
searchInputRef.current.focus();
// Place cursor at the end instead of selecting all text
const len = searchInputRef.current.value.length;
searchInputRef.current.setSelectionRange(len, len);
}
const selectedEl = selectedItemRef.current as HTMLElement | null;
if (selectedEl) selectedEl.scrollIntoView({ block: 'center' });
});
}, [popoverState.isOpen, searchable]);The |
Prevents text selection when opening select dropdown via typing, allowing users to continue typing without overwriting the first character.
Overview
This PR adds the ability to open select dropdowns and start searching immediately when typing while focused on the select trigger. This improves the user experience by making select components feel more responsive and keyboard-friendly.
Changes
Testing
The changes were tested and the component builds successfully:
yarn lintpassesyarn workspace @lambdacurry/forms buildsucceedsExample Usage
Now when you focus the select and start typing "Cal", the dropdown will open and show California in the filtered results.
Implementation Details
searchInputRefto track the search input elementhandleTriggerKeyDownto detect printable characters and trigger the open/search behavioruseEffectto auto-focus the search input when openingRequested by: Jake Ruesink (@jake)
💻 View my work • About Codegen
⛔ Remove Codegen from PR • 🚫 Ban action checks
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
✏️ Tip: You can customize this high-level summary in your review settings.