feat(tabs): add nav variant - #7924
Conversation
|
Preview: https://pf-pr-7924.surge.sh A11y report: https://pf-pr-7924-a11y.surge.sh |
| @@ -125,7 +125,7 @@ $pf-v6-c-tabs--spacer-map: build-spacer-map("none", "sm", "md", "lg", "xl", "2xl | |||
| --#{$tabs}__link--after--BorderBlockStartWidth: 0; | |||
| --#{$tabs}__link--after--BorderInlineEndWidth: 0; | |||
| --#{$tabs}__link--after--BorderInlineStartWidth: 0; | |||
| --#{$tabs}__item--m-current__link--after--BorderColor: var(--pf-t--global--border--color--clicked); | |||
| --#{$tabs}__item--m-current__link--after--BorderColor: var(--#{$tabs}--link-accent--color); | |||
There was a problem hiding this comment.
Looks like this broke the existing accent borders since these vars now look to be defined by one another.
I don't think we have to fix this in this issue if that part is tricky. We can just theme --pf-v6-c-tabs__item--m-current__link--after--BorderColor instead, which will apply to both animated and default tabs.
| @@ -205,6 +205,12 @@ $pf-v6-c-tabs--spacer-map: build-spacer-map("none", "sm", "md", "lg", "xl", "2xl | |||
| --#{$tabs}__add--PaddingInlineStart: var(--pf-t--global--spacer--sm); | |||
| --#{$tabs}__add--PaddingInlineEnd: var(--pf-t--global--spacer--sm); | |||
|
|
|||
| // Nav variant | |||
| --#{$tabs}--m-nav--inset: var(--pf-t--global--spacer--xl); | |||
| --#{$tabs}--m-nav__link-accent--color: var(--pf-t--color--red--50); | |||
There was a problem hiding this comment.
in the designs, buttons and other tabs are also red, which looks like a separate theme or maybe a change to the brand color tokens. I'd say we can take this out of the default component (and https://github.com/srambach/patternfly/blob/e613d7842f53406daee5d83df3fbeb5e3d676e14/src/patternfly/components/Tabs/tabs.scss#L522) and set this as an override/theme in the demos and mention it in compass docs on how to style it.
| @@ -205,6 +205,12 @@ $pf-v6-c-tabs--spacer-map: build-spacer-map("none", "sm", "md", "lg", "xl", "2xl | |||
| --#{$tabs}__add--PaddingInlineStart: var(--pf-t--global--spacer--sm); | |||
| --#{$tabs}__add--PaddingInlineEnd: var(--pf-t--global--spacer--sm); | |||
|
|
|||
| // Nav variant | |||
| --#{$tabs}--m-nav--inset: var(--pf-t--global--spacer--xl); | |||
There was a problem hiding this comment.
We can also take this out (and https://github.com/srambach/patternfly/blob/e613d7842f53406daee5d83df3fbeb5e3d676e14/src/patternfly/components/Tabs/tabs.scss#L527) and use the inset modifier/prop in our demos.
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
LGTM!
Your last commit allowed setting --#{$tabs}--link-accent--color on regular and animated tabs to theme the accent color, but we lost the ability to set either --#{$tabs}__item--m-current__link--after--BorderColor or --#{$tabs}__item--m-current__link--after--BorderColor on animated tabs to theme it. Since those worked before this change, and would have likely been how someone had themed that accent color, that's breaking.
Pushed an update to address that. It's basically just the way the CSS was originally for the accent color, but now the non-animated tabs use --link-accent--color, so you can set any of those 3 vars above on both animated and non-animated tabs and it themes the accent color.
|
🎉 This PR is included in version 6.5.0-prerelease.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* feat(tabs): add nav variant * feat(tabs): fix circular reference * feat(tabs): remove custom color and inset per comments * chore: update vars * chore: move example under other nav example --------- Co-authored-by: mcoker <[email protected]>
3 participants
Fixes #7911
Also has the old fashioned non-animated tab marker use the tab decoration color that the newer animated version uses.