fix(docs): incorrect code in Modal placement #4652
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes involve modifying the modal component documentation to introduce a new feature for modal placement. The implementation replaces the previous scroll behavior state with a new Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant Modal
User->>Component: Select modal placement
Component->>Modal: Set placement prop
Modal->>User: Render modal at specified position
Assessment against linked issues
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
wingkwong
changed the title
![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 (3)
apps/docs/content/components/modal/placement.raw.jsx (3)
1-1: Consider using constants for placement values.Define placement options as constants to maintain consistency and prevent typos throughout the code.
+const MODAL_PLACEMENTS = { + AUTO: "auto", + TOP: "top", + BOTTOM: "bottom", + CENTER: "center", + TOP_CENTER: "top-center", + BOTTOM_CENTER: "bottom-center", +} as const; -const [modalPlacement, setModalPlacement] = useState("auto"); +const [modalPlacement, setModalPlacement] = useState(MODAL_PLACEMENTS.AUTO);Also applies to: 16-16
23-34: Enhance accessibility for the RadioGroup component.Consider adding an aria-label to improve screen reader experience.
<RadioGroup label="Select modal placement" + aria-label="Modal placement options" orientation="horizontal" value={modalPlacement} onValueChange={setModalPlacement} >
36-36: Add JSDoc comment to document placement options.Consider adding a comment to document the available placement options for better developer experience.
+/** + * Modal component with configurable placement + * @param {string} placement - One of: "auto", "top", "bottom", "center", "top-center", "bottom-center" + */ <Modal isOpen={isOpen} placement={modalPlacement} onOpenChange={onOpenChange}>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/content/components/modal/placement.raw.jsx(2 hunks)
🔇 Additional comments (2)
apps/docs/content/components/modal/placement.raw.jsx (2)
19-22: Clean and well-structured layout implementation!The flex layout with proper spacing and centered alignment provides a good user experience for the demo.
36-36: Verify Modal component prop types support.Let's verify that the Modal component properly supports all the new placement values.
✅ Verification successful
✅ Modal placement prop is properly supported
The Modal component fully supports the placement prop with the following valid values:
auto(default),center,top,top-center,bottom, andbottom-center. The implementation includes proper TypeScript types and corresponding layout styles.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Modal component prop types definition ast-grep --pattern 'interface ModalProps { $$$ placement?: $_ $$$ }'Length of output: 76
Script:
#!/bin/bash # Search for Modal component and placement prop definitions using multiple patterns echo "=== Searching for Modal component definition ===" rg -A 10 "export.*Modal.*=" echo "=== Searching for placement prop usage ===" rg "placement.*=.*{" --type ts --type tsx --type jsx echo "=== Searching for type definitions ===" ast-grep --pattern 'type ModalProps = { $$$ }'Length of output: 3129
Script:
#!/bin/bash # Search for Modal implementation and placement prop echo "=== Searching for Modal props and placement in core files ===" rg -A 5 "type.*Modal.*Props" packages/components/modal/src/ rg -A 5 "interface.*Modal.*Props" packages/components/modal/src/ echo "=== Searching for placement in modal variants ===" cat packages/core/theme/src/components/modal.ts echo "=== Searching for placement prop usage with fixed pattern ===" rg "placement.*=" --type ts --type tsx --type jsxLength of output: 9574
Closes #4649
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
Documentation