Feat/modal component #76
Conversation
WalkthroughThe changes introduce a refactored Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Button
participant ModalOverlay
participant AriaModal
participant Dialog
User->>Button: Click "Open Modal"
Button->>ModalOverlay: Set isOpen=true
ModalOverlay->>AriaModal: Render modal structure
AriaModal->>Dialog: Render modal content
User->>Dialog: Interact with modal
Dialog->>ModalOverlay: onOpenChange (close modal)
ModalOverlay->>Button: Set isOpen=false
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 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: 1
🧹 Nitpick comments (1)
src/lib/container/modal.tsx (1)
35-37: Be cautious with duplicate prop spreading
{...props}is being spread to both theModalOverlay(line 35) and theAriaModal(line 37). This could potentially lead to prop conflicts or unexpected behavior if both components interpret the same props differently.Consider being more explicit about which props go to which component:
- <ModalOverlay - className={cn( - "fixed top-0 left-0 isolate", - "z-20 h-(--visual-viewport-height) w-full bg-black/[50%] backdrop-blur-md", - "flex items-center justify-center", - "entering:animate-[fadeIn_100ms_ease-out] exiting:animate-[fadeOut_100ms_ease-in]", - modalOverlayClassname, - )} - {...props} - > - <AriaModal {...props}> + <ModalOverlay + className={cn( + "fixed top-0 left-0 isolate", + "z-20 h-(--visual-viewport-height) w-full bg-black/[50%] backdrop-blur-md", + "flex items-center justify-center", + "entering:animate-[fadeIn_100ms_ease-out] exiting:animate-[fadeOut_100ms_ease-in]", + modalOverlayClassname, + )} + {...overlayProps} + > + <AriaModal {...modalProps}>Where
overlayPropsandmodalPropswould be appropriately filtered from the originalprops.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/container/modal.tsx(1 hunks)src/lib/index.ts(1 hunks)src/stories/modal.stories.tsx(2 hunks)src/styles/theme.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib/container/modal.tsx (2)
src/stories/modal.stories.tsx (1)
Modal(26-48)src/utils/index.ts (1)
cn(4-6)
🔇 Additional comments (8)
src/styles/theme.css (1)
37-37: Good improvement to class selector for dark themeChanging from
:root[class="dark"]to:root.darkis a more flexible approach that allows the dark theme to be applied alongside other root classes. The previous selector would only match if "dark" was the exact class attribute value, which is more restrictive.src/lib/container/modal.tsx (3)
11-17: Well-structured interface with good documentationThe
ModalPropsinterface is well-defined, extending the appropriate react-aria component props while adding specific customization options. The JSDoc comment formodalOverlayClassnameprovides clear guidance on its purpose.
25-25: Good use of Readonly type modifierUsing
Readonly<ModalProps>helps ensure immutability of props, which is a best practice for React components.
38-47: Good use of nested components for accessibilityThe nesting of
DialogwithinAriaModalwithinModalOverlayprovides good separation of concerns and aligns with accessibility best practices. The structure makes it clear which component is responsible for which aspect of the modal's behavior.src/lib/index.ts (1)
9-9: Good addition of Modal exportAdding the Modal component to the library exports makes it available for consumers, consistent with other components.
src/stories/modal.stories.tsx (3)
5-6: Fixed import path and added Button componentThe import path for the Modal component has been correctly updated, and the Button component is now imported to provide a trigger for the modal. This is a good improvement for the story's demonstration capability.
12-19: Good addition of Storybook controlsAdding
argTypesforisOpenandisDismissableprops improves the Storybook experience by allowing users to interactively control these properties.
33-46: Excellent implementation of controlled modal stateUsing
useStateto control the modal's open state and the Button component to trigger it provides a much better demonstration of how the modal should be used in a real application. The centered content layout also improves the presentation.
tractorss
dismissed stale reviews from coderabbitai[bot] and alcercu
via
59b5a66
|
PR-Codex overview
This PR focuses on updating the
Modalcomponent in the@kleros/ui-components-libraryby enhancing its functionality and styling, while also updating the version inpackage.json.Detailed summary
:root[class="dark"]to:root.darkinsrc/styles/theme.css.3.0.1to3.1.2inpackage.json.Modalexport insrc/lib/index.ts.ModalComponentinsrc/stories/modal.stories.tsx.isOpenandisDismissableprops inModalstory.useStatefor modal open state in the story.Modalcomponent insrc/lib/container/modal.tsx:modalOverlayClassnameandchildrenprops.AriaModaltoModalOverlaystructure.Dialogfor modal content rendering.Summary by CodeRabbit