Updated tab styling - #3351
Conversation
|
Pull request environment is available at https://stoctodocspr3351.z22.web.core.windows.net. You can view the ephemeral environment status in Octopus Deploy. This environment will be automatically deprovisioned when the pull request is closed, or after 7 days of inactivity. |
rosslovas
left a comment
rosslovas
left a comment
There was a problem hiding this comment.
Brilliant, that had been bugging me too! Looks so much better, and changes all look reasonable, I just spotted a few odd things. Ready to approve if you think none of these are worth addressing, just let me know; I only held off cause I noticed this was on auto-merge, just in case you did want to change anything 🙂
I'm also not sure we have anywhere that actually exercises this, but I tested it and found that too many tabs will overflow now instead of resulting in another row as they used to. Not sure how much that matters but thought I'd point it out regardless.
Before:
After:
| :is( | ||
| :is( |
There was a problem hiding this comment.
Here and in the one below, the double :is( I believe is unnecessary and doesn't change anything, correct me if I'm wrong though
| .tab-list button[aria-selected='true'] { | ||
| margin-block-start: 0; | ||
| color: var(--colorTextPrimary); | ||
| bottom: calc(-1 * var(--borderWidth1)); |
There was a problem hiding this comment.
I don't think this bottom is actually doing anything because we have top: 0 above, and I believe it takes precedence
| .tab-list button:focus-visible { | ||
| z-index: 4; | ||
| outline: var(--borderWidth2) solid var(--colorBorderSelected); | ||
| border-radius: var(--borderRadiusExtraSmall); |
There was a problem hiding this comment.
This reads to me like it's trying to set borderRadiusExtraSmall on focused-but-not-selected tabs, but it seems like it's impossible to actually do that, as I was only ever able to focus the selected tab. As a result, and due to the fact that this rule comes after the .tab-list button[aria-selected='true'] rule, the only impact this has is that the selected tab's border radius becomes smaller when you use the keyboard to switch tabs. It's very subtle so people aren't likely to notice but I wasn't sure, was that intended?
3 participants
Summary
This is a tab styling fix.
Before and after
Bonus: Added a proper focus state