fix(command): add highlight-on-hover to ListboxRoot#1722
fix(command): add highlight-on-hover to ListboxRoot#1722zxzinn wants to merge 1 commit intounovue:devfrom
Conversation
📝 WalkthroughWalkthroughTwo command-related component files receive minor updates: the addition of a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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 |
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 `@apps/v4/registry/new-york-v4/ui/command/Command.vue`:
- Around line 82-83: The template currently forces highlight-on-hover by placing
highlight-on-hover after v-bind="forwarded", which overrides any caller-supplied
prop; instead remove the static highlight-on-hover attribute from the template
and set its default in the component props (use withDefaults when declaring
props) so highlightOnHover defaults to true but can be overridden by callers;
update the prop name/shape that composes ListboxRootProps (the forwarded object)
and ensure withDefaults includes { highlightOnHover: true }, and make the
identical change in the companion command.json snippet so the default is
provided at the prop/schema level rather than in the template.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b92c7fb4-5766-44e0-b5f5-14160e781eb5
📒 Files selected for processing (2)
apps/v4/public/r/styles/new-york-v4/command.jsonapps/v4/registry/new-york-v4/ui/command/Command.vue
| v-bind="forwarded" | ||
| highlight-on-hover |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vue 3 templates, if a component has v-bind="obj" and the same prop is also declared statically after it, which value takes precedence?
💡 Result:
In Vue 3, v-bind="obj" is order-sensitive: when the same prop/attribute is declared twice, the one that appears later in the template wins. So if you write:
<MyComp v-bind="obj" foo="static" />the static foo="static" takes precedence over obj.foo. If you reverse the order, then obj.foo would override the static one. [1]
🏁 Script executed:
cd /repo && find . -name "Command.vue" -path "*/command/*" 2>/dev/null | head -5Repository: unovue/shadcn-vue
Length of output: 116
🏁 Script executed:
find . -name "Command.vue" -path "*/command/*" 2>/dev/null | head -5Repository: unovue/shadcn-vue
Length of output: 237
🏁 Script executed:
sed -n '75,90p' ./apps/v4/registry/new-york-v4/ui/command/Command.vueRepository: unovue/shadcn-vue
Length of output: 353
🏁 Script executed:
sed -n '1,80p' ./apps/v4/registry/new-york-v4/ui/command/Command.vueRepository: unovue/shadcn-vue
Length of output: 2207
🏁 Script executed:
find . -name "command.json" -path "*styles*" 2>/dev/nullRepository: unovue/shadcn-vue
Length of output: 385
🏁 Script executed:
cat ./apps/v4/public/r/styles/new-york-v4/command.jsonRepository: unovue/shadcn-vue
Length of output: 13887
Move highlightOnHover default to the component props instead of forcing it in the template.
When a prop is declared both via v-bind="forwarded" and as a static attribute in the template, Vue 3 applies the one that appears later. Because highlight-on-hover comes after v-bind="forwarded" (which spreads all ListboxRootProps), it forces the prop to true and prevents callers from disabling it with :highlight-on-hover="false". This breaks the component's public API contract.
Set the default in withDefaults() instead so the new behavior remains opt-out, and apply the same fix to the code snippet in apps/v4/public/r/styles/new-york-v4/command.json.
♻️ Proposed fix
const props = withDefaults(defineProps<ListboxRootProps & { class?: HTMLAttributes["class"] }>(), {
modelValue: "",
+ highlightOnHover: true,
}) <ListboxRoot
data-slot="command"
v-bind="forwarded"
- highlight-on-hover
:class="cn('bg-popover text-popover-foreground flex h-full w-full flex-col overflow-hidden rounded-md', props.class)"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/v4/registry/new-york-v4/ui/command/Command.vue` around lines 82 - 83,
The template currently forces highlight-on-hover by placing highlight-on-hover
after v-bind="forwarded", which overrides any caller-supplied prop; instead
remove the static highlight-on-hover attribute from the template and set its
default in the component props (use withDefaults when declaring props) so
highlightOnHover defaults to true but can be overridden by callers; update the
prop name/shape that composes ListboxRootProps (the forwarded object) and ensure
withDefaults includes { highlightOnHover: true }, and make the identical change
in the companion command.json snippet so the default is provided at the
prop/schema level rather than in the template.
🔗 Linked issue
N/A - Minor fix discovered during usage
❓ Type of change
📚 Description
Command items don't highlight on mouse hover. Only keyboard navigation triggers the highlight state.
Added
highlight-on-hoverprop toListboxRootinCommand.vue.📸 Screenshots (if appropriate)
N/A
📝 Checklist
Summary by CodeRabbit
Bug Fixes
New Features