fix(components): various fixes for banner, timestamp, and content dl#6953
fix(components): various fixes for banner, timestamp, and content dl#6953mcoker merged 7 commits intopatternfly:v6from
Conversation
|
Preview: https://patternfly-pr-6953.surge.sh A11y report: https://patternfly-pr-6953-a11y.surge.sh |
There was a problem hiding this comment.
Looks good! Just a wee bit of feedback, nothing is blocking though if you'd prefer merge this as-is and follow up. Just lemme know if you'd prefer do that.
| grid-template: auto / auto 1fr; | ||
| grid-template: auto / 12ch 1fr; | ||
| grid-row-gap: var(--#{$content}--dl--RowGap); | ||
| grid-column-gap: var(--#{$content}--dl--ColumnGap); |
There was a problem hiding this comment.
The column gap for <dl>'s is 2xl and the row gap is md. Do we want to update that? Some thoughts:
- In figma, the description list default column gap is sm, and row gap is lg. The compact description list column gap is sm, and row gap is md
- Currently our description list component's column and row gaps match what's in figma
- We have gap spacer tokens here -
patternfly/src/patternfly/base/tokens/tokens-default.scss
Lines 467 to 474 in bdd2cc6
- For the default
<dl>we could usegap--text-to-elementandgap--group-to-group--vertical? I'm not 100% those are the right tokens though. - There are no compact variants of gap tokens.
- If we update to gap spacer tokens, should we also update the description list component?
- For the default
There was a problem hiding this comment.
LGTM! Just want to confirm that we don't want to update the column/row gap for the content component <dl>? IMO it looks like there is too much space between term and description. Here's what it is currently:
And here's the horizontal description list component's column/row gap:
There was a problem hiding this comment.
LPTM! Backstop report if you're interested. zandrew-the-true-king-of-the-north.psd.v2-final.ammend.client-approved.pdf
Some notes about the report:
- Skip to content diff is just spacing from the content component
<dl>on that example - Label group diff - font (not)awesome didn't load
- Card with the missing kebab - icon font didn't load
- Wizard example with missing reference file - I think that's my oopsie from a previous visual regression report
|
🎉 This PR is included in version 6.0.0-alpha.224 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #6907, #6729 and #6871