Repository navigation
Tabs: icon tabs and text tabs have a different height #65624
Description
Text tabs (ie. <Tabs.Tab>text</Tabs.Tab>) are 48px tall: https://wordpress.github.io/gutenberg/?path=/story/components-experimental-tabs--default
Icon tabs (ie. <Tabs.Tab><Icon /></Tabs.Tab>) are 56px tall: https://wordpress.github.io/gutenberg/?path=/story/components-experimental-tabs--with-tab-icons-and-tooltips
Should we revise any aspects of this? It may be tricky to enforce it fully with the current set of APIs, since Tabs can not possibly know what its children are.
Activity
ciampo commentedon Sep 24, 2024
cc @WordPress/gutenberg-components @WordPress/gutenberg-design
jasmussen commentedon Sep 25, 2024
They should both have the same height, of course. Probably best to maintain the 48px height for now, since it affects a key visual of the editor:
jameskoster commentedon Sep 25, 2024
If we used
min-heightand removed vertical padding I think that would solve this issue? Appreciate it wouldn't account for children taller than48px, but that feels like an edge case to me.ciampo commentedon Sep 25, 2024
We could probably just remove the vertical padding and rely on flexbox alignment ?
patil-vipul commentedon Sep 25, 2024
Raising a PR as per the suggestion from @ciampo in sometime.
cc: @ciampo @jameskoster @jasmussen
DaniGuardiola commentedon Sep 25, 2024
The Tabs component is currently heavily in flux, so please hold off on working on this until a few tweaks have been merged. I believe one of the pending PRs fixes this already.
DaniGuardiola commentedon Oct 4, 2024
This will be fixed by #65387
ciampo commentedon Oct 17, 2024
Closing as fixed in #65387
t-hamano commentedon Oct 19, 2024
In WordPress 6.7 Beta3, I noticed that the icon tabs are
56pxtall:https://playground.wordpress.net/?wp=beta
Whereas in WordPress 6.6, it is
48px:https://playground.wordpress.net/?wp=6.6
Maybe a special backport is needed for WordPress 6.7?
ciampo commentedon Oct 19, 2024
Yup, feel free to backport the fix PR (and any dependent PRs)!
Otherwise I'll work on it on Monday
ciampo commentedon Oct 21, 2024
@t-hamano I'm actually not sure that backporting the PR that fixed this issue is a good idea — the fix is included in #65387, which introduced substantial changes to the component. In order to merge #65387, we'd likely need to merge a few dependent PRs before AND a few follow-up PRs that fixed a couple of regressions.
A potential list of backports could be:
Tabs: tweak sizing and overflow behavior of TabList #64371Separately, we may also want to backport these other fixes:
What do you think?
As an alternative, maybe we could apply a specific hotfix just for the 6.7 release?
t-hamano commentedon Oct 22, 2024
@ciampo I have submitted a minimal PR for WP 6.7.
Are these two bugs first introduced in WP 6.7? If not, I don't think a backport is necessary.