promote dialog and overlay headers to core #4655
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Fercas123
force-pushed
the
headers-core-promotion
branch
2 times, most recently
from
deb8713 to
0b5953e
Compare
Fercas123
force-pushed
the
headers-core-promotion
branch
from
717ebb7 to
5c3c7d0
Compare
Fercas123
force-pushed
the
headers-core-promotion
branch
from
4e728bb to
8a71815
Compare
Fercas123
force-pushed
the
headers-core-promotion
branch
from
8a71815 to
8ec5792
Compare
| }); | ||
|
|
||
| it("THEN should support initialFocus being set", () => { | ||
| cy.mount(<Default initialFocus={2} />); | ||
| cy.mount(<Default initialFocus={3} />); |
There was a problem hiding this comment.
worth to point out potential change in changeset
| } | ||
|
|
||
| .saltDialogHeader-header > .saltText { | ||
| margin: 0; |
There was a problem hiding this comment.
This feels magical, is this for global heading style
| preheader={preheader} | ||
| description={description} | ||
| actions={<CloseButton />} | ||
| />{" "} |
There was a problem hiding this comment.
Need to make sure DialogCloseButton would still work with the new header (non breaking).
| @@ -1,22 +1,38 @@ | |||
| /* Styles applied to the root element */ | |||
| .saltDialogHeader { | |||
| padding-bottom: var(--salt-spacing-100); | |||
| padding-bottom: var(--salt-spacing-300); | |||
There was a problem hiding this comment.
Changeset, this was highlighted by the community, that their existing header looks different than what's on the documentation, because our Site loads the lab CSS which overrides core
| import { useComponentCssInjection } from "@salt-ds/styles"; | ||
| import { useWindow } from "@salt-ds/window"; | ||
| import clsx from "clsx"; | ||
| import { clsx } from "clsx"; |
| @@ -93,11 +94,20 @@ NoStyleInjectionGrid.parameters = { | |||
| }; | |||
|
|
|||
| export const CloseButton: StoryFn<QAContainerProps> = (props) => { | |||
| const CloseButton = () => ( | |||
There was a problem hiding this comment.
may trigger lint error double CloseButton definition
| @@ -152,9 +168,19 @@ export const NoStyleInjectionCloseButton: StoryFn< | |||
| <Button>Show Overlay</Button> | |||
| </OverlayTrigger> | |||
| <OverlayPanel> | |||
| <OverlayPanelCloseButton /> | |||
There was a problem hiding this comment.
Similar to dialog, need to keep at least 1 OverlayPanelCloseButton for backwards compatibility check
| @@ -18,6 +17,7 @@ import { | |||
| useState, | |||
| } from "react"; | |||
| import "./dialog.stories.css"; | |||
| import { CloseIcon } from "@salt-ds/icons"; | |||
There was a problem hiding this comment.
Is it worth to use icon from useIcon rather than import from icons package directly, to show the best practice?
| <LivePreview componentName="dialog" exampleName="Preheader" > | ||
| ## Preheader | ||
|
|
||
| You can use the optional `preheader` prop to render a preheader above the main header of your dialog | ||
|
|
||
| </LivePreview> | ||
|
|
||
| <LivePreview componentName="dialog" exampleName="CloseButton" > | ||
| ## Close button |
There was a problem hiding this comment.
If this is solely for DialogCloseButton, maybe add "deprecated" to the heading text as well. like button variants, add move to bottom of examples
No description provided.