checkboxGroup primitive #1215
checkboxGroup primitive #1215
Conversation
WalkthroughThis change introduces a new CheckboxGroup component system, including primitive and UI-level components, context providers, and associated Storybook stories and SCSS styles. It establishes controlled and uncontrolled checkbox group state management, keyboard navigation, visual indicators, and label support, with all relevant types, contexts, and exports structured for modular use and theming. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant CheckboxGroup.Root
participant CheckboxGroup.Label
participant CheckboxGroup.Trigger
participant CheckboxGroup.Indicator
App->>CheckboxGroup.Root: Render with props (name, value, onChange)
CheckboxGroup.Root->>CheckboxGroup.Label: Render label for each option
CheckboxGroup.Label->>CheckboxGroup.Trigger: Render trigger with value
CheckboxGroup.Trigger->>CheckboxGroup.Indicator: Render indicator (if checked)
CheckboxGroup.Trigger-->>CheckboxGroup.Root: onCheckedChange (update checked values)
CheckboxGroup.Root-->>App: onValueChange callback (if controlled)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/core/primitives/CheckboxGroup/context/CheckboxGroupPrimitiveContext.tsx (1)
13-19: Consider using undefined for optional name default.The context initialization looks good, but consider using
undefinedinstead of empty string for thenamedefault value to be more semantically correct.- name: '', + name: undefined,src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx (1)
20-20: Clean up props destructuring.The props destructuring could be cleaner by extracting the remaining props properly.
-const CheckboxGroupPrimitiveRoot = ({ dir, orientation, loop, defaultValue = [], value, onValueChange, children, name, ...props }: CheckboxGroupPrimitiveRootProps) => { +const CheckboxGroupPrimitiveRoot = ({ dir, orientation, loop, defaultValue = [], value, onValueChange, children, name, required, disabled, ...props }: CheckboxGroupPrimitiveRootProps) => {src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveItem.tsx (1)
28-30: Consider showing children regardless of checked state.Currently, children are only shown when the checkbox is checked. This might not be the expected behavior for checkbox labels or icons.
- <button onClick={handleClick} className={className} aria-checked={checked} disabled={disabled} aria-required={required}> - {checked && children} - </button> + <button onClick={handleClick} className={className} aria-checked={checked} disabled={disabled} aria-required={required}> + {children} + </button>src/core/primitives/CheckboxGroup/stories/CheckboxGroupsPrimitive.stories.tsx (1)
8-10: Consider extracting TickIcon to a shared location.Since this icon is reused from CheckboxPrimitive stories, consider extracting it to a shared icons directory to avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/core/primitives/CheckboxGroup/CheckboxGroupsPrimitive.tsx(1 hunks)src/core/primitives/CheckboxGroup/context/CheckboxGroupPrimitiveContext.tsx(1 hunks)src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveItem.tsx(1 hunks)src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx(1 hunks)src/core/primitives/CheckboxGroup/stories/CheckboxGroupsPrimitive.stories.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/core/primitives/CheckboxGroup/CheckboxGroupsPrimitive.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/core/primitives/CheckboxGroup/context/CheckboxGroupPrimitiveContext.tsx (1)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx (3)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
src/core/primitives/CheckboxGroup/stories/CheckboxGroupsPrimitive.stories.tsx (2)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveItem.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
🧬 Code Graph Analysis (1)
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx (1)
src/core/hooks/useControllableState/index.tsx (1)
useControllableState(15-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/core/primitives/CheckboxGroup/context/CheckboxGroupPrimitiveContext.tsx (1)
5-11: LGTM! Well-structured context interface.The interface properly defines the shape of the checkbox group context with appropriate types.
src/core/primitives/CheckboxGroup/CheckboxGroupsPrimitive.tsx (2)
6-9: LGTM! Good compound component pattern.The warning message provides clear guidance for users, and returning
nullprevents accidental rendering.
11-12: LGTM! Clean static property assignment.The compound component pattern is implemented correctly with
RootandItemsubcomponents.src/core/primitives/CheckboxGroup/stories/CheckboxGroupsPrimitive.stories.tsx (1)
49-98: LGTM! Well-structured form example.The form story properly demonstrates checkbox group usage within forms with correct label association and form data handling.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx
Outdated
Show resolved
Hide resolved
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx
Show resolved
Hide resolved
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveItem.tsx
Outdated
Show resolved
Hide resolved
| </button> | ||
|
|
||
| </RovingFocusGroup.Item> | ||
| <input type="checkbox" checked={checked} name={name} value={value} style={{ display: 'none' }} required={required} disabled={disabled}/> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add onChange handler to hidden input.
The hidden input should have an onChange handler to prevent React warnings about controlled components.
- <input type="checkbox" checked={checked} name={name} value={value} style={{ display: 'none' }} required={required} disabled={disabled}/>
+ <input type="checkbox" checked={checked} name={name} value={value} style={{ display: 'none' }} required={required} disabled={disabled} onChange={() => {}} readOnly />🤖 Prompt for AI Agents
In src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveItem.tsx at
line 33, the hidden input element lacks an onChange handler, causing React
warnings about controlled components. Add an onChange handler to the input, even
if it is a no-op function, to satisfy React's requirement and prevent these
warnings.
There was a problem hiding this comment.
will do that in theme pr
There was a problem hiding this comment.
@GoldGroove06, sounds good! Deferring this to the theme PR makes sense since it's a non-critical warning fix.
Would you like me to create a follow-up issue to track this onChange handler addition so it doesn't get forgotten?
🧠 Learnings used
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
| <CheckboxGroupPrimitive.Root className="flex gap-4" name="fruits" defaultValue={['apple', 'banana']}> | ||
|
|
||
| <CheckboxGroupPrimitive.Item value="apple" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'> | ||
| <TickIcon /> | ||
| </CheckboxGroupPrimitive.Item> | ||
| <label className="flex items-center gap-2"> | ||
| Apple | ||
| </label> | ||
|
|
||
| <CheckboxGroupPrimitive.Item value="banana" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'> | ||
| <TickIcon /> | ||
| </CheckboxGroupPrimitive.Item> | ||
| <label className="flex items-center gap-2"> | ||
| Banana | ||
| </label> | ||
|
|
||
| <CheckboxGroupPrimitive.Item value="cherry" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'> | ||
| <span className="text-blue-900"><TickIcon /></span> | ||
| </CheckboxGroupPrimitive.Item> | ||
| <label className="flex items-center gap-2"> | ||
| Cherry | ||
| </label> | ||
| </CheckboxGroupPrimitive.Root> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix label association in Basic story.
The labels are not properly associated with their corresponding checkbox items, which creates accessibility issues. Users won't be able to click labels to toggle checkboxes.
- <CheckboxGroupPrimitive.Item value="apple" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'>
- <TickIcon />
- </CheckboxGroupPrimitive.Item>
- <label className="flex items-center gap-2">
- Apple
- </label>
+ <label className="flex items-center gap-2">
+ <CheckboxGroupPrimitive.Item value="apple" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'>
+ <TickIcon />
+ </CheckboxGroupPrimitive.Item>
+ Apple
+ </label>Apply the same pattern to banana and cherry items.
📝 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.
| <CheckboxGroupPrimitive.Root className="flex gap-4" name="fruits" defaultValue={['apple', 'banana']}> | |
| <CheckboxGroupPrimitive.Item value="apple" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'> | |
| <TickIcon /> | |
| </CheckboxGroupPrimitive.Item> | |
| <label className="flex items-center gap-2"> | |
| Apple | |
| </label> | |
| <CheckboxGroupPrimitive.Item value="banana" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'> | |
| <TickIcon /> | |
| </CheckboxGroupPrimitive.Item> | |
| <label className="flex items-center gap-2"> | |
| Banana | |
| </label> | |
| <CheckboxGroupPrimitive.Item value="cherry" className='bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center'> | |
| <span className="text-blue-900"><TickIcon /></span> | |
| </CheckboxGroupPrimitive.Item> | |
| <label className="flex items-center gap-2"> | |
| Cherry | |
| </label> | |
| </CheckboxGroupPrimitive.Root> | |
| <CheckboxGroupPrimitive.Root | |
| className="flex gap-4" | |
| name="fruits" | |
| defaultValue={['apple', 'banana']} | |
| > | |
| <label className="flex items-center gap-2"> | |
| <CheckboxGroupPrimitive.Item | |
| value="apple" | |
| className="bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center" | |
| > | |
| <TickIcon /> | |
| </CheckboxGroupPrimitive.Item> | |
| Apple | |
| </label> | |
| <label className="flex items-center gap-2"> | |
| <CheckboxGroupPrimitive.Item | |
| value="banana" | |
| className="bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center" | |
| > | |
| <TickIcon /> | |
| </CheckboxGroupPrimitive.Item> | |
| Banana | |
| </label> | |
| <label className="flex items-center gap-2"> | |
| <CheckboxGroupPrimitive.Item | |
| value="cherry" | |
| className="bg-gray-200 border border-blue-800 w-6 h-6 rounded-md flex items-center justify-center" | |
| > | |
| <span className="text-blue-900"> | |
| <TickIcon /> | |
| </span> | |
| </CheckboxGroupPrimitive.Item> | |
| Cherry | |
| </label> | |
| </CheckboxGroupPrimitive.Root> |
🤖 Prompt for AI Agents
In src/core/primitives/CheckboxGroup/stories/CheckboxGroupsPrimitive.stories.tsx
between lines 22 and 44, the label elements are not properly associated with
their corresponding CheckboxGroupPrimitive.Item components, causing
accessibility issues. Wrap each CheckboxGroupPrimitive.Item and its label inside
a single label element or use htmlFor and id attributes to link them properly.
Apply this fix consistently for the apple, banana, and cherry items so clicking
the label toggles the checkbox.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/components/ui/CheckboxGroup/fragments/CheckboxGroupTrigger.tsx (1)
3-3: Remove unused import.The
CheckboxGroupPrimitivePropsimport is not used in this component.-import CheckboxGroupPrimitive, { CheckboxGroupPrimitiveProps } from '~/core/primitives/CheckboxGroup/CheckboxGroupPrimitive'; +import CheckboxGroupPrimitive from '~/core/primitives/CheckboxGroup/CheckboxGroupPrimitive';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/components/ui/CheckboxGroup/CheckboxGroup.tsx(1 hunks)src/components/ui/CheckboxGroup/context/CheckboxGroupItemContext.tsx(1 hunks)src/components/ui/CheckboxGroup/context/CheckboxGroupRootContext.tsx(1 hunks)src/components/ui/CheckboxGroup/fragments/CheckboxGroupIndicator.tsx(1 hunks)src/components/ui/CheckboxGroup/fragments/CheckboxGroupItem.tsx(1 hunks)src/components/ui/CheckboxGroup/fragments/CheckboxGroupLabel.tsx(1 hunks)src/components/ui/CheckboxGroup/fragments/CheckboxGroupRoot.tsx(1 hunks)src/components/ui/CheckboxGroup/fragments/CheckboxGroupTrigger.tsx(1 hunks)src/components/ui/CheckboxGroup/stories/CheckboxGroup.stories.tsx(1 hunks)src/core/primitives/CheckboxGroup/CheckboxGroupPrimitive.tsx(1 hunks)src/core/primitives/CheckboxGroup/context/CheckboxGroupPrimitiveTriggerContext.tsx(1 hunks)src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveContent.tsx(1 hunks)src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx(1 hunks)src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx(1 hunks)src/core/primitives/CheckboxGroup/stories/CheckboxGroupPrimitive.stories.tsx(1 hunks)src/core/primitives/Primitive/index.tsx(1 hunks)styles/themes/components/checkbox-group.scss(1 hunks)styles/themes/default.scss(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- styles/themes/default.scss
- src/core/primitives/Primitive/index.tsx
- styles/themes/components/checkbox-group.scss
- src/components/ui/CheckboxGroup/fragments/CheckboxGroupIndicator.tsx
- src/components/ui/CheckboxGroup/context/CheckboxGroupRootContext.tsx
- src/core/primitives/CheckboxGroup/CheckboxGroupPrimitive.tsx
- src/core/primitives/CheckboxGroup/context/CheckboxGroupPrimitiveTriggerContext.tsx
- src/components/ui/CheckboxGroup/context/CheckboxGroupItemContext.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveRoot.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveContent.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupItem.tsx (1)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupLabel.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx (4)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
src/components/ui/CheckboxGroup/stories/CheckboxGroup.stories.tsx (1)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/CheckboxGroup/CheckboxGroup.tsx (3)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
src/core/primitives/CheckboxGroup/stories/CheckboxGroupPrimitive.stories.tsx (2)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupTrigger.tsx (3)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupRoot.tsx (3)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
🧬 Code Graph Analysis (1)
src/components/ui/CheckboxGroup/stories/CheckboxGroup.stories.tsx (1)
src/core/primitives/CheckboxGroup/stories/CheckboxGroupPrimitive.stories.tsx (1)
Basic(19-53)
🪛 Biome (1.9.4)
src/components/ui/CheckboxGroup/stories/CheckboxGroup.stories.tsx
[error] 28-28: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🪛 GitHub Check: lint
src/components/ui/CheckboxGroup/fragments/CheckboxGroupTrigger.tsx
[warning] 3-3:
'CheckboxGroupPrimitiveProps' is defined but never used
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (18)
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveContent.tsx (1)
1-17: LGTM! Clean primitive content component implementation.The component correctly uses context to conditionally render children based on the checked state, following the established primitive pattern in the codebase.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupLabel.tsx (1)
1-30: LGTM! Proper label implementation with correct event handling.The component correctly prevents default label behavior and manages state through context. The click handler properly toggles the checkbox state, and the CSS class composition follows the established pattern.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupItem.tsx (1)
1-21: LGTM! Proper item state management and context provision.The component correctly maintains internal checkbox state and provides context to children. The unused
propsparameter suggests future extensibility, which is acceptable.src/components/ui/CheckboxGroup/CheckboxGroup.tsx (1)
1-21: LGTM! Clean compound component pattern implementation.The component follows the established pattern in the codebase with proper warning for direct usage and clean API through static properties. The 'use client' directive is appropriate for client-side state management.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupTrigger.tsx (1)
11-22: LGTM! Proper integration between UI and primitive layers.The component correctly bridges the UI and primitive layers, using context for state management and styling. The composition with primitive components and props spreading provides good extensibility.
src/components/ui/CheckboxGroup/fragments/CheckboxGroupRoot.tsx (4)
1-7: Import structure looks good.The imports are well-organized and follow the project's conventions. The use of relative imports for internal components and absolute imports for core utilities is consistent.
10-18: Props interface is well-structured.The props interface properly extends the primitive props and includes all necessary UI-level customization options. The intersection with
CheckboxGroupPrimitiveProps.Rootensures compatibility with the underlying primitive.
20-26: Data attribute composition is implemented correctly.The component properly uses the custom hooks to create and compose data attributes for styling. The pattern of creating separate attribute sets and then composing them is consistent with the codebase patterns.
27-37: Component structure and context provision are correct.The component properly wraps the primitive with the UI context and passes through all necessary props. The context provider pattern ensures child components can access the rootClass for consistent styling.
src/components/ui/CheckboxGroup/stories/CheckboxGroup.stories.tsx (2)
1-16: Story setup and state management look good.The component demonstrates proper controlled usage with React state management. The options structure and change handler are well-implemented.
43-49: Storybook configuration is properly structured.The story metadata and export structure follow Storybook conventions correctly.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx (3)
16-24: Controllable state setup is correct.The use of
useControllableStatefollows the established pattern for supporting both controlled and uncontrolled modes.
40-46: Accessibility implementation is comprehensive.The component properly implements ARIA checkbox role with correct attributes and integrates well with the roving focus group for keyboard navigation.
49-49: Form integration is well-implemented.The hidden input maintains proper form semantics and syncs all relevant attributes correctly.
src/core/primitives/CheckboxGroup/stories/CheckboxGroupPrimitive.stories.tsx (4)
8-10: TickIcon reuse is good practice.Reusing the TickIcon from other stories maintains consistency and reduces duplication.
19-53: Basic story demonstrates the primitive API well.The story effectively shows the checkbox group primitive usage with proper defaultValue initialization and styling.
57-74: Form data processing logic is well-implemented.The form submit handler correctly processes multiple checkbox values and consolidates them into arrays when needed. This demonstrates good form integration patterns.
75-111: Form story structure is comprehensive.The form story effectively demonstrates real-world usage with proper form integration and submit handling.
| {options.map((option) => ( | ||
| <> | ||
|
|
||
| <CheckboxGroup.Item value={option.value}> | ||
| <CheckboxGroup.Trigger > | ||
| <CheckboxGroup.Indicator /> | ||
| </CheckboxGroup.Trigger> | ||
| <CheckboxGroup.Label> | ||
| {option.label} | ||
| </CheckboxGroup.Label> | ||
| </CheckboxGroup.Item> | ||
| </> | ||
| ))} |
There was a problem hiding this comment.
Fix missing key prop and remove unnecessary fragment.
The static analysis correctly identified missing key props in the map iteration. Also, the fragment wrapper is unnecessary here.
Apply this diff to fix the issues:
- {options.map((option) => (
- <>
-
- <CheckboxGroup.Item value={option.value}>
+ {options.map((option) => (
+ <CheckboxGroup.Item key={option.id} value={option.value}>
<CheckboxGroup.Trigger >
<CheckboxGroup.Indicator />
</CheckboxGroup.Trigger>
<CheckboxGroup.Label>
{option.label}
</CheckboxGroup.Label>
- </CheckboxGroup.Item>
- </>
+ </CheckboxGroup.Item>
))}📝 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.
| {options.map((option) => ( | |
| <> | |
| <CheckboxGroup.Item value={option.value}> | |
| <CheckboxGroup.Trigger > | |
| <CheckboxGroup.Indicator /> | |
| </CheckboxGroup.Trigger> | |
| <CheckboxGroup.Label> | |
| {option.label} | |
| </CheckboxGroup.Label> | |
| </CheckboxGroup.Item> | |
| </> | |
| ))} | |
| {options.map((option) => ( | |
| <CheckboxGroup.Item key={option.id} value={option.value}> | |
| <CheckboxGroup.Trigger> | |
| <CheckboxGroup.Indicator /> | |
| </CheckboxGroup.Trigger> | |
| <CheckboxGroup.Label> | |
| {option.label} | |
| </CheckboxGroup.Label> | |
| </CheckboxGroup.Item> | |
| ))} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🤖 Prompt for AI Agents
In src/components/ui/CheckboxGroup/stories/CheckboxGroup.stories.tsx between
lines 25 and 37, the map iteration over options is missing a key prop on the
root element inside the map, and there is an unnecessary fragment wrapping the
CheckboxGroup.Item component. Remove the fragment and add a unique key prop to
CheckboxGroup.Item, typically using option.value, to fix the React key warning
and clean up the code.
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx
Outdated
Show resolved
Hide resolved
src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx
Outdated
Show resolved
Hide resolved
| // @ts-ignore | ||
| for (const [key, value] of formData.entries()) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-ignore by using proper typing.
The TypeScript error can be resolved with proper typing instead of using @ts-ignore.
Apply this diff to fix the typing issue:
- // @ts-ignore
- for (const [key, value] of formData.entries()) {
+ for (const [key, value] of formData.entries() as IterableIterator<[string, FormDataEntryValue]>) {📝 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.
| // @ts-ignore | |
| for (const [key, value] of formData.entries()) { | |
| for (const [key, value] of formData.entries() as IterableIterator<[string, FormDataEntryValue]>) { |
🤖 Prompt for AI Agents
In src/core/primitives/CheckboxGroup/stories/CheckboxGroupPrimitive.stories.tsx
at lines 61-62, remove the @ts-ignore comment and fix the TypeScript error by
properly typing the formData variable. Ensure formData is typed as FormData or a
compatible type that supports the entries() method, so the for...of loop over
formData.entries() is correctly typed without needing to ignore TypeScript
errors.
kotAPI
added
the
automerge
Screen.Recording.2025-07-16.132234.mp4
Summary by CodeRabbit
New Features
Documentation
Style