Themes radio card #1224
Themes radio card #1224
Conversation
|
Warning Rate limit exceeded@GoldGroove06 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe changes overhaul the radio group and radio cards components, shifting from native input-based implementations to ARIA-compliant button structures with explicit context and indicator subcomponents. TypeScript types are clarified, contexts are simplified, and new styling conventions are introduced. Story and test files are updated to reflect the new component APIs and markup structures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RadioGroup
participant RadioGroup.Item
participant RadioGroup.Indicator
participant RadioGroup.Label
participant Context
User->>RadioGroup: Renders RadioGroup.Root with children
RadioGroup->>Context: Provides rootClass via context
User->>RadioGroup.Label: Renders label for each option
RadioGroup.Label->>Context: Consumes rootClass for styling
RadioGroup.Label->>RadioGroup.Item: Wraps RadioGroup.Item
RadioGroup.Item->>Context: Consumes rootClass, provides itemSelected via context
RadioGroup.Item->>RadioGroup.Indicator: Renders if itemSelected
RadioGroup.Indicator->>Context: Reads itemSelected, renders indicator if true
Suggested labels
Suggested reviewers
Poem
✨ 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: 2
🧹 Nitpick comments (4)
src/components/ui/RadioGroup/fragments/RadioGroupLabel.tsx (1)
14-14: Potential class duplication in className composition.The current implementation includes both
${rootClass}-labelandrootClassin the className composition. This might result in duplicate classes ifrootClassis already a complete class name. Consider whether both are necessary or if the composition logic needs adjustment.If
rootClassis meant to be a prefix, consider this approach:-return <Primitive.label {...props} className={clsx(`${rootClass}-label`, rootClass, className)} asChild={asChild}> {children} </Primitive.label>; +return <Primitive.label {...props} className={clsx(`${rootClass}-label`, className)} asChild={asChild}> {children} </Primitive.label>;Alternatively, if both classes are intentionally needed, please ensure this aligns with the styling conventions.
src/core/primitives/RadioGroup/tests/RadioGroupPrimtive.test.tsx (1)
15-25: Consider handling potential null values more explicitly.The non-null assertions could mask issues if elements are not found. Consider explicit null checks for better debugging:
- const radioA = screen.getByText('Option A').closest('button'); - const radioB = screen.getByText('Option B').closest('button'); - const radioC = screen.getByText('Option C').closest('button'); + const radioA = screen.getByText('Option A').closest('button'); + const radioB = screen.getByText('Option B').closest('button'); + const radioC = screen.getByText('Option C').closest('button'); + + expect(radioA).not.toBeNull(); + expect(radioB).not.toBeNull(); + expect(radioC).not.toBeNull(); + expect(radioA).toHaveAttribute('aria-checked', 'false'); expect(radioB).toHaveAttribute('aria-checked', 'true'); expect(radioC).toHaveAttribute('aria-checked', 'false'); - fireEvent.click(radioA!); + if (radioA) fireEvent.click(radioA);Also applies to: 35-37
styles/themes/components/radio-group.scss (2)
20-21: Guard animations withprefers-reduced-motionThree separate
transitionrules are unconditional. Wrap them in a media query to respect users who request reduced motion.- transition: border-color 0.2s, box-shadow 0.2s, background 0.2s; + @media (prefers-reduced-motion: no-preference) { + transition: border-color 0.2s, box-shadow 0.2s, background 0.2s; + }(Repeat for the indicator and label blocks.)
Also applies to: 40-41, 61-62
48-55:margin-top / margin-bottommay double-stack with the parentgapThe root already spaces children with
gap: 32px. Extra vertical margins on the label can create inconsistent spacing between items. Consider dropping the per-label margins and rely solely ongap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/components/ui/RadioCards/RadioCards.stories.tsx(2 hunks)src/components/ui/RadioCards/context/RadioCardsContext.tsx(1 hunks)src/components/ui/RadioCards/fragments/RadioCardsItem.tsx(1 hunks)src/components/ui/RadioCards/fragments/RadioCardsRoot.tsx(2 hunks)src/components/ui/RadioGroup/RadioGroup.tsx(2 hunks)src/components/ui/RadioGroup/context/RadioGroupContext.tsx(1 hunks)src/components/ui/RadioGroup/fragments/RadioGroupIndicator.tsx(1 hunks)src/components/ui/RadioGroup/fragments/RadioGroupItem.tsx(1 hunks)src/components/ui/RadioGroup/fragments/RadioGroupLabel.tsx(1 hunks)src/components/ui/RadioGroup/fragments/RadioGroupRoot.tsx(1 hunks)src/components/ui/RadioGroup/stories/RadioGroup.stories.tsx(2 hunks)src/core/primitives/Primitive/index.tsx(1 hunks)src/core/primitives/RadioGroup/RadioGroupPrimitive.stories.tsx(2 hunks)src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx(2 hunks)src/core/primitives/RadioGroup/context/RadioGroupContext.tsx(0 hunks)src/core/primitives/RadioGroup/context/RadioGroupPrimitiveItemContext.tsx(1 hunks)src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveIndicator.tsx(1 hunks)src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveItem.tsx(1 hunks)src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveRoot.tsx(2 hunks)src/core/primitives/RadioGroup/tests/RadioGroupPrimtive.test.tsx(1 hunks)styles/themes/components/radio-group.scss(1 hunks)styles/themes/components/radiocards.scss(1 hunks)
💤 Files with no reviewable changes (1)
- src/core/primitives/RadioGroup/context/RadioGroupContext.tsx
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
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/Primitive/index.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: krau5
PR: rad-ui/ui#609
File: src/components/ui/Text/Text.tsx:19-22
Timestamp: 2024-12-06T14:30:05.834Z
Learning: When the keys and values for the `as` prop are the same, we should define the type directly using string literals (e.g., `'div' | 'span' | 'p' | 'label'`) and eliminate extra mappings like `tagMap` to simplify the code.
src/components/ui/RadioCards/context/RadioCardsContext.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: 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/RadioGroup/stories/RadioGroup.stories.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: 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/RadioGroup/context/RadioGroupContext.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: 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/RadioGroup/fragments/RadioGroupIndicator.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/RadioGroup/RadioGroupPrimitive.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/components/ui/RadioCards/fragments/RadioCardsItem.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/components/ui/RadioCards/RadioCards.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/RadioGroup/fragments/RadioGroupPrimitiveRoot.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: 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/RadioGroup/fragments/RadioGroupPrimitiveIndicator.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.
styles/themes/components/radiocards.scss (1)
Learnt from: kotAPI
PR: rad-ui/ui#640
File: .github/workflows/build-rad-ui.yml:5-5
Timestamp: 2024-12-14T02:25:41.034Z
Learning: In the `rad-ui/ui` repository, the `security-fixes` branch only updates dependencies, so linting and Chromatic workflows are not required for this branch.
src/core/primitives/RadioGroup/RadioGroupPrimitive.stories.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: 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/RadioGroup/fragments/RadioGroupItem.tsx (2)
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: 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/RadioGroup/context/RadioGroupPrimitiveItemContext.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/RadioGroup/tests/RadioGroupPrimtive.test.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/RadioGroup/fragments/RadioGroupPrimitiveItem.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/RadioGroup/fragments/RadioGroupRoot.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/components/ui/RadioCards/fragments/RadioCardsRoot.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.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.
🧬 Code Graph Analysis (6)
src/components/ui/RadioGroup/fragments/RadioGroupLabel.tsx (1)
src/components/ui/RadioGroup/context/RadioGroupContext.tsx (1)
RadioGroupContext(3-5)
src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx (3)
src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveRoot.tsx (1)
RadioGroupPrimitiveRootProps(7-19)src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveItem.tsx (1)
RadioGroupPrimitiveItemProps(7-14)src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveIndicator.tsx (1)
RadioGroupPrimitiveIndicatorProps(5-9)
src/components/ui/RadioCards/fragments/RadioCardsItem.tsx (2)
src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx (1)
Item(19-19)src/components/ui/RadioCards/context/RadioCardsContext.tsx (1)
RadioCardsContext(3-7)
src/components/ui/RadioGroup/fragments/RadioGroupItem.tsx (2)
src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx (1)
Item(19-19)src/components/ui/RadioGroup/context/RadioGroupContext.tsx (1)
RadioGroupContext(3-5)
src/components/ui/RadioGroup/fragments/RadioGroupRoot.tsx (3)
src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx (1)
Root(18-18)src/core/hooks/createDataAttribute/index.ts (3)
useCreateDataAttribute(10-25)useCreateDataAccentColorAttribute(48-55)useComposeAttributes(33-40)src/components/ui/RadioGroup/context/RadioGroupContext.tsx (1)
RadioGroupContext(3-5)
src/components/ui/RadioCards/fragments/RadioCardsRoot.tsx (2)
src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx (1)
Root(18-18)src/components/ui/RadioCards/context/RadioCardsContext.tsx (1)
RadioCardsContext(3-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)
- GitHub Check: build
🔇 Additional comments (31)
src/core/primitives/Primitive/index.tsx (1)
4-4: LGTM! Adding label support enhances semantic HTML structure.The addition of
'label'to the supported HTML elements is essential for creating accessible radio group components. This enables proper label-input associations in the radio group implementations.src/components/ui/RadioCards/context/RadioCardsContext.tsx (1)
3-7: LGTM! Context simplification improves architecture.The removal of
defaultCheckedandonChangefrom the context aligns with the architectural goal of delegating state management to primitive components. The simplified context now focuses solely on styling concerns (rootClass), which is a cleaner separation of responsibilities.src/components/ui/RadioGroup/context/RadioGroupContext.tsx (1)
3-5: LGTM! Consistent context simplification across radio components.The simplification to only include
rootClassmaintains consistency with the RadioCardsContext and supports the architectural goal of moving state management to primitive components. This cleaner separation of concerns improves maintainability.src/components/ui/RadioGroup/fragments/RadioGroupLabel.tsx (1)
6-10: LGTM! Well-defined component props.The props interface is clean and follows the established pattern with proper support for
children,className, andasChildprops.src/components/ui/RadioGroup/RadioGroup.tsx (2)
4-5: LGTM! Clean API extension with new fragments.The addition of
RadioGroupIndicatorandRadioGroupLabelimports properly extends the RadioGroup API while maintaining consistency with the existing fragment pattern.
21-22: LGTM! Consistent fragment export pattern.The new fragment exports follow the established pattern and provide a clean API for consumers to use the new indicator and label components.
src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveIndicator.tsx (1)
1-17: LGTM! Clean conditional indicator implementation.The component correctly implements the conditional rendering pattern using context to determine selection state. The early return for non-selected items is efficient and the prop forwarding is handled appropriately.
src/components/ui/RadioGroup/stories/RadioGroup.stories.tsx (1)
13-22: LGTM! Improved component composition pattern.The new structure with
RadioGroup.LabelwrappingRadioGroup.ItemandRadioGroup.Indicatorprovides better semantic HTML and accessibility. The removal of internal state management simplifies the API and delegates state handling to the primitive layer appropriately.src/core/primitives/RadioGroup/context/RadioGroupPrimitiveItemContext.tsx (1)
5-13: LGTM! Well-designed context for item selection state.The context interface is appropriately minimal with just the
itemSelectedboolean. The default value offalseis sensible and the implementation follows React context best practices.src/components/ui/RadioCards/RadioCards.stories.tsx (1)
5-25: LGTM! Consistent API simplification pattern.The changes mirror the RadioGroup stories refactoring, removing internal state management and simplifying the component structure. This consistency across the codebase improves maintainability and provides a cleaner API surface.
src/components/ui/RadioGroup/fragments/RadioGroupIndicator.tsx (1)
11-18: LGTM! Well-structured UI layer component.The component properly wraps the primitive indicator with context-aware styling. The use of
clsxfor className combination and therootClass + '-indicator'pattern is consistent with the established architecture. Props are correctly typed and forwarded.src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveRoot.tsx (3)
7-19: LGTM: Well-structured type export for component composition.The export of
RadioGroupPrimitiveRootPropsenables proper type composition in higher-level components, which aligns with the architectural changes mentioned in the summary.
28-32: Good: Simplified context value removes unnecessarynameproperty.The removal of
namefrom the context aligns with the architectural shift towards ARIA-compliant button structures where thenameattribute is handled at the form level rather than distributed through context.
35-46: Excellent: ARIA-compliant structure with form compatibility.The restructuring provides several improvements:
- Proper ARIA attributes (
role="radiogroup",aria-required,aria-disabled) on the root element- Hidden input element preserves form submission functionality
- Maintains accessibility with RovingFocusGroup for keyboard navigation
This is a well-architected solution that balances accessibility, form compatibility, and modern component patterns.
styles/themes/components/radiocards.scss (2)
1-5: LGTM: Clean container styling.The trailing whitespace removal and container styles are well-structured for the RadioCards component.
6-22: Good: Consistent card dimensions and layout.The fixed dimensions (180x80px) and vertical flex layout with centered alignment provide a consistent visual appearance for radio cards. The removal of input-specific styles aligns with the architectural shift to ARIA-compliant button structures.
src/core/primitives/RadioGroup/RadioGroupPrimitive.stories.tsx (2)
35-42: LGTM: Improved semantic structure and accessibility.The changes enhance accessibility and semantic HTML:
- Wrapping each item in a
<label>element improves screen reader support- Moving the
keyprop to the label follows React best practices- Adding the
RadioGroupPrimitive.Indicatorcomponent provides clear visual feedback- Label text placement outside the item but inside the label maintains proper association
84-89: Good: Consistent story structure with proper form semantics.The
InFormstory maintains the same improved structure with proper label wrapping and indicator usage, which is essential for form submission testing.src/components/ui/RadioGroup/fragments/RadioGroupItem.tsx (3)
2-2: LGTM: Proper import for type composition.Adding
RadioGroupPrimitivePropsimport enables proper type extension and maintains consistency with the primitive layer.
6-10: Excellent: Strong typing with required value prop.The explicit type definition improves API safety by:
- Making the
valueprop required, preventing runtime errors- Extending from
RadioGroupPrimitiveProps.Itemensures consistency- Clear interface definition improves developer experience
12-14: Good: Explicit prop handling improves transparency.The explicit extraction and passing of the
valueprop makes the component interface clearer and ensures the required prop is properly forwarded to the primitive.src/core/primitives/RadioGroup/RadioGroupPrimitive.tsx (3)
5-5: LGTM: Proper import for new Indicator component.The import of
RadioGroupPrimitiveIndicatorand its props type correctly extends the primitive API surface.
17-21: Excellent: Well-organized namespace for prop types.The
RadioGroupPrimitivePropsnamespace provides clean access to all component prop types, improving developer experience and type discoverability. The addition of theIndicatortype completes the API.
25-25: Good: Consistent API expansion with static property.Adding
RadioGroupPrimitive.Indicatorfollows the established pattern and completes the primitive component API, enabling the indicator functionality demonstrated in the stories.src/components/ui/RadioCards/fragments/RadioCardsItem.tsx (1)
7-15: LGTM! Good improvement to type safety.The explicit type definition and required
valueprop enhance type safety and make the component's contract clearer. This aligns well with the pattern used in other RadioGroup components.src/core/primitives/RadioGroup/tests/RadioGroupPrimtive.test.tsx (1)
4-25: Test updates correctly reflect the new button-based implementation.The migration from native input queries to ARIA role-based queries and assertions is well-executed and maintains test coverage.
src/components/ui/RadioCards/fragments/RadioCardsRoot.tsx (1)
2-22: Excellent simplification of the component.The refactoring to extend
RadioGroupPrimitiveProps.Rootand delegate state management to the primitive reduces complexity and improves maintainability. The component now has a clear single responsibility for styling and context provision.src/core/primitives/RadioGroup/fragments/RadioGroupPrimitiveItem.tsx (2)
16-48: Excellent refactoring to ARIA-compliant button implementation.The consolidation into a single button element with proper ARIA attributes significantly improves accessibility and simplifies the component structure. The context provider pattern for selection state is well-implemented.
31-32: Verify the onFocus selection behavior.The component selects the radio on focus, which differs from native radio button behavior where focus doesn't change selection. Please confirm this is the intended UX pattern.
If this behavior should match native radio buttons, consider:
- onFocus={() => setSelectedValue(value)} + onFocus={props.onFocus}src/components/ui/RadioGroup/fragments/RadioGroupRoot.tsx (1)
17-30: Well-structured theming implementation.The addition of variant, size, and color props with data attribute generation provides a clean and flexible theming API. The use of composition hooks is a good architectural choice.
styles/themes/components/radio-group.scss (1)
34-40: Indicator dimensions include its 2 px border – is that intentional?
width/height: 12pxplus a2pxborder yields an 8 px filled circle, not 12 px.
If the visual spec calls for a 12 px filled dot inside the border, setbox-sizing: content-boxor increasewidth/heightto12 + 2*border = 16px.
| &:focus { | ||
| outline: 2px solid var(--rad-ui-color-accent-700); | ||
| outline-offset: 2px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer :focus-visible over :focus to avoid perpetual outlines
Using the generic :focus selector means the outline appears even when the item gains programmatic focus (e.g. mouse-down). Switching to :focus-visible keeps the outline only for keyboard / accessibility use-cases and aligns with modern a11y guidance.
- &:focus {
+ &:focus-visible {
outline: 2px solid var(--rad-ui-color-accent-700);
outline-offset: 2px;
}🤖 Prompt for AI Agents
In styles/themes/components/radio-group.scss around lines 28 to 31, replace the
:focus selector with :focus-visible to ensure the outline only appears during
keyboard navigation and not on mouse interactions, improving accessibility and
user experience.
kotAPI
added
the
automerge
kodiakhq
bot
removed
the
automerge
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
GoldGroove06
added
the
automerge
Screen.Recording.2025-07-18.150307.mp4
Summary by CodeRabbit
New Features
Refactor
Style
Documentation
Tests