feat(DropdownMenu): add DropdownMenuFilter component #2405
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Filter as DropdownMenuFilter
participant Content as MenuContentImpl
participant Item as MenuItemImpl
participant Template as Template
User->>Filter: Type characters
Filter->>Content: Update searchRef
Content->>Template: Trigger reactivity
Template->>Item: Re-render items (filtered)
User->>Filter: Press ArrowDown
Filter->>Content: Call onKeydownNavigation
Content->>Content: Update highlightedElement
Item->>Item: Recompute isHighlighted
Template->>Template: Render highlight
User->>Filter: Press Escape (with value)
Filter->>Content: Stop propagation, reset search
Content->>Template: Clear filter, maintain menu open
Template->>Template: Show all items again
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![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
🤖 Fix all issues with AI agents
In `@packages/core/src/DropdownMenu/DropdownMenuFilter.vue`:
- Line 4: The typeahead state (searchRef) can fall out of sync when the parent
initializes or updates modelValue because searchRef is only changed on user
input, Escape, and unmount; add a watcher for modelValue with immediate: true to
set searchRef.value = modelValue so the internal typeahead state reflects
external updates. Import watch from 'vue' (in addition to watchSyncEffect) and
add watch(modelValue, v => { searchRef.value = v ?? '' }, { immediate: true })
inside the DropdownMenuFilter component so searchRef stays synchronized on mount
and whenever modelValue changes.
🧹 Nitpick comments (3)
packages/core/src/Menu/MenuSubTrigger.vue (1)
26-36: Consider clearingactiveSubmenuContexton unmount.The watcher correctly synchronizes the active submenu context. However, if this component unmounts while the submenu is open,
activeSubmenuContextmay still reference the stale trigger.🔧 Suggested enhancement
onUnmounted(() => { clearOpenTimer() + if (contentContext.activeSubmenuContext.value?.trigger.value === subContext.trigger.value) { + contentContext.activeSubmenuContext.value = undefined + } })packages/core/src/DropdownMenu/story/DropdownMenuFilter.story.vue (1)
133-136: Consider separator visibility logic.The separator's visibility is tied only to "Developer Tools" matching. Typically, a separator should remain visible when any items exist on both sides of it. Consider showing the separator when any of the items above it match OR the "Developer Tools" item matches.
♻️ Suggested improvement
<DropdownMenuSeparator - v-if="matches('Developer Tools', subFilterText)" + v-if="(matches('Save Page As…', subFilterText) || matches('Create Shortcut…', subFilterText) || matches('Name Window…', subFilterText)) && matches('Developer Tools', subFilterText)" :class="separator" />packages/core/src/Menu/MenuContentImpl.vue (1)
162-166: Consider adding null check or early return.The
onKeydownEnterhandler clicks the highlighted element, but consider whether the calling code handles cases where no item is highlighted (e.g., when the filter yields no results). IfhighlightedElement.valueisundefined, this is a no-op, which is fine, but documenting this behavior or adding a guard would improve clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/core/constant/components.tspackages/core/src/DropdownMenu/DropdownMenuFilter.vuepackages/core/src/DropdownMenu/index.tspackages/core/src/DropdownMenu/story/DropdownMenuFilter.story.vuepackages/core/src/Menu/MenuContentImpl.vuepackages/core/src/Menu/MenuItemImpl.vuepackages/core/src/Menu/MenuSubContent.vuepackages/core/src/Menu/MenuSubTrigger.vuepackages/core/src/shared/useArrowNavigation.test.tspackages/core/src/shared/useArrowNavigation.tspackages/plugins/src/namespaced/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/shared/useArrowNavigation.test.ts (1)
packages/core/src/shared/useArrowNavigation.ts (1)
useArrowNavigation(76-145)
🔇 Additional comments (19)
packages/core/src/shared/useArrowNavigation.test.ts (2)
116-127: LGTM! Good edge case coverage.This test correctly validates that navigating forward (ArrowDown) from an external element lands on the first item in the collection.
129-145: LGTM! Complements the forward navigation test.This test correctly validates that navigating backward (ArrowUp) from an external element lands on the last item in the collection.
packages/core/src/shared/useArrowNavigation.ts (1)
172-184: LGTM! Clean handling of external element navigation.The logic correctly handles the case where
currentElementis not in the collection:
- Forward navigation starts from the first element
- Backward navigation starts from the last element
- The extra iteration (
elements.length + 1) ensures all elements can be checked even when starting from outside the collectionpackages/core/constant/components.ts (1)
178-182: LGTM! Consider reordering for consistency with Listbox.The component is correctly registered. For consistency with
listbox(whereListboxFilterappears near the top of the list afterListboxContent), you might consider placingDropdownMenuFiltercloser toDropdownMenuContent. This is purely a stylistic preference.packages/core/src/DropdownMenu/index.ts (1)
15-19: Export structure is correct and follows established conventions.The DropdownMenuFilter export properly matches the component definition, with both DropdownMenuFilterProps and DropdownMenuFilterEmits correctly exported. The placement is alphabetically ordered and consistent with the pattern used by all other components in this barrel file.
packages/core/src/Menu/MenuItemImpl.vue (3)
36-36: LGTM! Good separation of highlight state from focus.The computed
isHighlightedcorrectly combines local focus state with the sharedhighlightedElementcontext, enabling items to appear highlighted while focus remains on the filter input.
50-54: LGTM! Proper focus management with input awareness.The logic correctly prevents stealing focus from filter inputs by checking if an INPUT or TEXTAREA is currently focused before calling
focus(). The optional chaining ongetActiveElement()?.tagNamehandles the null case appropriately.
84-90: LGTM! Focus handler correctly updates highlighted element.The focus handler properly synchronizes
highlightedElementwith the focused item, maintaining consistency between the highlight and focus states.packages/core/src/DropdownMenu/story/DropdownMenuFilter.story.vue (2)
39-41: LGTM! Clean filter helper function.The
matcheshelper is concise and handles the empty filter case correctly by returningtruewhen no filter is applied.
67-72: LGTM! Good demonstration of nested filter usage.The story effectively showcases the DropdownMenuFilter in both the main menu and submenu contexts with proper
v-modelbinding and theauto-focusprop.Also applies to: 106-111
packages/plugins/src/namespaced/index.ts (1)
312-312: LGTM! Consistent namespace registration.The
DropdownMenuFilteris correctly added to both the runtime object and type assertion, following the established pattern for other components in this file.Also applies to: 330-330
packages/core/src/Menu/MenuSubContent.vue (1)
84-91: LGTM! Proper submenu close handling with filter awareness.The implementation correctly maintains focus on the parent's filter input while visually highlighting the submenu trigger, providing good UX for keyboard navigation. The
scrollIntoView({ block: 'nearest' })ensures the trigger remains visible without unnecessary scrolling.One minor consideration:
menuSubContext.trigger.valuecould theoretically beundefined, but since this code path only executes when closing an open submenu, the trigger should always exist at that point.packages/core/src/Menu/MenuContentImpl.vue (5)
143-160: LGTM! Well-structured arrow navigation handler.The
onKeydownNavigationfunction correctly usesuseArrowNavigationwithfocus: falseto calculate the next element without immediately focusing it, then updateshighlightedElementand ensures visibility withscrollIntoView. This enables keyboard navigation while keeping focus on the filter input.
171-176: LGTM! Smart submenu auto-close on navigation.The watcher correctly closes active submenus when the highlighted element moves away from the submenu trigger. This prevents "orphaned" open submenus during keyboard navigation and improves UX.
243-244: LGTM! Correct typeahead exclusion for input elements.Adding the check for INPUT and TEXTAREA tag names prevents typeahead search from interfering when users are typing in the filter input.
297-301: LGTM! Consistent input focus preservation.The
onItemLeavehandler now correctly checks if an input is focused before refocusing the content element, maintaining consistency with the focus management pattern used elsewhere in this PR.
312-319: LGTM! Context properly exposes new filter-related state.All new context fields are correctly provided, enabling child components to interact with the filter element, highlighted state, and navigation handlers.
packages/core/src/DropdownMenu/DropdownMenuFilter.vue (2)
88-103: Keyboard routing looks consistent with menu navigation.Arrow/home/end routing plus Enter handling and Escape-to-clear behavior are coherent with expected menu UX.
106-125: ARIA wiring and disabled semantics look solid.
role="searchbox",aria-activedescendant, and disabled attributes are well covered.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <script lang="ts"> | ||
| import type { PrimitiveProps } from '@/Primitive' | ||
| import { useVModel } from '@vueuse/core' | ||
| import { computed, onMounted, onUnmounted, ref, watchSyncEffect } from 'vue' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/core/src/DropdownMenu/DropdownMenuFilter.vueRepository: unovue/reka-ui
Length of output: 4605
🏁 Script executed:
rg -A 5 "searchRef" packages/core/src/Menu/ --type ts --type vue | head -100Repository: unovue/reka-ui
Length of output: 85
🏁 Script executed:
rg "searchRef" packages/core/src/Menu/ -A 3 -B 1 | head -150Repository: unovue/reka-ui
Length of output: 2359
🏁 Script executed:
rg -B 5 -A 10 "searchRef.value" packages/core/src/Menu/MenuContentImpl.vue | head -80Repository: unovue/reka-ui
Length of output: 631
🏁 Script executed:
rg -B 5 -A 10 "typeahead|search.*filter|searchRef" packages/core/src/Menu/MenuContentImpl.vue | grep -A 15 -B 5 "typeahead\|isValidTypeaheadKey"Repository: unovue/reka-ui
Length of output: 40
🏁 Script executed:
cat -n packages/core/src/Menu/MenuContentImpl.vue | grep -A 5 -B 5 "searchRef"Repository: unovue/reka-ui
Length of output: 1803
🏁 Script executed:
rg "searchRef\.value" packages/core/src/Menu/ -B 3 -A 3 | head -200Repository: unovue/reka-ui
Length of output: 1652
🏁 Script executed:
rg -B 10 -A 10 "onKeydownNavigation|onKeydownEnter" packages/core/src/Menu/MenuContentImpl.vue | head -150Repository: unovue/reka-ui
Length of output: 2748
🏁 Script executed:
rg "useTypeahead" packages/core/src/ -A 20 | head -100Repository: unovue/reka-ui
Length of output: 7764
🏁 Script executed:
find packages/core/src -name "*useTypeahead*" -o -name "*typeahead*" | head -20Repository: unovue/reka-ui
Length of output: 98
🏁 Script executed:
cat -n packages/core/src/shared/useTypeahead.tsRepository: unovue/reka-ui
Length of output: 3802
🏁 Script executed:
cat -n packages/core/src/DropdownMenu/DropdownMenuFilter.vue | sed -n '37,45p'Repository: unovue/reka-ui
Length of output: 398
Sync searchRef whenever modelValue changes to prevent typeahead desync.
searchRef only updates on user input, Escape key, and unmount. If modelValue is initialized non-empty or updated by the parent, the menu's typeahead state can drift from the input value, causing incorrect space-key behavior. Add a watch(modelValue) with immediate: true to keep searchRef in sync on mount and whenever the parent updates the value.
🔧 Suggested fix
-import { computed, onMounted, onUnmounted, ref, watchSyncEffect } from 'vue'
+import { computed, onMounted, onUnmounted, ref, watch, watchSyncEffect } from 'vue'
@@
const rootContext = injectMenuRootContext()
const contentContext = injectMenuContentContext()
const subContext = injectMenuSubContext(null)
+
+watch(modelValue, (value) => {
+ contentContext.searchRef.value = value ?? ''
+}, { immediate: true })
@@
function handleInput(event: InputEvent) {
const target = event.target as HTMLInputElement
modelValue.value = target.value
- // Update the menu's search ref to help with filtering
- contentContext.searchRef.value = target.value
}🤖 Prompt for AI Agents
In `@packages/core/src/DropdownMenu/DropdownMenuFilter.vue` at line 4, The
typeahead state (searchRef) can fall out of sync when the parent initializes or
updates modelValue because searchRef is only changed on user input, Escape, and
unmount; add a watcher for modelValue with immediate: true to set
searchRef.value = modelValue so the internal typeahead state reflects external
updates. Import watch from 'vue' (in addition to watchSyncEffect) and add
watch(modelValue, v => { searchRef.value = v ?? '' }, { immediate: true })
inside the DropdownMenuFilter component so searchRef stays synchronized on mount
and whenever modelValue changes.
1 participant
🔗 Linked issue
❓ Type of change
📚 Description
This PR adds filter support documentation and requirements to the
DropdownMenucomponent, aligning it with the existingListboxfilter behavior.The DropdownMenu previously did not define filtering or search behavior. This change documents an optional filter input that provides keyboard navigation, accessibility guarantees, and submenu awareness.
DropdownMenu📸 Screenshots (if appropriate)
📝 Checklist
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.