Conversation
kylengn
requested review from
EnzalRad,
dvxam and
noe-charmet
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR introduces a responsive modal component with breakpoint-aware desktop/mobile rendering using Radix Dialog and Vaul Drawer, implements a useMediaQuery hook for breakpoint detection, updates code-block footer components with typography-based properties, adds the vaul dependency, and exposes new components through barrel exports. Changes
Sequence DiagramsequenceDiagram
participant User
participant Modal as Modal Component
participant Breakpoint as useMediaQuery
participant Desktop as Radix Dialog
participant Mobile as Vaul Drawer
User->>Modal: Render modal
Modal->>Breakpoint: Check breakpoint (< md?)
alt Desktop (≥ md)
Breakpoint-->>Modal: true
Modal->>Desktop: Render DialogRoot + DialogOverlay
User->>Desktop: Interact with dialog
Desktop-->>Modal: Handle dialog events
else Mobile (< md)
Breakpoint-->>Modal: false
Modal->>Mobile: Render Drawer.Root + Drawer.Overlay
User->>Mobile: Interact with drawer
Mobile-->>Modal: Handle drawer events
end
Modal-->>User: Close with consistent API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Modal component that provides adaptive behavior between desktop dialog and mobile drawer implementations based on screen size. The component leverages Radix UI's Dialog primitive for desktop and the Vaul library for mobile drawer functionality.
Key Changes:
- Implemented a responsive Modal component with automatic desktop/mobile adaptation
- Added
useMediaQueryhook with singleton manager pattern for efficient media query handling - Integrated Vaul library for drawer functionality on mobile devices
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added vaul@1.1.2 dependency and updated biomejs package CPU constraints |
| libs/react/ui/package.json | Added vaul ^1.1.1 to dependencies |
| libs/react/ui/src/hooks/useMediaQuery.ts | New hook implementing MediaQueryManager class for shared media query handling |
| libs/react/ui/src/hooks/index.ts | Exported new useMediaQuery hook |
| libs/react/ui/src/components/modal/modal.tsx | Main Modal component with subcomponents for header, body, footer, and responsive breakpoint handling |
| libs/react/ui/src/components/modal/modal.stories.tsx | Storybook examples demonstrating Modal usage patterns |
| libs/react/ui/src/components/modal/index.ts | Barrel export file for Modal component and types |
| libs/react/ui/src/components/index.ts | Added Modal to main components export |
| libs/react/ui/src/components/moving-border/index.ts | Added barrel export for moving-border component |
| libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx | Updated import path for MovingBorder to use new barrel export |
| libs/react/ui/src/components/code-block/code-block-footer.tsx | Refactored to use Text component and improved component composition with better prop types |
| libs/react/ui/index.css | Updated backdrop color from neutral-0 to alpha-black-64 for better visual consistency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
libs/react/ui/index.css(1 hunks)libs/react/ui/package.json(1 hunks)libs/react/ui/src/components/code-block/code-block-footer.tsx(5 hunks)libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx(1 hunks)libs/react/ui/src/components/index.ts(1 hunks)libs/react/ui/src/components/modal/index.ts(1 hunks)libs/react/ui/src/components/modal/modal.stories.tsx(1 hunks)libs/react/ui/src/components/modal/modal.tsx(1 hunks)libs/react/ui/src/components/moving-border/index.ts(1 hunks)libs/react/ui/src/hooks/index.ts(1 hunks)libs/react/ui/src/hooks/useMediaQuery.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
We handle errors at the edge of our applications in most cases. Do not recommend to add error handling around every single function. We prefer them to bubble up and be handled at upper layers.
Files:
libs/react/ui/src/hooks/index.tslibs/react/ui/src/components/moving-border/index.tslibs/react/ui/src/components/modal/index.tslibs/react/ui/src/components/index.tslibs/react/ui/package.jsonlibs/react/ui/src/hooks/useMediaQuery.tslibs/react/ui/index.csslibs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsxlibs/react/ui/src/components/modal/modal.stories.tsxlibs/react/ui/src/components/code-block/code-block-footer.tsxlibs/react/ui/src/components/modal/modal.tsx
🧬 Code graph analysis (2)
libs/react/ui/src/components/modal/modal.stories.tsx (10)
libs/react/ui/src/components/button/button.tsx (1)
Button(41-65)libs/react/ui/src/components/label/label.tsx (1)
Label(13-15)libs/react/ui/src/components/input/input.tsx (1)
Input(24-43)libs/react/ui/src/components/moving-border/moving-border.tsx (1)
MovingBorder(18-67)libs/react/ui/src/components/dynamic-item/dynamic-item.tsx (1)
DynamicItem(23-68)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(71-74)libs/react/ui/src/components/item/item.tsx (1)
ItemTitle(156-169)libs/react/ui/src/components/button/button-link.tsx (1)
ButtonLink(46-76)libs/react/ui/src/components/code-block/code-block.tsx (8)
CodeBlock(38-63)CodeBlockHeader(67-77)CodeBlockFiles(83-93)CodeBlockFilename(99-117)CodeBlockCopyButton(125-148)CodeBlockBody(182-192)CodeBlockItem(199-236)CodeBlockContent(248-283)libs/react/ui/src/components/code-block/code-block-footer.tsx (1)
CodeBlockFooter(15-66)
libs/react/ui/src/components/modal/modal.tsx (3)
libs/react/ui/src/hooks/useMediaQuery.ts (1)
useMediaQuery(71-87)libs/react/ui/src/components/button/button.tsx (1)
Button(41-65)libs/react/ui/src/components/icon/icon.tsx (1)
Icon(71-74)
🔇 Additional comments (10)
libs/react/ui/src/components/code-block/code-block-footer.tsx (1)
103-107: Solid layout improvementAdding
min-w-0 flex-1inside the content wrapper fixes the flex truncation edge case so the message copy now ellipsizes instead of overflowing. Nice catch.libs/react/ui/src/components/moving-border/index.ts (1)
1-1: LGTM!Standard barrel export following the established pattern in the codebase.
libs/react/ui/src/components/dynamic-item/dynamic-item.stories.tsx (1)
4-4: LGTM!Import path updated to use the centralized barrel export, aligning with the new module resolution pattern.
libs/react/ui/index.css (1)
195-195: LGTM - Backdrop visual updated for better modal contrast.The backdrop color in light mode changed from opaque white to semi-transparent black, providing better visual separation for modal overlays. This aligns with common modal design patterns.
libs/react/ui/src/hooks/useMediaQuery.ts (2)
3-67: LGTM - Well-structured media query manager with proper resource cleanup.The MediaQueryManager correctly:
- Caches MediaQueryList instances per query
- Manages per-query listener subscriptions with proper add/remove semantics
- Attaches DOM event listeners only when the first subscriber registers and removes them when the last unsubscribes
- Cleans up all Maps to prevent memory leaks
71-87: LGTM - Hook properly integrates with the manager and handles SSR.The hook correctly:
- Uses lazy initialization to avoid unnecessary work
- Subscribes to query changes via the manager
- Calls
updateMatches()after subscription to ensure sync- Cleans up on unmount or query change
- Works safely in SSR contexts (returns
falsewhenwindowis undefined)libs/react/ui/src/components/index.ts (1)
13-13: LGTM!Modal components now exposed through the public API via the central barrel export.
libs/react/ui/src/hooks/index.ts (1)
2-2: LGTM!
useMediaQueryhook now exposed through the public API via the central barrel export.libs/react/ui/src/components/modal/index.ts (1)
1-23: LGTM!Comprehensive barrel export for the Modal component API, properly separating type exports from value exports and including component primitives, variants, and transitions.
libs/react/ui/package.json (1)
46-46: No action required — vaul version is secure and up-to-date.The latest stable version of the vaul npm package is 1.1.2, and the specified version
^1.1.1will allow updates to patch versions including 1.1.2. No security vulnerabilities were detected for the vaul package.
…' for props and enhance ModalHeader with Text component
…g into feat/add-modal-refactor
|
Now we have screenshot matching, would be great to have a story with the modal open to detect changes in the modal |
|
@noe-charmet All good, ready to approve and merge |
2 participants
Summary by CodeRabbit
New Features
Style
Chores