feat(Select): add 'id' prop to SelectRoot and BubbleSelect components#2569
feat(Select): add 'id' prop to SelectRoot and BubbleSelect components#2569rastuhacode wants to merge 4 commits intounovue:v2from
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an optional public Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/content/meta/SelectRoot.md (1)
110-122:⚠️ Potential issue | 🟡 MinorDocumentation inconsistency:
idprop missing from markdown table.The
idprop was added to the PropsTable component (lines 70-76) but is missing from the markdown props table in thellm-onlysection. Both documentation formats should include all props for consistency.Add the following row after line 122:
| `id` | Id of the element | `string` | No | - |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/meta/SelectRoot.md` around lines 110 - 122, Add the missing `id` prop row to the markdown props table so it matches the PropsTable component: insert a table row for the `id` prop (label "Id of the element", type `string`, required No, default `-`) into the existing props table in SelectRoot.md (so the llm-only markdown and the PropsTable component are consistent); locate the props table near the `autocomplete`/`by` entries and add the `id` row in the same format as the other rows.
🤖 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/meta/SelectRoot.md`:
- Around line 70-76: Update the docs table to include the new "id" prop entry so
the markdown props table matches the PropsTable data; then fix SelectRoot.vue by
exposing id from props (either add id to the toRefs destructure e.g. include
`id` alongside `required, disabled, multiple, dir: propDir` or reference
`props.id` in the template) so the template binding `:id="id"` on the
BubbleSelect has a defined value and avoids the runtime undefined reference.
---
Outside diff comments:
In `@docs/content/meta/SelectRoot.md`:
- Around line 110-122: Add the missing `id` prop row to the markdown props table
so it matches the PropsTable component: insert a table row for the `id` prop
(label "Id of the element", type `string`, required No, default `-`) into the
existing props table in SelectRoot.md (so the llm-only markdown and the
PropsTable component are consistent); locate the props table near the
`autocomplete`/`by` entries and add the `id` row in the same format as the other
rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49f3cb68-7395-47a8-8835-2214b18a5ff0
📒 Files selected for processing (1)
docs/content/meta/SelectRoot.md
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 `@packages/core/src/Select/SelectRoot.vue`:
- Around line 206-207: The rendering condition for the native select
(BubbleSelect) in SelectRoot.vue currently requires name (v-if="isFormControl &&
name"), which prevents rendering when only id is provided; update the condition
to allow rendering when id exists (e.g., v-if="isFormControl && (name || id)")
so the native <select> is created and the id prop has a DOM target, and verify
the :id="id" binding on BubbleSelect remains intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f131e6e-e589-4ed5-943b-4882d0ce2a35
📒 Files selected for processing (2)
docs/content/meta/SelectRoot.mdpackages/core/src/Select/SelectRoot.vue
✅ Files skipped from review due to trivial changes (1)
- docs/content/meta/SelectRoot.md
1 participant
🔗 Linked issue
#2548
❓ Type of change
📚 Description
Resolves #2548
When user passes
idprop toSelectRootit is considered attribute and being dropped out and unused. It causes accessibility browser issues when being used for ex. inside form withlabel forconstruction.idprop support forSelectRootandBubbleSelectcomponentsSelectForm.story.vueas it had the same accessibility issue in inspector📸 Screenshots (if appropriate)
Issue in story
📝 Checklist
Summary by CodeRabbit
New Features
Documentation