search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

Tabs: icon tabs and text tabs have a different height #65624

Closed ciampo closed 1 month ago

ciampo commented 1 month ago

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.

ciampo commented 1 month ago

cc @WordPress/gutenberg-components @WordPress/gutenberg-design

jasmussen commented 1 month ago

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:

Screenshot 2024-09-25 at 10 41 21

jameskoster commented 1 month ago

If we used min-height and removed vertical padding I think that would solve this issue? Appreciate it wouldn't account for children taller than 48px, but that feels like an edge case to me.

ciampo commented 1 month ago

We could probably just remove the vertical padding and rely on flexbox alignment ?

patil-vipul commented 1 month ago

Raising a PR as per the suggestion from @ciampo in sometime.

cc: @ciampo @jameskoster @jasmussen

DaniGuardiola commented 1 month ago

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 commented 1 month ago

This will be fixed by https://github.com/WordPress/gutenberg/pull/65387

ciampo commented 1 month ago

Closing as fixed in https://github.com/WordPress/gutenberg/pull/65387

t-hamano commented 4 weeks ago

In WordPress 6.7 Beta3, I noticed that the icon tabs are 56px tall:

https://playground.wordpress.net/?wp=beta

Image

Whereas in WordPress 6.6, it is 48px:

https://playground.wordpress.net/?wp=6.6

Image

Maybe a special backport is needed for WordPress 6.7?

ciampo commented 4 weeks ago

Yup, feel free to backport the fix PR (and any dependent PRs)!

Otherwise I'll work on it on Monday

ciampo commented 3 weeks ago

@t-hamano I'm actually not sure that backporting the PR that fixed this issue is a good idea — the fix is included in https://github.com/WordPress/gutenberg/pull/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:

Separately, 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 commented 3 weeks ago

@ciampo I have submitted a minimal PR for WP 6.7.

Separately, we may also want to backport these other fixes:

What do you think?

Are these two bugs first introduced in WP 6.7? If not, I don't think a backport is necessary.