-
- Notifications
You must be signed in to change notification settings - Fork 53
radio tests #1240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
radio tests #1240
Conversation
WalkthroughTwo new test suites were added for the Changes
Sequence Diagram(s)sequenceDiagram participant TestSuite as Test Suite participant RadioGroup as RadioGroup.Root participant RadioItem as RadioGroup.Item participant RadioLabel as RadioGroup.Label TestSuite->>RadioGroup: Render with options and props RadioGroup->>RadioItem: Render each radio option RadioGroup->>RadioLabel: Render label for each item TestSuite->>RadioItem: Simulate click/select RadioItem-->>TestSuite: Selection state updates TestSuite->>RadioGroup: Check props/className/attributes Estimated code review effort2 (~15 minutes) 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! ✨ 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 ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/ui/RadioGroup/fragments/RadioGroupRoot.tsx (1)
28-28: Inefficient function call in render path.The current approach stores the function and calls it during render, which means the function gets called on every render. The previous approach of storing the result would have been more performant.
Consider reverting to the more efficient pattern:
- const composedAttributes = useComposeAttributes(dataAttributes(), accentAttributes()); + const composedAttributes = useComposeAttributes(dataAttributes(), accentAttributes())();And then use it as:
- <RadioGroupPrimitive.Root className={clsx(`${rootClass}-root`, className)} {...composedAttributes()} {...props}> + <RadioGroupPrimitive.Root className={clsx(`${rootClass}-root`, className)} {...composedAttributes} {...props}>src/components/ui/RadioGroup/tests/RadioGroup.test.tsx (1)
86-86: Remove console.log statement.The
console.logstatement should be removed as it's not needed in the test and could clutter test output.- console.log('group:', group);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/ui/RadioCards/tests/RadioCards.test.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/tests/RadioGroup.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 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/components/ui/RadioGroup/fragments/RadioGroupLabel.tsx (2)
Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.143Z
Learning: In the CheckboxGroupPrimitiveTrigger component (src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx), the component uses two separate useEffect hooks with different purposes: the first useEffect (lines 25-27) with empty dependency array [] is meant to set the initial state only once on mount by syncing with the group's checked values, while the second useEffect (lines 28-34) handles ongoing state updates by propagating local isChecked changes back to the group's checkedValues. This pattern prevents infinite loops while maintaining proper synchronization.
Learnt from: kotAPI
PR: #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/RadioGroup/fragments/RadioGroupRoot.tsx (4)
Learnt from: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.143Z
Learning: In the CheckboxGroupPrimitiveTrigger component (src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx), the component uses two separate useEffect hooks with different purposes: the first useEffect (lines 25-27) with empty dependency array [] is meant to set the initial state only once on mount by syncing with the group's checked values, while the second useEffect (lines 28-34) handles ongoing state updates by propagating local isChecked changes back to the group's checkedValues. This pattern prevents infinite loops while maintaining proper synchronization.
Learnt from: decipher-cs
PR: #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: #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: #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/components/ui/RadioGroup/tests/RadioGroup.test.tsx (2)
Learnt from: decipher-cs
PR: #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: GoldGroove06
PR: #1215
File: src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx:25-27
Timestamp: 2025-07-18T16:43:26.143Z
Learning: In the CheckboxGroupPrimitiveTrigger component (src/core/primitives/CheckboxGroup/fragments/CheckboxGroupPrimitiveTrigger.tsx), the component uses two separate useEffect hooks with different purposes: the first useEffect (lines 25-27) with empty dependency array [] is meant to set the initial state only once on mount by syncing with the group's checked values, while the second useEffect (lines 28-34) handles ongoing state updates by propagating local isChecked changes back to the group's checkedValues. This pattern prevents infinite loops while maintaining proper synchronization.
src/components/ui/RadioCards/tests/RadioCards.test.tsx (1)
Learnt from: decipher-cs
PR: #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/RadioGroup/fragments/RadioGroupRoot.tsx (2)
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)
⏰ 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 (5)
src/components/ui/RadioCards/tests/RadioCards.test.tsx (1)
1-82: Comprehensive test suite with excellent coverage!This test suite demonstrates excellent testing practices with comprehensive coverage of the RadioCards component functionality including rendering, selection behavior, props handling, and accessibility features. The use of ARIA attributes for assertions aligns with accessibility best practices, and the helper function promotes good DRY principles.
src/components/ui/RadioGroup/fragments/RadioGroupRoot.tsx (2)
23-23: Good improvement removing default empty strings.Removing the default empty string values for
variantandsizeis a good change -undefinedis more semantically correct for optional props than empty strings.
31-31: Clean className composition simplification.The removal of redundant
rootClassfrom the className composition is a good cleanup that maintains functionality while simplifying the code.src/components/ui/RadioGroup/fragments/RadioGroupLabel.tsx (1)
14-14: Consistent className simplification.The removal of redundant
rootClassfrom the className composition aligns well with similar changes made inRadioGroupRoot.tsxand maintains functionality while simplifying the code.src/components/ui/RadioGroup/tests/RadioGroup.test.tsx (1)
1-101: Excellent comprehensive test suite with minor cleanup needed.This test suite provides excellent coverage of the RadioGroup component functionality, including rendering, selection behavior, accessibility features, prop forwarding, and error handling. The StoryRadioGroup helper component is well-designed and the test structure follows best practices. Just needs the console.log statement removed.
Summary by CodeRabbit
Tests
Refactor