feat(Combobox): v1 with button-trigger mode, shared popover motion, and curated demos#633
Conversation
- Combobox: rewrite per 08d spec with new per-item `slots` API, intrinsic trigger width, press feedback, chevron rotation; deprecate `render`, `slotName`, `input` event, and `placement`. Split implementation into Combobox.vue + ComboboxResults.vue + utils.ts. - Stories: curate to 7 real/fun demos (simple, grouped, creatable, status picker, member picker, command bar, emoji picker); drop legacy-API demos. - Tests: expand 7 → 34 covering rendering, selection, query, slots, legacy aliases, custom options, empty/loading/footer, positioning, reset. - Shared `usePopoverMotion` composable: pointer-recency check classifies opens as keyboard or pointer; keyboard opens skip the scale entrance and only run a short opacity fade to mask reka's position-settle. - Select / Dropdown: adopt the composable, drop keydown tracking + reset timer. Consistent motion across the family. - FormControl: forward `size` to Combobox now that it honors the prop. - v1 RFC: split shared spec into per-component sub-specs (08a–e) and add shared rule 10 "popover motion conventions". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Combobox: new `trigger: 'input' | 'button'` prop. Button mode renders a built-in Button as trigger; search input moves into popover header and auto-focuses on open. Button label + prefix auto-derive from the selected option (reuses `#item-prefix` slot, falls back to `selectedOption.icon`, then `#prefix` for the placeholder state). - Combobox: `#trigger` slot now implicitly activates button mode with `ComboboxTrigger`-wrapped content (keyboard + aria-expanded handled). - Combobox: `typedQuery` derived — slot props and custom-option handlers see empty string when the user hasn't typed, so a committed label doesn't leak into the slot context (e.g. no "Invite Alex"). - Stories: rework Simple (Frappe repo picker) and Grouped (spaces-by-team with accent squares). Rework EmojiPicker to use `trigger="button"` — single declarative component instead of custom slot wiring. Rework MemberPicker to use real avatars and template slots. Drop CommandBar. - Stories use frappe-ui's color palette only (indigo/rose/sky/emerald were silently dropped since they're not in the preset). - Docs: `<ComponentPreview layout="stacked">` shows preview and code together. Applied to Simple and EmojiPicker. The markdown plugin now forwards all attributes to the component rather than whitelisting `name`/`csr`/`css`. - Spec: document `trigger` prop + prefix priority in 08d. - Tests: 36 passing (added coverage for button-trigger mode). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🚀 VitePress preview is ready: |
|
Note
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The PR title clearly summarizes the main Combobox v1 release with three key features: button-trigger mode, shared popover motion, and curated demos. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
v1-release/combobox
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (9)
docs/.vitepress/plugins/componentTransformer.ts (1)
35-43:parseAttrssilently drops boolean/unquoted attributes.The regex requires
key="..."orkey='...'. Shorthand boolean attributes (e.g.<ComponentPreview name="X-Y" csr />or<ComponentPreview name="X-Y" layout=stacked />) are ignored, socsrwould need to be written ascsr="true"to take effect. If boolean/unquoted usage is not a supported convention in this codebase, no change needed; otherwise consider extending the regex to accept bare and unquoted forms.src/components/Combobox/stories/CustomValue.vue (1)
20-20: Use a semantic ink token instead oftext-gray-600.The sibling
Simple.vuestory usestext-ink-gray-5for the same "Selected:" label — worth matching for consistency and so stories model the token conventions.🎨 Proposed tweak
- <div class="text-sm text-gray-600">Selected: {{ value || 'None' }}</div> + <div class="text-sm text-ink-gray-5">Selected: {{ value || 'None' }}</div>As per coding guidelines: "Use semantic design tokens over hardcoded colors:
bg-surface-*,text-ink-*,border-outline-*,fill-ink-*,placeholder-ink-*".src/components/Combobox/stories/MemberPicker.vue (1)
17-54: Consider inlining (or fallbacking) avatar images for docs resilience.The story hard-codes
https://i.pravatar.cc/80?u=<email>for avatars. If the docs build runs offline or the third-party service is flaky, the preview will show broken images. Since this story anchors the docscombobox.mdMember Picker section, anAvatarwith justlabel(to render initials) or a local SVG would make the preview robust without changing the demonstrated API.src/components/Combobox/Combobox.cy.ts (1)
572-580: Use the documented@cypress/vuereturn shape instead of a defensive fallback chain.The code's own comment states "cypress-vue returns { wrapper, component }", yet it falls back through
mounted.component ?? mounted.wrapper?.vm ?? mounted. Per the@cypress/vuev3 documentation,cy.mount()returns an object with well-definedcomponentandwrapperproperties; thecomponentproperty is the mounted instance with access to exposed methods. Use the documented API directly:cy.mount(Combobox, { ... }).then(({ component }) => { component.reset() })Remove optional chaining (
?.) so a missingresetmethod becomes a TypeScript/linting error rather than a silent no-op.v1-release/08d-combobox-spec.md (1)
74-74: Align therendersignature in the spec with the exported type.The spec declares
render?: (() => VNode) | ItemSlots<ComboboxItemSlotProps>, but the actual exported type insrc/components/Combobox/types.ts(lines 41-43 and 69-71) widens the function form to(() => VNode | VNode[]) | ComboboxItemSlots<...>. Since this is a deprecated compatibility alias, readers migrating offrendermay be misled about what the legacy form actually accepts.📘 Proposed spec tweak
- /** `@deprecated` use `slots` — function → `slots.item`; object → `slots` */ - render?: (() => VNode) | ItemSlots<ComboboxItemSlotProps> + /** `@deprecated` use `slots` — function → `slots.item`; object → `slots` */ + render?: (() => VNode | VNode[]) | ItemSlots<ComboboxItemSlotProps>src/components/Combobox/utils.ts (4)
208-233: Verify intent:lgandxlmap to identical classes.
triggerSizeClassesanditemRootSizeClassesboth return the samemin-h-10/radius/padding forlgandxl, whileinputFontSizeClassesdoes differentiate them (text-lgvstext-xl). If rows/triggers forxlare meant to matchlgheight and only the typography scales, a brief "why" comment would prevent future "fixes" that unintentionally change sizing. Ifxlwas meant to be taller/roomier, these tables are likely under-scaled.As per coding guidelines: "Explain why in code comments, not what".
64-68: Minor: simplify legacy render wrapping and drop redundant cast.
legacyalready matches() => VNode | VNode[], so wrapping it as() => legacy()only exists to strip a props argument thatslots.itemmay pass; assigninglegacydirectly is equivalent and lets TypeScript widen to the slot signature structurally. Likewise the object-form is alreadyComboboxItemSlots<ComboboxItemSlotProps>pertypes.ts, so theas ResolvedItemSlotscast is unnecessary.♻️ Optional cleanup
- if (typeof legacy === 'function') { - legacySlots = { item: () => legacy() } - } else if (legacy && typeof legacy === 'object') { - legacySlots = legacy as ResolvedItemSlots - } + if (typeof legacy === 'function') { + // Legacy function render ignores slot props; expose as full-row takeover. + legacySlots = { item: legacy } + } else if (legacy && typeof legacy === 'object') { + legacySlots = legacy + }
251-258: Mixed Tailwind class syntaxes for the same token.
subtleusesborder-[--surface-gray-2](arbitrary CSS-var) for the border while the background uses the tokenizedbg-surface-gray-2. Ifsurface-gray-2is defined in the Tailwind theme (as thebg-usage implies), the border can use the matching tokenized utility for consistency and theme-resolver parity.♻️ Suggested alignment
- subtle: - 'border border-[--surface-gray-2] bg-surface-gray-2 hover:border-outline-gray-modals hover:bg-surface-gray-3', + subtle: + 'border border-surface-gray-2 bg-surface-gray-2 hover:border-outline-gray-modals hover:bg-surface-gray-3',
111-113: Confirm empty-string values are intentionally allowed here.
option.value === undefined || option.value === nullletsvalue: ''through as a valid selectable. Given theEMPTY_SELECTABLE_VALUE_PREFIXsentinel exists specifically to synthesize keys for empty-valued selections upstream, this is likely deliberate — worth a shortwhycomment so future readers don't tighten the check and break that path.As per coding guidelines: "Explain why in code comments, not what".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d526a3d1-c089-424a-ad28-41a711d148fe
📒 Files selected for processing (41)
components.d.tsdocs/.vitepress/plugins/componentTransformer.tsdocs/components/Docs/Demo.vuedocs/content/components.d.tsdocs/content/docs/components/combobox.mddocs/css/shiki.cssdocs/meta/Calendar.mddocs/meta/Combobox.mddocs/meta/CommandPalette.mddocs/meta/FileUploader.mddocs/meta/ListFilter.mddocs/meta/ListView.mddocs/meta/Popover.mddocs/meta/TabButtons.mddocs/meta/TimePicker.mddocs/meta/Toast.mddocs/meta/Tooltip.mdsrc/components/Combobox/Combobox.cy.tssrc/components/Combobox/Combobox.vuesrc/components/Combobox/ComboboxResults.vuesrc/components/Combobox/stories/CustomRender.vuesrc/components/Combobox/stories/CustomValue.vuesrc/components/Combobox/stories/EmojiPicker.vuesrc/components/Combobox/stories/Grouped.vuesrc/components/Combobox/stories/MemberPicker.vuesrc/components/Combobox/stories/OptionSlots.vuesrc/components/Combobox/stories/Simple.vuesrc/components/Combobox/stories/StatusPicker.vuesrc/components/Combobox/stories/WithIcons.vuesrc/components/Combobox/types.tssrc/components/Combobox/utils.tssrc/components/Dropdown/Dropdown.vuesrc/components/FormControl/FormControl.vuesrc/components/Select/Select.vuesrc/composables/usePopoverMotion.tsv1-release/08-selection-and-menu-api-spec.mdv1-release/08a-itemlist-spec.mdv1-release/08b-dropdown-spec.mdv1-release/08c-select-spec.mdv1-release/08d-combobox-spec.mdv1-release/08e-multiselect-spec.md
💤 Files with no reviewable changes (4)
- src/components/Combobox/stories/WithIcons.vue
- src/components/Combobox/stories/OptionSlots.vue
- src/components/Combobox/stories/CustomRender.vue
- docs/content/components.d.ts
| const forwardedAttrs = Object.entries(attrs) | ||
| .filter(([key]) => key !== 'csr') | ||
| .map(([key, value]) => ` ${key}="${value}"`) | ||
| .join('') | ||
| state.tokens[idx].content = | ||
| `${open}<ComponentPreview name="${name}"${cssAttr}><${storyImportName} /><template #code>` | ||
| `${open}<ComponentPreview${forwardedAttrs}><${storyImportName} /><template #code>` |
There was a problem hiding this comment.
Forwarded attribute values aren't HTML-escaped when re-quoted.
parseAttrs accepts both "..." and '...' forms, so a single-quoted value can legitimately contain ". Re-emitting every attribute with `${key}="${value}"` without escaping will produce a malformed Vue tag in that case. Recommend escaping &, <, and " (or at minimum ") before interpolating.
🛡️ Proposed fix
+ const escapeAttr = (v: string) =>
+ v.replace(/&/g, '&').replace(/"/g, '"').replace(/</g, '<')
+
const forwardedAttrs = Object.entries(attrs)
.filter(([key]) => key !== 'csr')
- .map(([key, value]) => ` ${key}="${value}"`)
+ .map(([key, value]) => ` ${key}="${escapeAttr(value)}"`)
.join('')📝 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.
| const forwardedAttrs = Object.entries(attrs) | |
| .filter(([key]) => key !== 'csr') | |
| .map(([key, value]) => ` ${key}="${value}"`) | |
| .join('') | |
| state.tokens[idx].content = | |
| `${open}<ComponentPreview name="${name}"${cssAttr}><${storyImportName} /><template #code>` | |
| `${open}<ComponentPreview${forwardedAttrs}><${storyImportName} /><template #code>` | |
| const escapeAttr = (v: string) => | |
| v.replace(/&/g, '&').replace(/"/g, '"').replace(/</g, '<') | |
| const forwardedAttrs = Object.entries(attrs) | |
| .filter(([key]) => key !== 'csr') | |
| .map(([key, value]) => ` ${key}="${escapeAttr(value)}"`) | |
| .join('') | |
| state.tokens[idx].content = | |
| `${open}<ComponentPreview${forwardedAttrs}><${storyImportName} /><template `#code`>` |
| { | ||
| name: 'header', | ||
| description: '', | ||
| type: '{ currentMonthYear: any; currentYear: number; currentMonth: number; enabledModes: { label: string; v' | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for a length/truncation cap in the propsgen script and other meta files with the same cut-off symptom.
fd -H 'propsgen.ts' scripts 2>/dev/null | xargs -r rg -nC2 -P 'slice|substr|\.\.\.|truncat|maxLen|length\s*[<>]'
rg -nP "[A-Za-z_]$" docs/meta -g '*.md' | head -n 50Repository: frappe/frappe-ui
Length of output: 2329
🏁 Script executed:
# Find and examine the propsgen script
find . -name 'propsgen.ts' -type fRepository: frappe/frappe-ui
Length of output: 86
🏁 Script executed:
# Read the Calendar.md file to see the actual truncation
head -60 docs/meta/Calendar.md | tail -20Repository: frappe/frappe-ui
Length of output: 481
🏁 Script executed:
# Also check Popover.md for similar issues
grep -n "target\|body-main" docs/meta/Popover.md | head -10Repository: frappe/frappe-ui
Length of output: 260
🏁 Script executed:
# Read the propsgen.ts file to find truncation logic
cat docs/scripts/propsgen.tsRepository: frappe/frappe-ui
Length of output: 9900
🏁 Script executed:
# Check the actual slot types in Popover.md
sed -n '85,110p' docs/meta/Popover.mdRepository: frappe/frappe-ui
Length of output: 808
Slot types are truncated to 100 characters in the generated docs.
The header slot type string is cut off mid-identifier (enabledModes: { label: string; v). This occurs because docs/scripts/propsgen.ts line 204 limits slot type strings to 100 characters via .slice(0, 100). The same truncation affects Popover's target, body, and body-main slots. Remove or increase the character limit in the generator to expose complete type information to docs readers.
| { | ||
| name: 'placement', | ||
| description: '', | ||
| required: false, | ||
| type: 'PopoverAlign', | ||
| default: undefined | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect ComboboxPlacement and the propsgen logic to confirm (1) type identity
# and (2) whether `@deprecated` is extracted.
fd -t f 'types.ts' src/components/Combobox | xargs rg -nC2 -e 'ComboboxPlacement|PopoverAlign|placement'
fd -t f 'propsgen.ts' scripts | xargs rg -nC3 -e 'deprecated|description|@deprecated'Repository: frappe/frappe-ui
Length of output: 50485
🏁 Script executed:
#!/bin/bash
# 1. Check the actual placement prop in Combobox.md at lines 127-133
echo "=== Combobox.md lines 120-140 ==="
sed -n '120,140p' docs/meta/Combobox.md
# 2. Find and inspect Combobox.vue component
echo -e "\n=== Combobox.vue placement prop definition ==="
find src -name "Combobox.vue" -o -name "Combobox.ts" | head -1 | xargs cat | grep -A 3 "placement"
# 3. Check propsgen.ts for `@deprecated` extraction logic
echo -e "\n=== propsgen.ts extraction logic ==="
cat docs/scripts/propsgen.ts | head -200 | tail -100Repository: frappe/frappe-ui
Length of output: 3671
🏁 Script executed:
#!/bin/bash
# Find Combobox component and check placement prop definition with full context
echo "=== Locate Combobox component ==="
find src/components -name "*ombobox*" -type f | head -5
echo -e "\n=== Check Combobox.vue for placement prop definition ==="
rg -A 5 "placement\?" src/components/Combobox/ -B 2 | head -60
echo -e "\n=== Check types.ts ComboboxPlacement alias ==="
rg "ComboboxPlacement" src/components/Combobox/types.ts -A 1 -B 1Repository: frappe/frappe-ui
Length of output: 915
🏁 Script executed:
#!/bin/bash
# Check how propsgen extracts metadata from Vue components
echo "=== Top of propsgen.ts to see imports and metadata extraction ==="
head -50 docs/scripts/propsgen.ts
echo -e "\n=== Search for vue-docgen or similar tool usage ==="
rg -i "docgen|parse.*component|extract.*meta" docs/scripts/ -l
echo -e "\n=== Check package.json for relevant tools ==="
cat package.json | grep -A 5 -B 5 "docgen\|vue-component"Repository: frappe/frappe-ui
Length of output: 2241
🏁 Script executed:
#!/bin/bash
# Check how the Combobox component actually defines its props (in the .vue file)
echo "=== Combobox.vue script section with props definition ==="
sed -n '/<script/,/<\/script>/p' src/components/Combobox/Combobox.vue | head -100
echo -e "\n=== Check if Combobox.vue imports and uses ComboboxProps interface ==="
rg "ComboboxProps|withDefaults|defineProps" src/components/Combobox/Combobox.vue -B 2 -A 2Repository: frappe/frappe-ui
Length of output: 3026
placement prop description is empty and doesn't document the deprecation.
The placement prop is documented in types.ts (line 156) as /** @deprecated alias for \align` */, but the generated documentation table shows an empty description. The vue-component-metatool used bypropsgen.tsdoesn't extract@deprecated` JSDoc tags into the description field.
Update docs/meta/Combobox.md line 128 to add:
description: '@deprecated alias for `align`'
The type PopoverAlign shown in the table is correct—ComboboxPlacement is merely a deprecated alias for PopoverAlign, so the underlying type is accurate.
| const propsData = [ | ||
| { | ||
| name: 'modelValue', | ||
| description: '', | ||
| required: false, | ||
| type: 'Record<string, any>', | ||
| default: '{}' | ||
| }, | ||
| { | ||
| name: 'docfields', | ||
| description: '', | ||
| required: false, | ||
| type: 'unknown[]', | ||
| default: '[]' | ||
| } | ||
| ] |
There was a problem hiding this comment.
Missing descriptions and vague types reduce documentation value.
Both props have empty descriptions, making it difficult for users to understand their purpose and usage. Additionally, the unknown[] type for docfields is not helpful—based on the name, it should likely have a specific interface describing field definitions.
Since this file is auto-generated, these issues stem from the source component (ListFilter.vue). The source component uses runtime props instead of TypeScript generics, preventing accurate type inference and documentation generation.
💡 Recommended fix: Update source component to use TypeScript generics
In src/components/ListFilter/ListFilter.vue, refactor to use TypeScript-based props with proper types and JSDoc descriptions:
interface DocField {
// Define the structure of a docfield
fieldname: string
fieldtype: string
label: string
// ... other properties
}
interface Props {
/** The current filter values as a key-value map */
modelValue?: Record<string, any>
/** Array of field definitions for filter options */
docfields?: DocField[]
}
const props = withDefaults(defineProps<Props>(), {
modelValue: () => ({}),
docfields: () => [],
})This will enable the generator to produce accurate types and descriptions in the auto-generated documentation.
Based on learnings: Use defineProps<Props>(), defineEmits<Emits>(), and defineSlots<Slots>() with TypeScript generics.
| { | ||
| name: 'action', | ||
| description: '', | ||
| type: '[]' |
There was a problem hiding this comment.
Add a meaningful description for the action emit.
action has an empty description at Line 67, which makes the API table ambiguous for consumers.
📝 Suggested doc fix
{
name: 'action',
- description: '',
+ description: 'Fired when the toast action is triggered.',
type: '[]'
}📝 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.
| { | |
| name: 'action', | |
| description: '', | |
| type: '[]' | |
| { | |
| name: 'action', | |
| description: 'Fired when the toast action is triggered.', | |
| type: '[]' |
| const model = useVModel(props, 'modelValue', emit, { | ||
| defaultValue: null, | ||
| passive: (props.modelValue === undefined) as false, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @vueuse/core useVModel, what are the behavioral differences between passive: true and passive: false, and is it safe to cast the passive option to a literal false when the runtime value may be true?
💡 Result:
In @vueuse/core's useVModel, the passive option controls the reactivity mechanism and return type. Behavioral differences: - passive: false (default): Returns a WritableComputedRef that directly reads from props[key] on get and emits on set. It provides tight, immediate synchronization via Vue's computed getter/setter, but does not support deep watching or cloning. - passive: true: Returns a regular Ref (local proxy ref). Uses two watch effects for bidirectional sync: one watches props[key] to update the ref (with optional deep: true for nested changes), and the other watches the ref to emit if the value differs from prop (preventing loops via isUpdating flag and nextTick). Supports extras like deep, clone, and defaultValue. passive: true enables features like deep reactivity but introduces watcher overhead and potential one-tick sync delay. Safety of casting passive to literal false when runtime value may be true: No, it is not safe. The option is strongly typed with overloads: UseVModelOptions<P[K], false> vs UseVModelOptions<P[K], true>. The return type changes (WritableComputedRef vs Ref), affecting downstream usage (e.g., data.value works, but computed-specific optimizations fail). Forcing {passive: false} when runtime passive is true breaks type safety, causes runtime mismatch (wrong sync mechanism), and leads to bugs like lost deep watching or failed emits. Use conditional logic with separate calls or type guards instead.
Citations:
- 1: https://vueuse.org/core/usevmodel/
- 2: vueuse/vueuse@f029595
- 3: https://vueuse.org/useVModel
- 4: https://github.com/vueuse/vueuse/blob/main/packages/core/useVModel/index.md
- 5: https://github.com/vueuse/vueuse/blob/main/packages/core/useVModel/index.ts
🏁 Script executed:
# First, let's look at the Combobox.vue file around lines 67-70
cat -n src/components/Combobox/Combobox.vue | sed -n '50,85p'Repository: frappe/frappe-ui
Length of output: 1241
🏁 Script executed:
# Search for modelValue prop definition in Combobox
rg -A 5 -B 5 'modelValue' src/components/Combobox/Combobox.vue | head -40Repository: frappe/frappe-ui
Length of output: 757
🏁 Script executed:
# Look for how model is used after it's defined
cat -n src/components/Combobox/Combobox.vue | sed -n '65,120p'Repository: frappe/frappe-ui
Length of output: 2099
The passive: (...) as false type cast breaks type safety with useVModel.
The expression (props.modelValue === undefined) as false evaluates to true when modelValue is undefined but the cast forces TypeScript to treat passive as literal false. This type lie causes TypeScript to pick the wrong overload: useVModel with passive: false returns WritableComputedRef, but the actual runtime value (when uncontrolled) expects the Ref type returned by passive: true. The mismatch can silently produce incorrect behavior downstream where model.value is used (e.g., in the internalModelValue computed property at line 108).
Fix: Branch at setup time instead of casting:
Branching solution
-const model = useVModel(props, 'modelValue', emit, {
- defaultValue: null,
- passive: (props.modelValue === undefined) as false,
-})
+const model =
+ props.modelValue === undefined
+ ? useVModel(props, 'modelValue', emit, { defaultValue: null, passive: true })
+ : useVModel(props, 'modelValue', emit, { defaultValue: null, passive: false })📝 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.
| const model = useVModel(props, 'modelValue', emit, { | |
| defaultValue: null, | |
| passive: (props.modelValue === undefined) as false, | |
| }) | |
| const model = | |
| props.modelValue === undefined | |
| ? useVModel(props, 'modelValue', emit, { defaultValue: null, passive: true }) | |
| : useVModel(props, 'modelValue', emit, { defaultValue: null, passive: false }) |
| function reset() { | ||
| query.value = '' | ||
| hasTypedSinceOpen.value = false | ||
| model.value = null | ||
| emit('update:selectedOption', null) | ||
| } |
There was a problem hiding this comment.
reset() clears query but doesn’t emit update:query.
The function clears query.value (line 303), yet only update:selectedOption is emitted (line 306). Consumers wiring @update:query for telemetry/derived state will miss this transition. model.value = null does emit update:modelValue via useVModel, so the asymmetry is specifically on the query channel.
🛠️ Proposed fix
function reset() {
query.value = ''
hasTypedSinceOpen.value = false
model.value = null
+ emit('update:query', '')
emit('update:selectedOption', null)
}As per coding guidelines, "Use @update:query event for searchable selection/menu components" — emitting on all query mutations keeps the contract consistent.
📝 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 reset() { | |
| query.value = '' | |
| hasTypedSinceOpen.value = false | |
| model.value = null | |
| emit('update:selectedOption', null) | |
| } | |
| function reset() { | |
| query.value = '' | |
| hasTypedSinceOpen.value = false | |
| model.value = null | |
| emit('update:query', '') | |
| emit('update:selectedOption', null) | |
| } |
| <template v-if="isButtonMode"> | ||
| <ComboboxAnchor as-child> | ||
| <ComboboxTrigger | ||
| data-slot="trigger" | ||
| :data-state="open ? 'open' : 'closed'" | ||
| :data-disabled="disabled ? '' : undefined" | ||
| :data-variant="variant" | ||
| :data-size="size" | ||
| :disabled="disabled" | ||
| :class="attrs.class" | ||
| :style="attrs.style" | ||
| :as-child="$slots.trigger ? true : undefined" | ||
| @pointerdown="markPointerDown" | ||
| > | ||
| <slot | ||
| v-if="$slots.trigger" | ||
| name="trigger" | ||
| v-bind="{ | ||
| open, | ||
| disabled: !!disabled, | ||
| query: typedQuery, | ||
| selectedOption, | ||
| displayValue, | ||
| }" | ||
| /> | ||
|
|
||
| <Button v-else :variant="variant" :size="size" :disabled="disabled"> | ||
| <!-- | ||
| Prefix priority when trigger="button": | ||
| 1. selected + #item-prefix → reuse the per-row prefix slot | ||
| 2. selected + option.icon → render the icon component | ||
| 3. no selection + #prefix → consumer's placeholder icon | ||
| --> | ||
| <template v-if="selectedOption && $slots['item-prefix']" #prefix> | ||
| <slot | ||
| name="item-prefix" | ||
| v-bind="{ | ||
| item: selectedOption, | ||
| query: '', | ||
| selected: true, | ||
| }" | ||
| /> | ||
| </template> | ||
| <template v-else-if="selectedOption?.icon" #prefix> | ||
| <component :is="selectedOption.icon" class="size-4" /> | ||
| </template> | ||
| <template v-else-if="!selectedOption && $slots.prefix" #prefix> | ||
| <slot name="prefix" /> | ||
| </template> | ||
| {{ selectedOption?.label ?? placeholder }} | ||
| </Button> | ||
| </ComboboxTrigger> | ||
| </ComboboxAnchor> | ||
| </template> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In reka-ui 1.5.3, does ComboboxTrigger render as a <button> element by default, and is as-child the recommended pattern when wrapping a custom button component inside it?
💡 Result:
Yes, in reka-ui (including version 1.5.3, as no version-specific changes to ComboboxTrigger rendering were found and the library follows Radix primitives patterns consistently across versions), ComboboxTrigger renders as a <button> element by default. The official documentation explicitly states that the as prop defaults to "button", and the source code confirms as: 'button' with handling the rendering, setting type="button" when as === 'button'. Yes, as-child (or asChild) is the recommended pattern when wrapping a custom button component inside it. The documentation describes asChild as changing the default rendered element for the one passed as a child, merging props and behavior, and links to the Composition guide. The guide explicitly recommends using asChild to compose Reka's functionality onto custom Vue components like buttons, with examples showing nesting for triggers: . This ensures accessibility and functionality while using design system components.
Citations:
- 1: https://reka-ui.com/docs/components/combobox
- 2: https://reka-ui.com/meta/ComboboxTrigger
- 3: https://reka-ui.com/docs/guides/composition
- 4: https://github.com/unovue/reka-ui/blob/49d1d32b/packages/core/src/Combobox/ComboboxTrigger.vue
🏁 Script executed:
# Find the Combobox.vue file
fd "Combobox.vue" src/components/Repository: frappe/frappe-ui
Length of output: 96
🏁 Script executed:
# Read the Combobox.vue file to verify additional concerns
# Focus on lines 69, 74, 264, 302-307, 474
wc -l src/components/Combobox/Combobox.vueRepository: frappe/frappe-ui
Length of output: 100
🏁 Script executed:
# Read specific sections of Combobox.vue to check the additional concerns
sed -n '65,75p' src/components/Combobox/Combobox.vue
echo "---"
sed -n '260,270p' src/components/Combobox/Combobox.vue
echo "---"
sed -n '300,310p' src/components/Combobox/Combobox.vue
echo "---"
sed -n '470,480p' src/components/Combobox/Combobox.vueRepository: frappe/frappe-ui
Length of output: 1181
🏁 Script executed:
# Get the full context around the button mode rendering (lines 361-414)
sed -n '361,414p' src/components/Combobox/Combobox.vueRepository: frappe/frappe-ui
Length of output: 1960
🏁 Script executed:
# Check if there are any other related trigger patterns in the file
rg "as-child" src/components/Combobox/Combobox.vue -A 2 -B 2Repository: frappe/frappe-ui
Length of output: 361
Built-in button mode produces nested <button> elements (invalid HTML + a11y).
According to reka-ui's docs, ComboboxTrigger renders as a <button> by default. In your code (line 371), the conditional :as-child="$slots.trigger ? true : undefined" means when no custom trigger slot is provided, as-child becomes undefined, so reka renders its own <button>. Then the <Button> component on line 387 renders another <button> inside it. This nesting is invalid HTML and breaks keyboard/assistive tech expectations.
Fix by always using :as-child="true" in button mode, delegating the trigger element to Button:
Proposed fix
<ComboboxTrigger
data-slot="trigger"
:data-state="open ? 'open' : 'closed'"
:data-disabled="disabled ? '' : undefined"
:data-variant="variant"
:data-size="size"
:disabled="disabled"
:class="attrs.class"
:style="attrs.style"
- :as-child="$slots.trigger ? true : undefined"
+ :as-child="true"
`@pointerdown`="markPointerDown"
>This matches the composition pattern recommended by reka-ui and used in shadcn-vue/Una UI.
📝 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.
| <template v-if="isButtonMode"> | |
| <ComboboxAnchor as-child> | |
| <ComboboxTrigger | |
| data-slot="trigger" | |
| :data-state="open ? 'open' : 'closed'" | |
| :data-disabled="disabled ? '' : undefined" | |
| :data-variant="variant" | |
| :data-size="size" | |
| :disabled="disabled" | |
| :class="attrs.class" | |
| :style="attrs.style" | |
| :as-child="$slots.trigger ? true : undefined" | |
| @pointerdown="markPointerDown" | |
| > | |
| <slot | |
| v-if="$slots.trigger" | |
| name="trigger" | |
| v-bind="{ | |
| open, | |
| disabled: !!disabled, | |
| query: typedQuery, | |
| selectedOption, | |
| displayValue, | |
| }" | |
| /> | |
| <Button v-else :variant="variant" :size="size" :disabled="disabled"> | |
| <!-- | |
| Prefix priority when trigger="button": | |
| 1. selected + #item-prefix → reuse the per-row prefix slot | |
| 2. selected + option.icon → render the icon component | |
| 3. no selection + #prefix → consumer's placeholder icon | |
| --> | |
| <template v-if="selectedOption && $slots['item-prefix']" #prefix> | |
| <slot | |
| name="item-prefix" | |
| v-bind="{ | |
| item: selectedOption, | |
| query: '', | |
| selected: true, | |
| }" | |
| /> | |
| </template> | |
| <template v-else-if="selectedOption?.icon" #prefix> | |
| <component :is="selectedOption.icon" class="size-4" /> | |
| </template> | |
| <template v-else-if="!selectedOption && $slots.prefix" #prefix> | |
| <slot name="prefix" /> | |
| </template> | |
| {{ selectedOption?.label ?? placeholder }} | |
| </Button> | |
| </ComboboxTrigger> | |
| </ComboboxAnchor> | |
| </template> | |
| <template v-if="isButtonMode"> | |
| <ComboboxAnchor as-child> | |
| <ComboboxTrigger | |
| data-slot="trigger" | |
| :data-state="open ? 'open' : 'closed'" | |
| :data-disabled="disabled ? '' : undefined" | |
| :data-variant="variant" | |
| :data-size="size" | |
| :disabled="disabled" | |
| :class="attrs.class" | |
| :style="attrs.style" | |
| :as-child="true" | |
| `@pointerdown`="markPointerDown" | |
| > | |
| <slot | |
| v-if="$slots.trigger" | |
| name="trigger" | |
| v-bind="{ | |
| open, | |
| disabled: !!disabled, | |
| query: typedQuery, | |
| selectedOption, | |
| displayValue, | |
| }" | |
| /> | |
| <Button v-else :variant="variant" :size="size" :disabled="disabled"> | |
| <!-- | |
| Prefix priority when trigger="button": | |
| 1. selected + `#item-prefix` → reuse the per-row prefix slot | |
| 2. selected + option.icon → render the icon component | |
| 3. no selection + `#prefix` → consumer's placeholder icon | |
| --> | |
| <template v-if="selectedOption && $slots['item-prefix']" `#prefix`> | |
| <slot | |
| name="item-prefix" | |
| v-bind="{ | |
| item: selectedOption, | |
| query: '', | |
| selected: true, | |
| }" | |
| /> | |
| </template> | |
| <template v-else-if="selectedOption?.icon" `#prefix`> | |
| <component :is="selectedOption.icon" class="size-4" /> | |
| </template> | |
| <template v-else-if="!selectedOption && $slots.prefix" `#prefix`> | |
| <slot name="prefix" /> | |
| </template> | |
| {{ selectedOption?.label ?? placeholder }} | |
| </Button> | |
| </ComboboxTrigger> | |
| </ComboboxAnchor> | |
| </template> |
| > | ||
| <!-- | ||
| Full-row takeover precedence: | ||
| 1. template #item slot | ||
| 2. item.slots.item (or legacy function-form item.render) | ||
| 3. legacy per-item direct slot (#<slotName>) | ||
| 4. default row shell | ||
| --> | ||
| <component | ||
| :is="ItemSlotRender" | ||
| v-if="slotFns.item && !shouldUseDynamicItemSlot(item)" | ||
| :render="slotFns.item" | ||
| :slot-props="getItemSlotProps(item)" | ||
| /> | ||
|
|
||
| <component | ||
| :is="ItemSlotRender" | ||
| v-else-if="item.resolvedSlots.item" | ||
| :render="item.resolvedSlots.item" | ||
| :slot-props="getItemSlotProps(item)" | ||
| /> | ||
|
|
||
| <component | ||
| :is="ItemSlotRender" | ||
| v-else-if="shouldUseLegacyDirectSlot(item)" | ||
| :render="slotFns[getLegacyDirectSlotName(item)!]" | ||
| :slot-props="getLegacySlotProps(item)" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether any test asserts the intended precedence between `#item` and `#item-`<slot>.
rg -nP -C3 "#item\b|item-<slot>|shouldUseDynamicItemSlot" --type=ts --type=vueRepository: frappe/frappe-ui
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Search for shouldUseDynamicItemSlot implementation and related tests
rg "shouldUseDynamicItemSlot" -C5Repository: frappe/frappe-ui
Length of output: 3202
🏁 Script executed:
#!/bin/bash
# Look for the spec file and check precedence documentation
find . -name "*combobox*spec*" -o -name "*08*selection*"Repository: frappe/frappe-ui
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Search for tests related to item slot precedence
rg "\.item|item-prefix|item-label|item-suffix" -t ts -C3 | head -100Repository: frappe/frappe-ui
Length of output: 6889
🏁 Script executed:
#!/bin/bash
# Check the spec for precedence rules
cat v1-release/08d-combobox-spec.md | head -500Repository: frappe/frappe-ui
Length of output: 16911
🏁 Script executed:
#!/bin/bash
# Look specifically for precedence and slot override rules in the spec
rg "precedence|#item|item-" v1-release/08d-combobox-spec.md -B2 -A2 | head -150Repository: frappe/frappe-ui
Length of output: 3986
🏁 Script executed:
#!/bin/bash
# Check if there are tests for the precedence behavior
rg "slots.item|item-label|shouldUseDynamic" src/components/Combobox/Combobox.cy.ts -B5 -A10Repository: frappe/frappe-ui
Length of output: 1041
Precedence comment doesn't match code or spec.
The inline comment lists template #item slot as unconditional precedence #1, but line 233's guard slotFns.item && !shouldUseDynamicItemSlot(item) skips the global #item when the item has a matching #item-<slot> template. The spec (v1-release/08d-combobox-spec.md) documents #item as full-row precedence #1 with no exception, and separately documents #item-<slot> as overriding the label region only—not the full row.
Either:
- The behavior is intentional (per-item slots beat global full-row slots): update the spec to document this exception and clarify the comment.
- The code should follow the spec: remove the
!shouldUseDynamicItemSlot(item)clause so#itemwins when present.
- trigger=button: pass `as-child` on ComboboxTrigger unconditionally in button mode; previously the built-in <Button> (and non-slot branch) nested inside ComboboxTrigger's default <button>, producing invalid HTML and breaking a11y. - reset(): also emit `update:query` with '' so `@update:query` consumers see the clear transition alongside `update:modelValue`. - useVModel: drop the `passive: (...) as false` type cast; pass `passive: true` unconditionally so the returned model is a plain Ref regardless of controlled/uncontrolled usage. Leave a comment explaining the trade-off. - ComboboxResults: rewrite the full-row precedence comment so it matches the code — per-item `#item-<slot>` dispatch wins over the generic `#item` slot. - types.ts: give `placement` a proper JSDoc summary so the generated meta table documents it instead of leaving the description empty. Test coverage: add `trigger="button"` nested-button guard assertion and an `update:query` assertion on the reset() test. 37/37 passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed CodeRabbit review in In-scope fixes applied:
Not addressed (out of scope for this PR):
The three deferred comments (2, 4, 5) all touch components outside this PR's scope — they surfaced because I regenerated Tests: 37/37 pass (+1 nested-button guard, +1 |
Using `<Button>` inside `<ComboboxTrigger as-child>` broke the reka composition chain: reka's trigger attrs (tabindex, aria-expanded, aria-controls) were routed to Button's root, which is Tooltip with `inheritAttrs: false` — the attrs never reached the native <button>, so the element was unreachable via Tab. Drop the `<Button>` wrapper and render the trigger content directly inside `<ComboboxTrigger>`, which already emits a native <button> with all the right semantics. Styling comes from the existing triggerClasses (same visual as input mode), with the chevron rotating via data-state. Added a focus + Enter-to-open test so this regression can't recur silently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
**Combobox**: replace `useVModel(..., { passive: (x) as false })` with
`defineModel<string | null>({ default: null })`. Removes the type cast,
drops the `@vueuse/core` import, and aligns with the pattern already
used in Dropdown. `modelValue` + `update:modelValue` come from the
compiler now, so they're removed from `ComboboxProps` / `ComboboxEmits`
(still public API for consumers via `v-model`, just not manually
declared).
**Button**: move reka Tooltip primitives inline so `<button>` is the
effective DOM root. Previously `<Tooltip>` (a Vue component with
`inheritAttrs: false`) wrapped the button and swallowed any attrs
forwarded through it — notably `tabindex`/`aria-expanded` from
`ComboboxTrigger as-child`, which made Button unreachable via Tab when
used as a combobox trigger. TooltipProvider/Root/Trigger are renderless,
so the final DOM is just the native `<button>` with all forwarded attrs
preserved. Tooltip behavior unchanged (same styling, same arrow, same
delay defaults; only rendered when `tooltip?.length`).
Tests: 50/50 pass across Button (8), Combobox (38), Tooltip (4).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both `<Tooltip>` (the wrapper component) and `<Button>` (which inlines reka Tooltip primitives to keep `<button>` as its DOM root) duplicated the same styled popover + arrow markup. Extract it into a single `TooltipBubble` component that owns the bubble styling, arrow, and content-slot composition — both callers now render the tooltip popover through it. Tooltip.vue keeps its existing public API (`text`, `placement`, `arrowClass`, `#content`, `#body` slots); TooltipBubble accepts the same shape so the arrow-class customization still works. Button uses the default arrow styling. Tests: 50/50 pass across Button (8), Tooltip (4), Combobox (38). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
reka-ui's `ComboboxTrigger` hardcodes `tabindex=\"-1\"` because its reference combobox has a separate `ComboboxInput` carrying the tab stop and the trigger is just a chevron affordance. That assumption breaks button mode, where the search input lives inside the popover and there is no visible input at rest — so the trigger ends up outside the tab order and Tab skips right past the component. Swap to `ComboboxAnchor` (positioning only, no tabindex meddling) with a plain native `<button>` as its child. Click + pointerdown are attached on the anchor so they forward via `as-child` — consumer `#trigger` elements now "just work" without wiring any handlers. ARIA (`aria-haspopup`, `aria-expanded`) is set directly on the built-in button; consumer triggers manage their own. Added a regression test that asserts the rendered trigger is a native <button> with a non-negative tabindex, so if anything re-routes through ComboboxTrigger it'll fail loudly instead of silently losing Tab focus. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
**Group label color**: unify `text-ink-gray-4` across Combobox, Dropdown, and ItemList. Previously Dropdown used `text-ink-gray-7` via `getDropdownTextColor(group)` and ItemList used `text-ink-gray-7` directly. Combobox was already updated. Gray-4 reads as a section header — quieter than row labels — which is what group titles should be. Removed the now-unused `getDropdownTextColor` call path for group labels in DropdownMenuList (it's still used for item rows). **KebabMenu story**: new `06_KebabMenu.vue` showcasing the classic row-actions pattern — a ghost icon button opens a grouped menu per task row. Uses `#trigger` slot with `LucideMoreHorizontal` and flips the button to `active` while the menu is open via the `open` slot prop. **EmojiPicker story**: small tweak from the user's local edits — emoji prefix now uses fixed `size-4` with flex centering so the glyph aligns visually with the surrounding row height. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Match Combobox's style — one demo per file, concrete context, Lucide icon components. Replaces the previous multi-demo-per-file layout that made the stacked code previews noisy. New lineup: 1. Simple — basic actions menu (Edit / Duplicate / Delete), default Button trigger. Stacked layout so the full source is visible next to the rendered menu. 2. Shortcuts — menu items with keyboard-shortcut `<kbd>` suffixes rendered through `#item-suffix`. Stacked layout. 3. Submenus — grouped "Manage / Danger" actions with nested Share → Invite people → email/slack submenus. 4. Switches — toggle items in a Preferences menu (Notifications, Read receipts, Focus mode, Auto-save). Demonstrates `switch: true` with reactive `switchValue` via a computed options array. 5. Kebab Menu — row-actions pattern, already added. 6. User Menu — workspace / profile menu, moved from 04. Removed: 01_Basic, 02_MenuPatterns, 03_TriggerPatterns, 05_ItemSlots- AndEmptyState — their ideas are covered across the new set without duplication. Trigger customization is demonstrated by Kebab Menu and User Menu; item slots and empty state are features any story can show when needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Registers a Tailwind utility per lucide-static icon (~1800 of them): `lucide-menu`, `lucide-check`, `lucide-trash-2`, etc. Each class renders as a 1em inline-block that masks the icon SVG with the current text color, so it participates in `size-*`, `text-*`, and layout classes without any template-side import: <span class="lucide-menu size-4 text-ink-gray-6" /> How it works: - Plugin reads lucide-static/icons/*.svg at build time. - For each `lucide-<name>` reference found in source (JIT), emits a rule with `mask-image: url(data:image/svg+xml;utf8,<svg>)` plus `background-color: currentColor` — currentColor flows through so the icon inherits parent text color. - SVGs are inlined as data URIs; no network request, no runtime import, no FS lookup on the client. Cache keyed by name. - Tailwind's JIT scans sources, so only classes that actually appear in templates make it into the output CSS. Registered via `tailwind/preset.js`, which frappe-ui consumers already inherit. This unlocks a path to drop per-component Lucide imports in Button / Dropdown / etc. — `icon: 'more-horizontal'` can route to a `<span class="lucide-more-horizontal">` instead of FeatherIcon. That migration isn't in this commit; the plugin is the foundation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ugin
`{ label: 'Edit', icon: 'lucide-edit' }` now "just works" — the string
is passed through as a class name onto a styled <span>. Consumers no
longer need to import each Lucide icon component in their dropdown
definitions:
// Before
import LucidePen from '~icons/lucide/pen'
const actions = [{ label: 'Edit', icon: LucidePen, onClick }]
// After
const actions = [{ label: 'Edit', icon: 'lucide-pen', onClick }]
Rules inside DropdownMenuItemContent:
1. `item.icon` starts with `lucide-` → <span :class="[item.icon, …]">.
Tailwind plugin provides the CSS; scanner picks up the literal
string in source so the class is emitted.
2. Other string → FeatherIcon (back-compat with existing
`icon: 'edit'` / `icon: 'copy'` call sites).
3. Component value → rendered as-is (unchanged).
Stories 01–06 migrated to the string form. Only LucideMoreHorizontal
stays imported in KebabMenu — it's passed to `Button`'s `icon` prop,
and Button hasn't adopted the string path yet (separate migration).
UserMenu drops three Lucide component imports; the two template-slot
icons (chevron-down on the trigger, check on item-suffix) become
`<span class="lucide-chevron-down …" />` / `<span class="lucide-check
…" />` inline.
Stories annotated with `DropdownOptions` type where needed so TS
doesn't widen `theme: 'red'` to `string`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
**Plugin**: override stroke-width to 1.5 on every inlined SVG. Lucide's default stroke-width=2 reads heavy next to the rest of the design system's iconography — 1.5 balances visual density better. Applied at encode time so the change is transparent to consumers. **Selection components** now auto-render `item.icon` in the prefix region when no consumer slot (`#item-prefix` or `item.slots.prefix`) claims it. Precedence added per component: - ComboboxResults: `#item-prefix` → `item.slots.prefix` → item.icon (lucide string or Component) → empty - Select: `#item-prefix` → `option.slots.prefix` → option.icon → empty - ItemList: `#item-prefix` → item.icon → empty Each component uses the same `isLucideIconString` guard — strings that start with `lucide-` route through the Tailwind plugin; Component values render as `<component :is>`; anything else is ignored so existing consumers using custom prefix slots are unaffected. **Spec**: - new shared design rule 11 in 08-selection-and-menu-api-spec.md documenting the `lucide-*` convention, JIT-scanner caveat, and the Component-vs-string fallback semantics. Deprecation policy renumbered to 12. - 08a-itemlist, 08c-select, 08d-combobox updated to reflect auto-rendering in the prefix region. Tests: 69/69 pass (Combobox 38, Select 15, Dropdown 6, ItemList 6, ItemListRow 4). Docs build clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switched from `matchUtilities` to `matchComponents`. `size-4` / `w-*` / `h-*` are Tailwind utilities and live in the utilities layer, which is emitted after the components layer — so they always win the cascade race against the plugin's `width: 1em; height: 1em` defaults without requiring `!important` or source-order tricks at the call site. Verified by byte-offset on the built CSS: `.lucide-edit` lives around byte 65k (components layer); `.size-4` lives around byte 105k (utilities layer). Same specificity, later rule wins — `size-4` overrides. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When an icon prop is a `lucide-*` string, render a <span> with that class via the Tailwind plugin instead of routing through FeatherIcon. Uses size-* classes for square dimensions at each button size. Also removes ~icons/lucide/* imports from Combobox, Select, and Dropdown story examples, replacing component usage with lucide-* class spans. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
netchampfaris
force-pushed
the
v1-release/combobox
branch
from
a187a44 to
e2e5ed6
Compare
Mutating a ref inside a computed is a Vue antipattern — computeds should be pure. Moved the dev-only warning into a watchEffect guarded by import.meta.env.DEV. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the hand-rolled <button> in button mode with the shared Button component so trigger styling (variants, sizes, focus rings, disabled states) stays consistent with the rest of the UI. Also drops the active-scale micro-interaction on both Combobox and Select triggers. Button mode no longer renders #item-prefix inside the trigger for the selected option — consumers who want richer trigger visuals should use the #trigger slot. Selected-option icons still render via iconLeft. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Standardize item-row backgrounds across Combobox, Select, Dropdown, MultiSelect, ItemList, and CommandPalette: - Hover/highlighted: surface-gray-2 - Selected/checked: surface-gray-3 Previously Combobox/Select used gray-2 for both, Dropdown/MultiSelect used gray-3 for highlighted, and CommandPalette used gray-3 for its active row — inconsistent signalling across the selection surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
In button mode the search input lives in the popover, separate from the trigger. Previously the query was kept in sync with the selected label and clearing the input cleared the selection — both correct for input mode but wrong here. - `displayValue` watcher no longer writes to `query` in button mode - Opening in button mode resets `query` to ''; closing resets it too - Empty-input clearSelection only runs in input mode Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reka's item-aligned positioning anchors content to SelectItemText's bounding rect. SelectItemText was rendered `sr-only` (1x1px at the SelectItem's corner), so the popup's left edge landed somewhere after the trigger's prefix — prefix icon showed through. - Remove the `sr-only` SelectItemText copy. - Wrap the visible label with SelectItemText (as=div) in every slot path (dynamic `#item-<slot>`, `#item-label`, `#option`, default) so reka reads the rect of the on-screen label. - Override SelectValue's slot in the trigger with `selectedOption.label` so the trigger text stays clean — otherwise textContent would pick up description / sibling content inside SelectItemText. - Simplify `itemSize` and drop unused `ref` import. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Every lucide-* utility now emits `color: var(--ink-gray-6)` alongside the mask-image and `background-color: currentColor`. Consumer `text-*` tints still win because utilities override the components layer where lucide-* lives, so call sites that previously paired `lucide-foo` with `text-ink-gray-6` can now drop the explicit color class. Also updates 01_Simple story to show a lucide-* icon on the built-in button trigger. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
netchampfaris
assigned
netchampfaris and
safwansamsudeen and unassigned
netchampfaris and
safwansamsudeen
Re-implement MultiSelect against the v1 selection-and-menu API spec: - Share the Combobox / Select option schema (label, value, icon, description, disabled, slot, slots) plus grouped options. - Button-mode trigger with popover positioning props (side, align, offset, portalTo); in-popover search input, internal filtering, and update:query. - Default Clear All / Select All footer, customizable via #footer. - Item slot system: #item-prefix / #item-label / #item-suffix, per-row takeover via #item, dynamic #item-<slot> matching option.slot, #empty, #group-label, and a #trigger escape hatch. - Trigger width is pinned via a phantom sizer pseudo-element. Shows the label when exactly one option is selected (reusing #item-prefix or auto-rendering option.icon), "N selected" for 2+, placeholder for 0. - Stories: Example, Options (item-prefix), Grouped, Footer, TriggerSlot, TagsTrigger (chips-style #trigger with removable Badges). Docs updated. Also adds a shared src/utils/iconString.ts with isLucideIconString / isEmojiIconString helpers, used by MultiSelect to auto-render emoji icons (and reused by Combobox / Select / Dropdown in follow-up commits). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ment
- The trigger now auto-renders the selected option's prefix. If the
consumer defines #item-prefix, reuse it on the trigger with
{ item: selectedOption, query: '', selected: true } so the selected
row and the trigger render identically without a second slot. Falls
back to auto-rendering option.icon (lucide / emoji / component), then
to the existing #prefix slot for the placeholder state.
- Emoji strings in item.icon (e.g. '🚀') are now detected via
isEmojiIconString and rendered as text. Legacy feather names like
'copy' still route to FeatherIcon. Keeps every item and the trigger
icon path consistent.
- Button-mode trigger is now a raw <button> with triggerClasses. Using
<Button> wrapped the label in its own default-slot <span>, which is
content-sized and centered the label when a width class was applied.
Direct flex children (prefix, label flex-1, chevron) now align to
justify-between correctly.
- Combined hover+selected item background moves to surface-gray-4 (on
top of gray-2 hover / gray-3 selected). Scoped CSS in ComboboxResults
clears the inner ItemListRow bg so the outer combined state paints
through.
- EmojiPicker story now uses option.icon = '🚀' and drops the manual
#item-prefix — the auto-render handles both the list and the trigger.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- The trigger auto-renders the selected option's prefix. If the consumer defines #item-prefix, reuse it on the trigger so the selected row and the trigger render identically without a second slot. Falls back to auto-rendering option.icon (lucide / emoji / component), then to the existing #prefix slot for the placeholder state. - Emoji strings in option.icon (e.g. '🚀') render as text via isEmojiIconString. Legacy feather names still route to FeatherIcon. - Combined hover+selected item background moves to surface-gray-4 (on top of gray-2 hover / gray-3 selected). Scoped CSS clears the inner ItemListRow bg so the outer combined state paints through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Items with item.selected now surface data-state="checked" on the outer DropdownMenuItem, which picks up surface-gray-3 for selected and surface-gray-4 for the combined hover+selected state. Scoped CSS in DropdownMenuItemContent clears the inner ItemListRow bg so the outer combined state paints through. - Emoji strings in item.icon (e.g. '🚀') render as text via isEmojiIconString. Legacy feather names still route to FeatherIcon. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The story demonstrates `#item-prefix` + `#item-label` (the preferred v1 slots), not the deprecated `#option` alias. The old heading was misleading — readers could infer they should reach for `#option` first. Reword and add a short description explaining what the example actually shows. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
propsgen now reads the `tags` field from vue-component-meta output and
emits a `deprecated` entry (string for replacement guidance, or `true`
when the JSDoc tag has no message) for every prop, slot, and emit that
carries `@deprecated`.
PropsTable, SlotsTable, and EmitsTable accept the optional `deprecated`
field. The row name renders with a line-through plus an amber
"deprecated" badge, and the description area appends the migration
hint ("Deprecated — use X instead").
Note: vue-component-meta does not currently preserve JSDoc tags for
interface-based emits; emit deprecations (e.g. Combobox's `input`)
won't surface through this path until the upstream limitation is
fixed. Props and slots work today.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…onent
Bring Dropdown up to the shared selection-and-menu v1 spec.
- `item.slots` — new inline slot object on each option with optional
prefix / label / suffix / item functions. Replaces `item.component`
for full-row takeovers and fills the JS-authored option path that
template slots can't reach. Shares the mental model with the
equivalent path on Combobox / Select / MultiSelect.
- Full-row takeover via `item.slots.item` registered as a new branch
in DropdownMenuList (before the legacy `item.component` branch).
- Per-region content via `item.slots.{prefix,label,suffix}` added
as a new fallback layer in DropdownMenuItemContent — template
slots (`#item-prefix` etc.) still win over the inline slots.
- `item.component` is now documented as `@deprecated`; it keeps
working through v1.x.
- New `align` prop (`'start' | 'center' | 'end'`) following the shared
popover positioning vocabulary. `placement` remains as a
`@deprecated` back-compat alias that maps to `align` when `align` is
not provided.
- The Dropdown content's `origin-top-*` classes now derive from the
resolved `align`, not from the raw `placement`, so consumers that
pass `align` directly get the correct transform origin.
- `KebabMenu` story switched from `placement="right"` to `align="end"`.
Meta docs regenerated — both Dropdown and the other selection components
pick up the new @deprecated rendering added in the previous commit.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two follow-ups from the v1 selection-spec audit: - New `emptyText` prop (default `'No options'`) replaces the previously hard-coded fallback so consumers can localize or rephrase the empty-state copy without using `#empty`. - `#option` slot marked `@deprecated` in favor of `#item-label`. It still works through v1.x — the spec keeps it as a compat alias. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace amber badge + opacity with strikethrough name; hide type when deprecated and show only the deprecation message, flush with no leading whitespace. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
08d-combobox-spec.md— new per-itemslotsAPI ({ prefix, label, suffix, item }), intrinsic-width trigger, press feedback, chevron rotation. Deprecations with back-compat aliases:render,slotName,inputevent,placement.trigger: 'input' | 'button'prop): renders a built-inButton, moves the search input into the popover header and auto-focuses it on open. Label + prefix auto-wire from the selected option (reuses#item-prefix, falls back toselectedOption.icon, then#prefixas placeholder).#triggerslot remains for fully custom triggers.usePopoverMotioncomposable — Combobox, Select, and Dropdown classify opens as pointer-driven or keyboard-driven via pointer-recency (@pointerdowntimestamp within 300ms of the open transition). Keyboard opens skip the scale entrance and run an 80ms opacity-only fade to mask reka's 1-frame position-settle; pointer opens play the full animation.<ComponentPreview layout="stacked">shows preview + code together; the markdown plugin now forwards arbitrary props instead of whitelisting.08a–08e); new shared rule 10 "Popover motion conventions".Test plan
yarn docs:buildsucceedstrigger="button"reuses#item-prefixfor the selected-state button prefix and#prefixas the placeholder icon.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests