fix(tabs): update padding and subtab height - #8499
Conversation
WalkthroughTabs templates suppress scroll buttons for non-scrollable lists while retaining disabled-state handling. Tabs styling updates link spacing tokens and adds subtab padding and block-margin variables. ChangesTabs updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview: https://pf-pr-8499.surge.sh A11y report: https://pf-pr-8499-a11y.surge.sh |
|
Sorry just thought of this but we should verify with design if vertical tabs need any updates, too. |
|
|
||
| // Subtab link | ||
| --#{$tabs}--m-subtab__link--MarginBlock: calc(var(--pf-t--global--spacer--control--vertical--plain) - var(--pf-t--global--spacer--control--vertical--compact) + (var(--pf-t--global--font--size--body--default) * var(--pf-t--global--font--line-height--body) - 1lh) / 2); | ||
| --#{$tabs}--m-subtab__link--FontSize: var(--pf-t--global--font--size--xs); |
There was a problem hiding this comment.
While we're here can you update this to use the more semantic token?
| --#{$tabs}--m-subtab__link--FontSize: var(--pf-t--global--font--size--xs); | |
| --#{$tabs}--m-subtab__link--FontSize: var(--pf-t--global--font--size--body--sm); |
|
|
||
| &.pf-m-subtab { | ||
| --#{$tabs}__link--FontSize: var(--#{$tabs}--m-subtab__link--FontSize); | ||
| --#{$tabs}__link--PaddingBlockStart: var(--#{$tabs}--m-subtab__link--PaddingBlockStart); |
There was a problem hiding this comment.
Just an aside - stylelint prefers logical directions in this order.
block-start
block-end
inline-start
inline-end
The order of vars doesn't matter and isn't linted, so you can write them however makes the most sense to you, but when I write new vars, I try to do them in the same order as property declarations just because that's how I've come to expect them in the code.
| --#{$tabs}__add--c-button--FontSize: var(--#{$tabs}--m-subtab__add--c-button--FontSize); | ||
|
|
||
| .#{$tabs}__link { | ||
| margin-block-start: var(--#{$tabs}--m-subtab__link--MarginBlock); |
There was a problem hiding this comment.
typo
| margin-block-start: var(--#{$tabs}--m-subtab__link--MarginBlock); | |
| margin-block-start: var(--#{$tabs}--m-subtab__link--MarginBlockStart); |
mcoker
left a comment
There was a problem hiding this comment.
Looks like a million bucks to me!
lboehling
left a comment
There was a problem hiding this comment.
lgtm! the only thing i see off is that the plain icon buttons in the tabs with actions appear to be stretched vertically... but that appears on tabs as is (not introduced by this PR) so maybe can be handled in a follow up?
|
🎉 This PR is included in version 6.6.0-prerelease.25 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #8253
Updates Tabs padding tokens and subtab.
--pf-t--global--spacer--action--horizontal--plain--plain.--pf-t--global--spacer--control--vertical--compactand--pf-t--global--spacer--action--horizontal--plain--defaultSummary by CodeRabbit