-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tabs: unify vertical tabs styles #65387
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: -2.79 kB (-0.16%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
@WordPress/gutenberg-design for now I've only removed style overrides. Here's the inserter > patterns Before (with overrides): After (without overrides): The main differences I spot in the previous style overrides vs default
For reference, here's the preferences dialog tabs, which already don't have any style overrides: These are the only two instances of Tabs at the moment in Gutenberg. Ideally, we'd find styles that work in both cases. If necessary, we can consider introducing some sort of variant, but let's try to keep that at a minimum and find something that we feel good about regarding future usage of the component, to prevent more style overrides in the future. Please leave some feedback, happy to implement any ideas so we can test them! |
+100 to removing the overrides. That said, I think the appearance in the Editor is the direction we want to go in. So would it make sense to update the component first, then remove the overrides after? That way there should be minimal visual disruption. One detail we need to consider is that the color change (gray or blue) is of inadequate contrast to meet requirements, so we need another way to communicate which tab is active. We could address this while updating the component. A good place to start could be making the active tab font weight slightly heavier. What do you think? |
Well, the thing is we only have two instances of vertical tabs, so what you're proposing doesn't make much sense purely in terms of the sequence of steps hehe. If, instead of removing overrides, I update the base component, then the preferences tabs will be affected, so either way there'd be visual disruption. Here's how I think this PR should evolve:
I hope that makes sense to you? Regarding your suggestion that the styles of the inserter tabs are closer to what we want, that makes sense to me. I'll tweak the base component, so both instances will look closer to it. In parallel, I will also apply your suggestion about the active tab styles. Then, we can take it from there. Sound good? :) |
@DaniGuardiola In my experience we don't tend to update multiple things in a single PR like you're suggesting, hence my idea to update the component first. That said, it does make sense to me in this context, providing other folks in the @WordPress/gutenberg-components team agree. |
…mprove/unify-vertical-tabs-styles
Currently, the weight for tabs is 500. In the inserter, it was overridden to 400. What if we:
This is how that looks like in inserter and preferences: font-weight.mp4I also tried making the active tab have the theme accent color (blue in this case) like the inserter style overrides did. For reference, this is the same color that tabs have when hovered. Here's how it looks like: active-color.mp4I quite like this with the light gray indicator. However, it might still not be enough for colorblind or low-vision people. Here, I tried making the active tab have a weight of 600 (on top of all previous changes): Kapture.2024-09-19.at.05.55.38.mp4And here's with 700: Kapture.2024-09-19.at.05.57.31.mp4I personally feel much more comfortable shipping this with the increased font weights. Thoughts? |
By the way, the font weight and text color changes also affect horizontal tabs, as you can see in the videos. That said, we could always special-case it to vertical tabs if we don't want that to happen. Maybe worth doing it in both for consistency though, it communicates more clearly that both (vertical and horizontal) are the same component rather than different ones. |
I reckon we should only apply the weight change to vertical tabs, if possible. Horizontal tabs are fine by virtue of the 'active' shape having enough contrast. In #64340 (comment) we're leaning towards Here's a visual for the different states, hopefully this communicates things better than words :D |
You can see this in my previous comment, it's the first example. Qs:
For now, I'm gonna assume that the answer is yes to points 1 and 2 and push those changes in this PR. FYI, there's a problem that #64371 fixes that makes the indicator not show up in the inserter, in case you wanted to try it locally. I'm performing a quick hack to get these screenshots and videos for now, but you won't be able to test it locally for now. Hopefully we'll be able to merge that soon though. Let me know if you want me to record or capture something in the meantime. |
I applied all changes, including the 400/500 weight change, and here's how that looks like currently. 500 (semibold) still seems too low for me, especially with the light blue indicator which looks even dimmer than the previous gray one to me. Kapture.2024-09-20.at.02.23.19.mp4 |
You may be right about the weight, but the more we increase it the more legibility in certain languages decreases. I think we should proceed with We can gather feedback and open a dedicated issue for this detail. There are other options to explore like adding a shape to the active tab: It's not something that needs to block the other enhancements here, and needs broader visibility to determine the best direction. |
One small detail; the radius on the active tab should be |
@jameskoster various thoughts:
|
What?
Unifies the styles of vertical tabs across Gutenberg.
Why?
See #64307
How?
By removing style overrides and tweaking the
Tabs
component to adapt to all needs.Testing Instructions
Try the inserter patterns/media vertical tabs.
Screenshots or screencast
TODO