feat(components/tabs): tokenize tabs styling #2850
Conversation
Blackbaud-ErikaMcVey
added
the
risk level (author): 3
☁️ Nx Cloud ReportCI is running/has finished running commands for commit c4b05ad. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
| --sky-override-tab-button-radius: 0; | ||
| --sky-override-tab-line-height: 1.8; | ||
| } | ||
|
|
||
| .sky-dropdown-button-type-tab { | ||
| @include mixins.sky-btn-tab(); |
There was a problem hiding this comment.
Should we just move the contents of these mixins here? This is the last places these mixins are used.
There was a problem hiding this comment.
We could also then eliminate the import all together in this file.
There was a problem hiding this comment.
I can move those if we'd like--these are the default styles though. Did you want to change those?
There was a problem hiding this comment.
I don't necessarily want to change them - but we talked about removing the mixins. And moving the styles from the mixin here would help prepare us for that.
| .sky-tabset { | ||
| border-bottom: 1px solid $sky-theme-modern-border-color-neutral-medium; | ||
| overflow: visible; /* for drop shadow visibility on focused tabs */ |
There was a problem hiding this comment.
It looks like we already have an overflow specification for sky-tabset in the default styles - could we use an override of some sort to move this last style and eliminate the modern section all together?
| @@ -148,6 +148,7 @@ a.sky-btn { | |||
| font-weight: 400; | |||
| color: var(--sky-text-color-default); | |||
| line-height: 1.6em; | |||
| --sky-override-tab-line-height: 1.6em !important; | |||
There was a problem hiding this comment.
Sorry if I'm missing something - why are we doing this here instead of in a default override in a component somewhere? We are doing the modern version in the component.
There was a problem hiding this comment.
The tabs have a different line height in gemini vs modern and in tabs vs wizard, so modern always has an override. This was the bare minimum to keep modern wizard buttons working before we tokenize them in their own story. I think specificity changed when removing the styles from the mixin, because this no longer takes effect over the line height set for regular tabs without the !important. I can find a cleaner approach if you'd like.
There was a problem hiding this comment.
It just felt odd to have an override variable just hanging out here outside of an override section. I'd like to clean that up if possible - but I won't die on that hill.
No description provided.