Conversation
PR Reviewer Guide 🔍(Review updated until commit 7bcf81b)Here are some key observations to aid the review process:
|
|
📦 Bundle Size Analysis ✅ No bundle size changes detected. Unchanged Components
📊 Summary:
|
rivka-ungar
left a comment
There was a problem hiding this comment.
👑 👑 👑
Should it be in the separate dialog package?
| const EMPTY_OBJECT: CSSProperties = {}; | ||
| const ESCAPE_KEYS = [keyCodes.ESCAPE]; | ||
|
|
||
| export interface DialogContentProps extends VibeComponentProps { |
There was a problem hiding this comment.
Should the types be in a separate types file?
packages/core/src/components/next/Dialog/hooks/__tests__/useDisableScroll.test.ts
Show resolved
Hide resolved
packages/core/src/components/next/Dialog/components/DialogContent/DialogContent.module.scss
Outdated
Show resolved
Hide resolved
In v4 yes, I just didn't want to move it to @vibe/dialog as next, so it will replace the existing @vibe/dialog in the next major |
User description
https://monday.monday.com/boards/3532714909/pulses/5319093293
PR Type
Enhancement
Description
Migrate Dialog component to next folder with floating-ui
Replace Popper.js with Floating UI for positioning
Add comprehensive Dialog types and props interface
Implement scroll disable hook and DialogContent component
Add extensive test coverage for Dialog functionality
Diagram Walkthrough
File Walkthrough
10 files
Define comprehensive Dialog types and propsImplement Dialog component with Floating UIAdd Dialog arrow and positioning stylesExport Dialog component and typesImplement scroll disable hook for dialogsImplement DialogContent with animationsAdd DialogContent animation and padding stylesCreate Refable utility for ref forwardingExport Dialog from next componentsUpdate Dialog stories to use Floating UI4 files
Add tests for useDisableScroll hookUpdate useDisableScroll test import pathAdd comprehensive tests for Refable componentAdd extensive Dialog component tests1 files
Add floating-ui dependency to core