-
Notifications
You must be signed in to change notification settings - Fork 0
feat: implement drawer component to replace FullscreenModal in EntityBrowser #145
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
base: main
Are you sure you want to change the base?
Conversation
3f39200 to
f971df3
Compare
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.
Pull request overview
This PR implements a custom Drawer component to replace FullscreenModal in EntityBrowser, addressing the need for a slide-in panel UI pattern. The implementation uses Radix Dialog primitives with CSS animations, providing better test compatibility than the previously attempted Vaul library solution.
Key Changes:
- Created a new reusable Drawer component with support for four slide-in directions and comprehensive accessibility features
- Migrated EntityBrowser from FullscreenModal to the new Drawer component with right-side slide-in
- Updated EntityBrowser tests to verify ESC key functionality instead of close button interaction
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/ui/Drawer.tsx |
New drawer component built on Radix Dialog with accessibility, animation support, and configurable behavior |
src/components/ui/drawer.css |
CSS animations and styles for drawer transitions, responsive design, and reduced-motion support |
src/components/ui/index.ts |
Added Drawer to public UI component exports |
src/components/EntityBrowser.tsx |
Replaced FullscreenModal with Drawer, removed close button, adjusted styling for drawer layout |
src/components/__tests__/EntityBrowser.test.tsx |
Updated test to verify ESC key closing behavior instead of close button click |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/components/ui/Drawer.tsx
Outdated
| <Dialog.Portal> | ||
| <Dialog.Overlay | ||
| className="drawer-overlay" | ||
| onClick={closeOnBackdropClick ? () => onOpenChange(false) : undefined} |
Copilot
AI
Dec 4, 2025
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.
The onClick handler on the overlay (line 94) conflicts with the onPointerDownOutside and onInteractOutside handlers on the Dialog.Content (lines 109-110). When closeOnBackdropClick is true, clicking the overlay will trigger both the overlay's onClick (calling onOpenChange(false)) and the Dialog's built-in outside click handlers. This creates redundant close calls.
Remove the onClick handler from the overlay and let Radix UI's built-in outside interaction handlers manage this behavior. They already handle backdrop clicks correctly when onPointerDownOutside and onInteractOutside are not prevented.
| onClick={closeOnBackdropClick ? () => onOpenChange(false) : undefined} |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Summary
Related Issues
Closes #143 (Find alternative drawer solution for EntityBrowser)
Addresses #136 (Fullscreen modal should only be used for CameraCard)
Context
This PR replaces the failed Vaul-based implementation (PR #142) with a custom drawer solution. The Vaul library had significant test environment compatibility issues that made it unsuitable for our needs.
Implementation Details
Custom Drawer Component (
src/components/ui/Drawer.tsx)Key Features
EntityBrowser Updates
Testing
npm test)npm run lint)npm run typecheck)Test Evidence
All tests pass without any failures. The drawer component works seamlessly in both development and test environments, completely avoiding the jsdom compatibility issues we encountered with the Vaul library in PR #142.
Breaking Changes
None - This is an internal UI component change that maintains the same external API.
Implementation Notes