feat(CheckboxGroup/RadioGroup): add icon field in items#6726
feat(CheckboxGroup/RadioGroup): add icon field in items#6726solidusite wants to merge 7 commits into
icon field in items#6726Conversation
- Introduced `leadingIcon` prop in Checkbox and RadioGroup components to display an icon next to the label. - Updated CheckboxGroup and RadioGroup to handle the new `icon` property in items. - Enhanced styles in themes to accommodate leading icons. - Added tests to verify rendering of leading icons in CheckboxGroup and RadioGroup.
- Updated documentation to reflect the new `icon` property and its usage in examples. - Modified playground examples to demonstrate the new icon functionality in Checkbox and RadioGroup components.
📝 WalkthroughWalkthroughCheckbox and radio components now support leading icons. Checkbox and radio group items accept optional icon values, which are passed to or rendered by the underlying controls. Theme slots and size variants define icon styling. Documentation, playgrounds, and render matrices demonstrate the new APIs, including icon-based card grids. Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/runtime/components/RadioGroup.vue (1)
18-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider typing
iconasIconProps['name']for consistency with Checkbox.Checkbox.vue types
leadingIconasIconProps['name'](string | any), allowing both Iconify string names and component references. RadioGroupItem'sicon?: stringis more restrictive, preventing component-based icons thatUIconotherwise supports via<component :is="name" />. Aligning the type would ensure cross-component consistency and unlock the fullUIconcontract.Based on learnings,
data-slotattributes should only be added to elements with corresponding theme-slot class bindings — theleadingIconslot at line 222 satisfies this.♻️ Proposed type alignment
export type RadioGroupItem = RadioGroupValue | { label?: string description?: string disabled?: boolean value?: RadioGroupValue /** * The icon displayed next to the label. * `@IconifyIcon` */ - icon?: string + icon?: IconProps['name'] class?: any ui?: Pick<RadioGroup['slots'], 'item' | 'container' | 'base' | 'indicator' | 'wrapper' | 'label' | 'leadingIcon' | 'description'> [key: string]: any }This would also require adding
import type { IconProps } from './Icon.vue'to the type imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/components/RadioGroup.vue` around lines 18 - 24, Update the RadioGroupItem `icon` prop type to use `IconProps['name']` instead of `string`, adding the corresponding type-only import from `Icon.vue` and preserving the existing UI contract.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/components/CheckboxGroupIcon.a4cec7.spec.ts`:
- Around line 8-12: In test/components/CheckboxGroupIcon.a4cec7.spec.ts lines
8-12, define a non-optional firstItem from items[0] and replace all listed
items[0] usages with firstItem. Apply the same change in
test/components/RadioGroupIcon.781254.spec.ts lines 7-11, replacing all listed
indexed usages so mountSuspended receives a CheckboxGroupItem or RadioGroupItem
rather than an undefined-capable value.
---
Nitpick comments:
In `@src/runtime/components/RadioGroup.vue`:
- Around line 18-24: Update the RadioGroupItem `icon` prop type to use
`IconProps['name']` instead of `string`, adding the corresponding type-only
import from `Icon.vue` and preserving the existing UI contract.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de8b0df1-0fd7-4ad1-82a6-b4a22664615e
⛔ Files ignored due to path filters (4)
test/components/__snapshots__/Checkbox-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/CheckboxGroup-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/RadioGroup-vue.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/Table-vue.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
docs/content/docs/2.components/checkbox-group.mddocs/content/docs/2.components/checkbox.mddocs/content/docs/2.components/radio-group.mdplaygrounds/nuxt/app/pages/components/checkbox-group.vueplaygrounds/nuxt/app/pages/components/checkbox.vueplaygrounds/nuxt/app/pages/components/radio-group.vuesrc/runtime/components/Checkbox.vuesrc/runtime/components/CheckboxGroup.vuesrc/runtime/components/RadioGroup.vuesrc/theme/checkbox.tssrc/theme/radio-group.tstest/components/CheckboxGroupIcon.a4cec7.spec.tstest/components/RadioGroupIcon.781254.spec.ts
- Deleted test files for CheckboxGroup and RadioGroup components that were previously verifying leading icon functionality. - This cleanup is part of a broader effort to streamline test coverage and focus on essential components.
- Changed the type of the `icon` property in CheckboxGroup and RadioGroup components from `string` to `IconProps['name']` for better type safety. - Updated tests to include scenarios for rendering items with icons in both CheckboxGroup and RadioGroup components. - Added snapshot tests to verify correct rendering of icons in the respective components.
… and Table components - Adjusted snapshots to reflect changes in label rendering, ensuring labels now include conditional rendering comments. - Updated multiple test files to maintain consistency across Checkbox, CheckboxGroup, RadioGroup, and Table components.
commit: |
The per-item `icon` was spread onto the underlying `UCheckbox`, so a checked item rendered the item icon in place of its check indicator. `useForwardProps` strips `undefined`, so the group-level `icon` did not shadow it. Omit `icon` from the forwarded props and bind it only to `leading-icon`. - Type item `icon` as `IconProps['name']` - Add a regression test asserting an item `icon` maps to `leadingIcon` and leaves the checkbox `icon` untouched
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/components/CheckboxGroup.spec.ts (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
VueWrapperas a type to satisfy@typescript-eslint/consistent-type-imports.
VueWrapperis only used in a type cast (as unknown as VueWrapper<any>), so ESLint flags it as a value import used only as a type.♻️ Proposed fix
-import { flushPromises, mount, VueWrapper } from '`@vue/test-utils`' +import { flushPromises, mount, type VueWrapper } from '`@vue/test-utils`'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/CheckboxGroup.spec.ts` around lines 3 - 5, Update the imports in CheckboxGroup.spec.ts so VueWrapper is imported as a type, while keeping flushPromises, mount, and Checkbox as regular imports.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/components/CheckboxGroup.spec.ts`:
- Around line 3-5: Update the imports in CheckboxGroup.spec.ts so VueWrapper is
imported as a type, while keeping flushPromises, mount, and Checkbox as regular
imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06f4f3f2-9adc-4ba0-9f74-ea929976c460
⛔ Files ignored due to path filters (4)
test/components/__snapshots__/Checkbox.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/CheckboxGroup.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/RadioGroup.spec.ts.snapis excluded by!**/*.snaptest/components/__snapshots__/Table.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/runtime/components/CheckboxGroup.vuetest/components/CheckboxGroup.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/runtime/components/CheckboxGroup.vue
icon field in items
🔗 Linked issue
Resolves #5990
❓ Type of change
📚 Description
Add support for an optional icon per item in URadioGroup and UCheckboxGroup, alongside the existing text label. Each item's icon should render next to the label so it becomes possible to build visual pickers, such as layout or view selectors, where a label alone doesn't convey the choice clearly enough. The prop should stay optional so that items without an icon keep rendering exactly as they do today, and it should work across every visual style and size these two components already support. Screen reader behavior for the label must remain unaffected by the presence of an icon.
Example of implementation like the issue requires:
📝 Checklist