-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(InputMenu/SelectMenu): prevent unintended item creation and improve focus handling #5986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v4
Are you sure you want to change the base?
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
π WalkthroughWalkthroughAdds combobox root refs and a post-flush watcher in InputMenu.vue and SelectMenu.vue to call highlightFirstItem() when props.items change while the dropdown is open, ignoreFilter is true, and a createItem exists. Introduces isDropdownOpened state, binds comboboxRootRef, and adds an onInputEnter handler wired to Enter key events to manage create-vs-select behavior. Playground pages (input-menu.vue and select-menu.vue) add parallel "create" flows: searchTermCreate and debounced variants, lazy useFetch calls producing usersCreate with avatar transforms, statusCreate loading flags, and onUserCreate handlers; templates include create-enabled multiple selection inputs/selects. Estimated code review effortπ― 3 (Moderate) | β±οΈ ~20 minutes π₯ Pre-merge checks | β 5β Passed checks (5 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing touchesπ§ͺ Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π€ Fix all issues with AI agents
In `@src/runtime/components/SelectMenu.vue`:
- Around line 487-492: The onInputEnter handler currently calls onCreate(e)
whenever there is no highlighted element regardless of whether creation is
enabled; update onInputEnter to first check props.createItem (or an equivalent
create-enabled flag) and only call onCreate(e) when props.createItem is truthy
and comboboxRootRef.value?.highlightedElement is falsy, so pressing Enter wonβt
emit a create event when create is disabled; refer to the onInputEnter function,
the onCreate handler, and props.createItem to implement this guard.
β¦ndle focus and items properly
|
Just realised there was still a not intuitive situation, when focus was in the input-tag, and user moved the focus out of the Let me know if there is any need for change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π€ Fix all issues with AI agents
In `@src/runtime/components/InputMenu.vue`:
- Around line 503-516: The onInputEnter function currently calls onCreate
unconditionally when there are no items; update it to guard against unwanted
creation by checking the creation flag (e.g., props.createItem or whichever prop
enables creation) before invoking onCreate. In onInputEnter, after the existing
combobox focus and items checks, add a conditional that ensures props.createItem
is truthy (or the appropriate creation-enabled prop) and only then call
onCreate(e); otherwise do nothing (or return). This change should reference
onInputEnter, onCreate, comboboxRootRef, and props.items to locate and fix the
behavior.
β¦putMenu` and `SelectMenu`
InputMenu and SelectMenu (#5974)|
Figured out, that props.items might be changing even if dropdown isn't displayed. In that case focus should not be triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π€ Fix all issues with AI agents
In `@src/runtime/components/SelectMenu.vue`:
- Around line 489-506: The onInputEnter handler currently checks props.items to
decide whether to call comboboxRootRef.value?.highlightFirstItem?.(), which
prevents onCreate from being reached when client-side filtering leaves no
visible items; change that guard to use the rendered list length
(filteredItems.value.length) instead of props.items so Enter will create the
visible "create" option when there are no filtered matches; update the same
pattern in InputMenu.vue (the analogous handler) and keep the rest of the logic
(comboboxRootRef, createItem.value, onCreate) unchanged.
| function onInputEnter(e: Event) { | ||
| // In case combobox has any focus, skip | ||
| if (comboboxRootRef.value?.highlightedElement) { | ||
| return | ||
| } | ||
| // In case we have items but our focus is not within the combobox, just bring the focus back | ||
| if ((props.items && props.items.length > 0)) { | ||
| comboboxRootRef.value?.highlightFirstItem?.() | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| return | ||
| } | ||
| // In case creation is not allowed skip | ||
| if (!createItem.value) { | ||
| return | ||
| } | ||
| onCreate(e) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π§© Analysis chain
π Script executed:
fd -t f "SelectMenu.vue" --strip-cwd-prefixRepository: nuxt/ui
Length of output: 146
π Script executed:
wc -l src/runtime/components/SelectMenu.vueRepository: nuxt/ui
Length of output: 92
π Script executed:
sed -n '480,520p' src/runtime/components/SelectMenu.vue | cat -nRepository: nuxt/ui
Length of output: 1357
π Script executed:
sed -n '1,100p' src/runtime/components/SelectMenu.vue | cat -nRepository: nuxt/ui
Length of output: 4554
π Script executed:
sed -n '100,250p' src/runtime/components/SelectMenu.vue | cat -nRepository: nuxt/ui
Length of output: 7563
π Script executed:
sed -n '250,400p' src/runtime/components/SelectMenu.vue | cat -nRepository: nuxt/ui
Length of output: 5910
π Script executed:
sed -n '400,500p' src/runtime/components/SelectMenu.vue | cat -nRepository: nuxt/ui
Length of output: 3165
π Script executed:
fd -t f "InputMenu.vue" --strip-cwd-prefixRepository: nuxt/ui
Length of output: 144
π Script executed:
wc -l src/runtime/components/InputMenu.vueRepository: nuxt/ui
Length of output: 91
π Script executed:
rg -n "onInputEnter" src/runtime/components/InputMenu.vueRepository: nuxt/ui
Length of output: 189
π Script executed:
sed -n '505,525p' src/runtime/components/InputMenu.vue | cat -nRepository: nuxt/ui
Length of output: 777
π Script executed:
rg -n "filteredItems" src/runtime/components/InputMenu.vue | head -20Repository: nuxt/ui
Length of output: 502
π Script executed:
rg -n "ignoreFilter" src/runtime/components/SelectMenu.vue | head -10Repository: nuxt/ui
Length of output: 206
props.items check prevents onCreate when client-side filtering yields no matches.
When ignoreFilter is false (the default), a user can type a search term that matches nothing in filteredItems, making the create option visible. However, this guard checks props.items (the full, unfiltered list) rather than what's actually rendered. If the raw list has items but filtered results are empty, pressing Enter will call highlightFirstItem() and return instead of reaching onCreate.
This forces the user to press Enter twice (once to highlight the create item, once to select it).
The same pattern exists in InputMenu.vue (line 505).
Use filteredItems.value.length instead:
Proposed fix
- if ((props.items && props.items.length > 0)) {
+ if (filteredItems.value.length > 0) {
comboboxRootRef.value?.highlightFirstItem?.()
e.preventDefault()
e.stopPropagation()
return
}π 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.
| function onInputEnter(e: Event) { | |
| // In case combobox has any focus, skip | |
| if (comboboxRootRef.value?.highlightedElement) { | |
| return | |
| } | |
| // In case we have items but our focus is not within the combobox, just bring the focus back | |
| if ((props.items && props.items.length > 0)) { | |
| comboboxRootRef.value?.highlightFirstItem?.() | |
| e.preventDefault() | |
| e.stopPropagation() | |
| return | |
| } | |
| // In case creation is not allowed skip | |
| if (!createItem.value) { | |
| return | |
| } | |
| onCreate(e) | |
| } | |
| function onInputEnter(e: Event) { | |
| // In case combobox has any focus, skip | |
| if (comboboxRootRef.value?.highlightedElement) { | |
| return | |
| } | |
| // In case we have items but our focus is not within the combobox, just bring the focus back | |
| if (filteredItems.value.length > 0) { | |
| comboboxRootRef.value?.highlightFirstItem?.() | |
| e.preventDefault() | |
| e.stopPropagation() | |
| return | |
| } | |
| // In case creation is not allowed skip | |
| if (!createItem.value) { | |
| return | |
| } | |
| onCreate(e) | |
| } |
π€ Prompt for AI Agents
In `@src/runtime/components/SelectMenu.vue` around lines 489 - 506, The
onInputEnter handler currently checks props.items to decide whether to call
comboboxRootRef.value?.highlightFirstItem?.(), which prevents onCreate from
being reached when client-side filtering leaves no visible items; change that
guard to use the rendered list length (filteredItems.value.length) instead of
props.items so Enter will create the visible "create" option when there are no
filtered matches; update the same pattern in InputMenu.vue (the analogous
handler) and keep the rest of the logic (comboboxRootRef, createItem.value,
onCreate) unchanged.
π Linked issue
Resolves #5974
β Type of change
π Description
Based on the issues described in #5974, following fixes were applied:
TagsInputInputpreventing calling default Reka behaviour, which triggers an internalonAddValue, which isn't connected in any way to Nuxt UIitemsprop, and in filters are ignored and creation is enabled, trigger to select first item in the dropdown.π Checklist
I don't think there is a need to write anything to documentation?