fix(combobox): merge consumer props through getReferenceProps (#806)#2014
fix(combobox): merge consumer props through getReferenceProps (#806)#2014kotAPI wants to merge 1 commit into
Conversation
Pass trigger and search props through floating-ui getReferenceProps so consumer onClick and other handlers compose with interaction hooks. Fixes #806
|
📝 WalkthroughWalkthroughThe ChangesCombobox getReferenceProps Prop Forwarding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CoverageThis report compares the PR with the base branch. "Δ" shows how the PR affects each metric.
Coverage decreased for at least one metric. Please add or update tests to improve coverage. Run |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/primitives/Combobox/fragments/ComboboxPrimitiveSearch.tsx`:
- Around line 42-48: The onChange event handler in ComboboxPrimitiveSearch is
overwriting props.onChange instead of composing it, which prevents user-provided
onChange handlers from executing when the search input changes. Locate the
onChange handler definition in the component (around lines 58-63), and ensure it
calls the original props.onChange callback after or alongside the custom
onChange logic, following the same composition pattern used in handleKeyDown
where props.onKeyDown?.(event) is called to preserve the prop composition chain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 460ca7a8-2918-4c7f-934c-eabe5551a55e
📒 Files selected for processing (3)
src/core/primitives/Combobox/contexts/ComboboxPrimitiveContext.tsxsrc/core/primitives/Combobox/fragments/ComboboxPrimitiveSearch.tsxsrc/core/primitives/Combobox/fragments/ComboboxPrimitiveTrigger.tsx
| const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => { | ||
| if (activeIndex !== null && event.key === KEYBOARD_KEYS.ENTER) { | ||
| event.preventDefault(); | ||
| handleSelect(activeIndex); | ||
| } | ||
| props.onKeyDown?.(event); | ||
| }; |
There was a problem hiding this comment.
Consumer onChange is dropped instead of composed.
Line 61 overwrites props.onChange and never calls it, so user input handlers on Search won't run. That breaks the prop/event composition objective for this primitive.
Proposed fix
- const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
- if (activeIndex !== null && event.key === KEYBOARD_KEYS.ENTER) {
- event.preventDefault();
- handleSelect(activeIndex);
- }
- props.onKeyDown?.(event);
- };
+ const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
+ if (activeIndex !== null && event.key === KEYBOARD_KEYS.ENTER) {
+ event.preventDefault();
+ handleSelect(activeIndex);
+ }
+ props.onKeyDown?.(event);
+ };
+
+ const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+ setSearch(e.target.value);
+ props.onChange?.(e);
+ };
{...getReferenceProps({
...props,
value: search,
- onChange: (e: React.ChangeEvent<HTMLInputElement>) => setSearch(e.target.value),
+ onChange: handleChange,
onKeyDown: handleKeyDown
})}Also applies to: 58-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/primitives/Combobox/fragments/ComboboxPrimitiveSearch.tsx` around
lines 42 - 48, The onChange event handler in ComboboxPrimitiveSearch is
overwriting props.onChange instead of composing it, which prevents user-provided
onChange handlers from executing when the search input changes. Locate the
onChange handler definition in the component (around lines 58-63), and ensure it
calls the original props.onChange callback after or alongside the custom
onChange logic, following the same composition pattern used in handleKeyDown
where props.onKeyDown?.(event) is called to preserve the prop composition chain.
Code reviewLGTM. Matches project patterns for portal Theme refs, Floating UI prop merge, controlled-switch/lazy-mount/RTL tests, or focused bug fixes. No changes requested. |
1 participant
Summary
Fixes #806
Test plan
Summary by CodeRabbit