Merged
Conversation
Add `filter`, `filterFields`, `ignoreFilter` props and `searchTerm` model to enable search/filtering within the dropdown menu using Reka UI's new `DropdownMenuFilter` component. The filter input uses the same `useFilter` composable as SelectMenu for consistent filtering behavior (exact > startsWith > contains). Structural items (labels, separators) are preserved and an empty state is shown when no items match.
Open
Contributor
|
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 client-side filtering to DropdownMenu: new per-item Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/runtime/components/DropdownMenuContent.vue (1)
196-213: Forward parent filter config to recursive submenu content for consistent UX.Line 211 forwards only
item.filter; submenu filters won’t inherit parentsize,filterFields, orignoreFilterunless repeated manually in eachitem.content.Proposed refactor
<UDropdownMenuContent sub :class="item.ui?.content" :ui="ui" :ui-override="uiOverride" :portal="portal" :items="(item.children as T)" align="start" :align-offset="-4" :side-offset="3" :label-key="labelKey" :description-key="descriptionKey" :checked-icon="checkedIcon" :loading-icon="loadingIcon" :external-icon="externalIcon" + :size="size" + :filter-fields="filterFields" + :ignore-filter="ignoreFilter" :filter="item.filter" v-bind="item.content" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/DropdownMenuContent.vue` around lines 196 - 213, The submenu UDropdownMenuContent is only receiving item.filter so recursive submenus lose parent-level filter configuration (size, filterFields, ignoreFilter); update the props passed to UDropdownMenuContent (the call site of UDropdownMenuContent inside DropdownMenuContent.vue) to forward the parent filter-related props when item doesn't override them — e.g., pass :filter="item.filter ?? filter", :size="item.size ?? size", :filter-fields="item.filterFields ?? filterFields", and :ignore-filter="item.ignoreFilter ?? ignoreFilter" (or the equivalent prop names used in this component) so recursive submenus inherit parent filter config unless explicitly overridden in item.content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/2.components/context-menu.md`:
- Line 322: The MD003 heading-style lint failures are coming from the new
example blocks that include the line "collapse: true"; fix by either converting
the offending Setext-style headings in those example blocks to consistent
ATX-style headings (use '#' prefixes) or wrap only the example block with
targeted lint directives <!-- markdownlint-disable MD003 --> before the block
and <!-- markdownlint-enable MD003 --> after it so the rest of the file remains
linted; update the two places associated with the "collapse: true" example
blocks (the blocks currently flagged) accordingly and re-run pnpm run lint.
In `@docs/content/docs/2.components/dropdown-menu.md`:
- Line 103: The fragment link for the `filter?: boolean | InputProps` API entry
points to `#with-filtered-items` but the actual section heading is “With filter
items”; update the link target to match the heading (e.g., change
`](`#with-filtered-items`)` to `](`#with-filter-items`)`) or rename the heading to
`With filtered items` so the anchor and link match; edit the markdown line
containing `filter?: boolean | InputProps` to fix the fragment.
In `@src/runtime/components/DropdownMenuContent.vue`:
- Around line 111-114: The current assignment const fields =
Array.isArray(props.filterFields) ? props.filterFields : [props.labelKey] allows
an empty array to be used and thus disables matching; change the logic so that
when props.filterFields is not an array or is an empty array you fall back to
[props.labelKey]. Update the fields determination (used in the filterGroups call
with groups.value and searchTerm.value) to check
Array.isArray(props.filterFields) && props.filterFields.length > 0 and only then
use props.filterFields, otherwise set fields = [props.labelKey].
---
Nitpick comments:
In `@src/runtime/components/DropdownMenuContent.vue`:
- Around line 196-213: The submenu UDropdownMenuContent is only receiving
item.filter so recursive submenus lose parent-level filter configuration (size,
filterFields, ignoreFilter); update the props passed to UDropdownMenuContent
(the call site of UDropdownMenuContent inside DropdownMenuContent.vue) to
forward the parent filter-related props when item doesn't override them — e.g.,
pass :filter="item.filter ?? filter", :size="item.size ?? size",
:filter-fields="item.filterFields ?? filterFields", and
:ignore-filter="item.ignoreFilter ?? ignoreFilter" (or the equivalent prop names
used in this component) so recursive submenus inherit parent filter config
unless explicitly overridden in item.content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4debd045-cb00-4511-8854-644779dad535
⛔ Files ignored due to path filters (6)
test/components/__snapshots__/DropdownMenu-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/DropdownMenu.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/InputMenu-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/InputMenu.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/SelectMenu-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/SelectMenu.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (74)
docs/app/components/content/examples/dropdown-menu/DropdownMenuFilterFieldsExample.vuedocs/app/components/content/examples/dropdown-menu/DropdownMenuFilterItemsExample.vuedocs/app/components/content/examples/dropdown-menu/DropdownMenuIgnoreFilterExample.vuedocs/app/components/content/examples/dropdown-menu/DropdownMenuSwitchItemsExample.vuedocs/content/docs/2.components/context-menu.mddocs/content/docs/2.components/dropdown-menu.mdplaygrounds/nuxt/app/pages/components/dropdown-menu.vuesrc/runtime/components/DropdownMenu.vuesrc/runtime/components/DropdownMenuContent.vuesrc/runtime/components/SelectMenu.vuesrc/runtime/locale/ar.tssrc/runtime/locale/az.tssrc/runtime/locale/be.tssrc/runtime/locale/bg.tssrc/runtime/locale/bn.tssrc/runtime/locale/ca.tssrc/runtime/locale/ckb.tssrc/runtime/locale/cs.tssrc/runtime/locale/da.tssrc/runtime/locale/de.tssrc/runtime/locale/de_ch.tssrc/runtime/locale/el.tssrc/runtime/locale/en.tssrc/runtime/locale/es.tssrc/runtime/locale/et.tssrc/runtime/locale/eu.tssrc/runtime/locale/fa_ir.tssrc/runtime/locale/fi.tssrc/runtime/locale/fr.tssrc/runtime/locale/gl.tssrc/runtime/locale/he.tssrc/runtime/locale/hi.tssrc/runtime/locale/hr.tssrc/runtime/locale/hu.tssrc/runtime/locale/hy.tssrc/runtime/locale/id.tssrc/runtime/locale/is.tssrc/runtime/locale/it.tssrc/runtime/locale/ja.tssrc/runtime/locale/ka.tssrc/runtime/locale/kk.tssrc/runtime/locale/km.tssrc/runtime/locale/ko.tssrc/runtime/locale/ky.tssrc/runtime/locale/lb.tssrc/runtime/locale/lo.tssrc/runtime/locale/lt.tssrc/runtime/locale/mn.tssrc/runtime/locale/ms.tssrc/runtime/locale/nb_no.tssrc/runtime/locale/nl.tssrc/runtime/locale/pl.tssrc/runtime/locale/pt.tssrc/runtime/locale/pt_br.tssrc/runtime/locale/ro.tssrc/runtime/locale/ru.tssrc/runtime/locale/sk.tssrc/runtime/locale/sl.tssrc/runtime/locale/sq.tssrc/runtime/locale/sv.tssrc/runtime/locale/th.tssrc/runtime/locale/tj.tssrc/runtime/locale/tr.tssrc/runtime/locale/ug_cn.tssrc/runtime/locale/uk.tssrc/runtime/locale/ur.tssrc/runtime/locale/uz.tssrc/runtime/locale/vi.tssrc/runtime/locale/zh_cn.tssrc/runtime/locale/zh_tw.tssrc/runtime/types/locale.tssrc/theme/dropdown-menu.tssrc/theme/input-menu.tssrc/theme/select.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/components/DropdownMenu.spec.ts (1)
132-133: Add one explicit no-match assertion for thesearchTermscenario.The new case is valuable, but a targeted assertion for empty-state rendering would make regressions easier to detect than matrix rendering alone.
Suggested focused test
+it('shows empty state when filter has no match', async () => { + const wrapper = await mountSuspended(DropdownMenu, { + props: { ...props, filter: true, searchTerm: 'No match value' } + }) + + expect(wrapper.text()).toContain('No matching data') +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/components/DropdownMenu.spec.ts` around lines 132 - 133, The 'with filter and searchTerm' matrix case in DropdownMenu.spec.ts lacks an explicit assertion that verifies the empty-state when there are no matches; update the test that iterates the matrix (or the specific case handling the tuple ['with filter and searchTerm', { props: { ...props, filter: true, searchTerm: 'No match value' } }]) to include a focused expectation that the component renders the no-match UI (e.g., asserts presence of the empty state text or an element like "no results" from DropdownMenu/DropdownList), so the test specifically fails if the empty-state is not shown for that searchTerm scenario. Ensure you reference the same matrix entry and add the assertion near where other case-specific expectations are run.src/runtime/components/DropdownMenuContent.vue (2)
115-118: Extract structural-item detection into one helper.The structural check is duplicated and can drift over time; centralizing it improves maintainability.
Refactor sketch
+const isStructuralItem = (item: DropdownMenuItem) => !!item.type && ['label', 'separator'].includes(item.type) + const filteredGroups = computed(() => { if (!props.filter || props.ignoreFilter || !searchTerm.value) { return groups.value } const fields = Array.isArray(props.filterFields) && props.filterFields.length ? props.filterFields : [props.labelKey] as string[] return filterGroups(groups.value, searchTerm.value, { fields, - isStructural: (item: DropdownMenuItem) => !!item.type && ['label', 'separator'].includes(item.type) + isStructural: isStructuralItem }) }) -const hasFilteredItems = computed(() => filteredGroups.value.some(group => group.some(item => !item.type || !['label', 'separator'].includes(item.type)))) +const hasFilteredItems = computed(() => filteredGroups.value.some(group => group.some(item => !isStructuralItem(item))))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/DropdownMenuContent.vue` around lines 115 - 118, Create a single helper to detect structural items (e.g., isStructuralItem) and replace the duplicated inline checks: extract the logic currently used in the isStructural computed and the hasFilteredItems computed into that helper (use the DropdownMenuItem type and check for item.type being 'label' or 'separator'), then call isStructuralItem(item) from both the existing isStructural computed and the filteredGroups.some(...) used in hasFilteredItems so both use the same centralized predicate (update any imports/exports if needed and keep function name isStructuralItem to make usages obvious).
196-212: Consider forwardingsizeto nested submenu content.When a submenu uses
item.filter, its input currently won’t inherit parentsizeunless redefined initem.filterprops.Small consistency fix
<UDropdownMenuContent sub :class="item.ui?.content" :ui="ui" :ui-override="uiOverride" + :size="size" :portal="portal" :items="(item.children as T)" align="start" :align-offset="-4" :side-offset="3"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/DropdownMenuContent.vue` around lines 196 - 212, The nested UDropdownMenuContent should inherit the parent size so submenu filters get the same input sizing; update the UDropdownMenuContent invocation (the component with props like :ui="ui", :items="(item.children as T)", :filter="item.filter", v-bind="item.content") to forward the size prop (e.g., add :size="size") so nested content receives the parent size unless explicitly overridden in item.content or item.filter props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/runtime/components/DropdownMenuContent.vue`:
- Around line 115-118: Create a single helper to detect structural items (e.g.,
isStructuralItem) and replace the duplicated inline checks: extract the logic
currently used in the isStructural computed and the hasFilteredItems computed
into that helper (use the DropdownMenuItem type and check for item.type being
'label' or 'separator'), then call isStructuralItem(item) from both the existing
isStructural computed and the filteredGroups.some(...) used in hasFilteredItems
so both use the same centralized predicate (update any imports/exports if needed
and keep function name isStructuralItem to make usages obvious).
- Around line 196-212: The nested UDropdownMenuContent should inherit the parent
size so submenu filters get the same input sizing; update the
UDropdownMenuContent invocation (the component with props like :ui="ui",
:items="(item.children as T)", :filter="item.filter", v-bind="item.content") to
forward the size prop (e.g., add :size="size") so nested content receives the
parent size unless explicitly overridden in item.content or item.filter props.
In `@test/components/DropdownMenu.spec.ts`:
- Around line 132-133: The 'with filter and searchTerm' matrix case in
DropdownMenu.spec.ts lacks an explicit assertion that verifies the empty-state
when there are no matches; update the test that iterates the matrix (or the
specific case handling the tuple ['with filter and searchTerm', { props: {
...props, filter: true, searchTerm: 'No match value' } }]) to include a focused
expectation that the component renders the no-match UI (e.g., asserts presence
of the empty state text or an element like "no results" from
DropdownMenu/DropdownList), so the test specifically fails if the empty-state is
not shown for that searchTerm scenario. Ensure you reference the same matrix
entry and add the assertion near where other case-specific expectations are run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37597c12-8a22-464a-890d-f006de6db391
⛔ Files ignored due to path filters (2)
test/components/__snapshots__/DropdownMenu-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/DropdownMenu.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
docs/content/docs/2.components/dropdown-menu.mdsrc/runtime/components/DropdownMenuContent.vuetest/components/DropdownMenu.spec.ts
commit: |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/runtime/components/DropdownMenuContent.vue (1)
213-215: Consider inheriting parent filter options for sub-menus by default.Only
item.filteris forwarded here. If parent content usesfilterFieldsorignoreFilter, sub-menus won’t inherit that behavior unless repeated manually.♻️ Optional consistency refactor
:size="size" :filter="item.filter" + :filter-fields="filterFields" + :ignore-filter="ignoreFilter" v-bind="item.content"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/DropdownMenuContent.vue` around lines 213 - 215, In DropdownMenuContent.vue, sub-menus only receive item.filter and lose parent filter options; update the binding so sub-menus inherit parent's filter-related props (e.g., filterFields, ignoreFilter) by merging them with item.content when rendering the child: include filterFields and ignoreFilter (and any other relevant parent filter props) alongside item.filter in the v-bind payload for the child component (preserving item.content values but letting parent-level filter settings be applied by default).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/runtime/components/DropdownMenuContent.vue`:
- Around line 213-215: In DropdownMenuContent.vue, sub-menus only receive
item.filter and lose parent filter options; update the binding so sub-menus
inherit parent's filter-related props (e.g., filterFields, ignoreFilter) by
merging them with item.content when rendering the child: include filterFields
and ignoreFilter (and any other relevant parent filter props) alongside
item.filter in the v-bind payload for the child component (preserving
item.content values but letting parent-level filter settings be applied by
default).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc0c92c2-beee-4118-a12b-560c28807355
📒 Files selected for processing (1)
src/runtime/components/DropdownMenuContent.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/components/DropdownMenuContent.vue`:
- Around line 252-255: The template in DropdownMenuContent.vue emits the slot
prop using the kebab-case key :search-term, but the declared slot type expects
an object with a camelCase property searchTerm; change the slot binding on the
empty slot from :search-term to :searchTerm so parents can destructure {
searchTerm } consistently (update the <slot name="empty"
:searchTerm="searchTerm"> usage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6b73e6f-3796-4917-a105-992843cbbf5e
📒 Files selected for processing (3)
docs/content/docs/2.components/dropdown-menu.mdsrc/runtime/components/DropdownMenu.vuesrc/runtime/components/DropdownMenuContent.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/app/components/content/examples/dropdown-menu/DropdownMenuIgnoreFilterExample.vue`:
- Around line 20-23: The onOpen handler currently ignores the boolean payload
from `@update`:open and calls execute() even when the menu is closed; update the
onOpen function signature to accept the open boolean (e.g., onOpen(open:
boolean)) and only call execute() when open === true and users.value is
empty/undefined, ensuring the fetch is gated to the menu opening and not
triggered on close; reference the onOpen function and the execute() call as the
places to change, and keep handling of users.value?.length the same.
In `@docs/content/docs/2.components/dropdown-menu.md`:
- Around line 144-146: The docs list for the item API uses `filter?: boolean |
InputProps` but runtime excludes `modelValue` and `defaultValue`; update the
`filter` type description to exactly match the supported shape by stating it is
either `boolean` or `InputProps` with the `modelValue` and `defaultValue`
properties removed (i.e., use the InputProps variant that omits `modelValue` and
`defaultValue`) so consumers won’t assume those two props are accepted; adjust
the line that mentions `filter?: boolean | InputProps` accordingly and keep
`filterFields` and `ignoreFilter` as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 347274e5-7cf6-4e3c-98e0-c956bd460112
📒 Files selected for processing (8)
docs/app/components/content/examples/dropdown-menu/DropdownMenuFilterFieldsExample.vuedocs/app/components/content/examples/dropdown-menu/DropdownMenuFilterItemsExample.vuedocs/app/components/content/examples/dropdown-menu/DropdownMenuIgnoreFilterExample.vuedocs/app/components/content/examples/select-menu/SelectMenuIgnoreFilterExample.vuedocs/content/docs/2.components/dropdown-menu.mddocs/content/docs/2.components/input-menu.mddocs/content/docs/2.components/select-menu.mdplaygrounds/nuxt/app/pages/components/dropdown-menu.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/app/components/content/examples/dropdown-menu/DropdownMenuFilterItemsExample.vue
- docs/app/components/content/examples/dropdown-menu/DropdownMenuFilterFieldsExample.vue
docs/app/components/content/examples/dropdown-menu/DropdownMenuIgnoreFilterExample.vue
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
Follows-up on #6147
❓ Type of change
📚 Description
📝 Checklist