-
Notifications
You must be signed in to change notification settings - Fork 24
feat(Dialog): update Dialog according to P+ specs #4805
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
Conversation
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
Updates the Dialog component and related stories to match Pentaho+ design specs, including icon handling, border styles, and button ordering.
- Removes semantic status bar and top borders in dialogs and actions
- Replaces legacy iconVariant logic with
HvStatusIcon
in titles - Introduces dynamic content borders on overflow, updates dialog border radius, and reorders buttons in stories
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
packages/lab/src/Wizard/WizardTitle/WizardTitle.tsx | Disable default icon rendering in wizard titles |
packages/core/src/themes/pentahoPlus.ts | Add dialog border-radius and remove status bar/border styles |
packages/core/src/themes/ds5.ts | Update dialog title icon color and remove content borders with !important |
packages/core/src/themes/ds3.ts | Remove content borders and update dialog title icon color |
packages/core/src/QueryBuilder/ConfirmationDialog/ConfirmationDialog.tsx | Disable default icon rendering in confirmation dialog title |
packages/core/src/FileUploader/FileUploader.stories.tsx | Disable dialog title icon in file preview dialog story |
packages/core/src/Drawer/Drawer.stories.tsx | Disable dialog title icon in drawer story |
packages/core/src/Dialog/stories/SemanticVariantsStory.tsx | Swap button order, fix title variant, and use HvStatusIcon for custom icons |
packages/core/src/Dialog/stories/MainStory.tsx | Swap actions order in main dialog story |
packages/core/src/Dialog/stories/FormStory.tsx | Swap actions order in form dialog story |
packages/core/src/Dialog/Title/Title.tsx | Import and use HvStatusIcon directly, replacing legacy iconVariant logic |
packages/core/src/Dialog/Dialog.styles.tsx | Remove unwanted border radius for fullscreen dialogs |
packages/core/src/Dialog/Content/Content.tsx | Add dynamic overflow border detection with ResizeObserver |
packages/core/src/Dialog/Content/Content.styles.tsx | Update padding and add border classes for content overflow |
Comments suppressed due to low confidence (1)
packages/core/src/Dialog/Content/Content.tsx:68
- [nitpick] The use of a CSS custom property (
--content-border
) for conditional borders is clever but may be non-obvious. Consider adding a brief comment explaining this pattern to ease future maintenance.
["--content-border" as string]: hasBorder
fae77c5
to
26c9b1b
Compare
@zettca reviewed after feedback |
Notable changes Dialog component in this PR: