New Header for Overlay and Dialog #4138
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
tomhazledine
force-pushed
the
overlay-and-dialog-headers
branch
2 times, most recently
from
d2d3fd4
to
ba95ab9
Compare
| /** | ||
| * Description text is displayed just below the header | ||
| **/ | ||
| description?: string; |
There was a problem hiding this comment.
There was a question on the previous PR about why description|header|preheader were limited to strings. (same applies to DialogHeader)
IMO string is the right choice here. The design is very deliberate about only allowing text content in these sections, and I don't think there's a scenario where we would want users to be able to inject any component they like here.
@joshwooding @origami-z @navkaur76 thoughts?
There was a problem hiding this comment.
In most cases, pure string would be provided anyway. However, in case teams want to put additional layout, icon or other relevant stuff, we could try to accommodate? A few other components does this
There was a problem hiding this comment.
I would rather use ReactNode or something similar. It's not worth being restrictive
| box-sizing: border-box; | ||
| } | ||
|
|
||
| .saltOverlayHeader-body { |
There was a problem hiding this comment.
@joshwooding following up on question from previous PR: "Is "body" the correct way to refer to this?"
I think it is, as it's the body of the header (multiple text elements that are wrapped together, discrete from the end adornment). Not a strongly-help opinion, though, so happy to hear counter arguments 😄
There was a problem hiding this comment.
It needs to be aligned with design @navkaur76
There was a problem hiding this comment.
In figma, the frame containing the main heading, pre-header and description is called "Block", if that helps? The accent bar sits outside of this frame.
There was a problem hiding this comment.
Closest block in the system is in stepper input?
| /** | ||
| * End adornment component | ||
| */ | ||
| endAdornment?: ReactNode; |
There was a problem hiding this comment.
Using endAdornment to be consistent with other components. Are there any strong opinions about the name? @joshwooding @origami-z
There was a problem hiding this comment.
adornment is really only form controls previously... how does design refer to this? @navkaur76
There was a problem hiding this comment.
It's "Close button" in figma
There was a problem hiding this comment.
Adornment is also used in grid corner triangle? @bhoppers2008
Fercas123
force-pushed
the
overlay-and-dialog-headers
branch
from
ba95ab9
to
744b89a
Compare
|
Replaced by other PRs. |
OverlayHeadercomponent to replace deprecatedOverlayPanelCloseButtonDialogHeaderto match updated Figma spec and deprecateDialogCloseButton