Accordion controlled uncontrolled #1045
Conversation
|
""" WalkthroughThe Accordion component was updated to support both controlled and uncontrolled modes by introducing new props: Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant AccordionRoot
participant useControllableState
ParentComponent->>AccordionRoot: Pass value/defaultValue/onValueChange props
AccordionRoot->>useControllableState: Initialize state (controlled or uncontrolled)
useControllableState-->>AccordionRoot: Return current state and setter
AccordionRoot-->>ParentComponent: Call onValueChange when state changes (if controlled)
ParentComponent-->>AccordionRoot: Update value prop (if controlled)
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (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:
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: 0
🧹 Nitpick comments (2)
src/components/ui/Accordion/stories/Accordion.stories.tsx (1)
101-103: Fix typo in story name.There's a spelling error in the story name: "WithDeafultValue" should be "WithDefaultValue".
-export const WithDeafultValue: Story = { +export const WithDefaultValue: Story = { render: () => <AccordionExample defaultValue={[2]} /> };src/components/ui/Accordion/tests/Accordion.test.tsx (1)
145-146: Remove extra blank line.There's an extra blank line that should be removed according to the ESLint warning.
}); - test('works with defaultValue to show initial item', () => {🧰 Tools
🪛 ESLint
[error] 145-146: More than 1 blank line not allowed.
(no-multiple-empty-lines)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/ui/Accordion/fragments/AccordionRoot.tsx(2 hunks)src/components/ui/Accordion/stories/Accordion.stories.tsx(2 hunks)src/components/ui/Accordion/tests/Accordion.test.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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.
src/components/ui/Accordion/fragments/AccordionRoot.tsx (1)
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.
🪛 ESLint
src/components/ui/Accordion/tests/Accordion.test.tsx
[error] 145-146: More than 1 blank line not allowed.
(no-multiple-empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (8)
src/components/ui/Accordion/fragments/AccordionRoot.tsx (4)
6-6: Good addition of the useControllableState hook.This properly supports both controlled and uncontrolled modes as mentioned in the retrieved learning.
22-24: Well-implemented props for controlled/uncontrolled state management.These new props align perfectly with React patterns:
valuefor controlled statedefaultValuefor initial uncontrolled stateonValueChangecallback to notify parent components of state changes
27-27: Good default value initialization.Setting
defaultValue = []ensures the component works correctly in uncontrolled mode without any items initially open.
31-35: Excellent implementation of useControllableState.The useControllableState hook is correctly implemented to handle both controlled and uncontrolled modes, managing the internal state while respecting external control when provided.
src/components/ui/Accordion/stories/Accordion.stories.tsx (1)
105-119: Well-implemented controlled mode example.This story effectively demonstrates how to use the Accordion in controlled mode with external state management. The inclusion of buttons to manipulate the state provides a clear example of how to control the component from outside.
src/components/ui/Accordion/tests/Accordion.test.tsx (3)
6-6: Good updates to TestAccordion to support new props.The TestAccordion component has been correctly updated to accept and spread Accordion props, enabling proper testing of the new functionality.
Also applies to: 17-19
105-143: Comprehensive test for controlled mode.This test thoroughly verifies the controlled behavior of the Accordion component, ensuring it responds correctly to external state changes.
146-153: Good test for defaultValue initialization.This test correctly verifies that the Accordion properly initializes with the specified defaultValue.
kotAPI
added
the
automerge
2 participants
added useControllableState to accordion. closes #1010
Summary by CodeRabbit