Skip to content
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: icon tabs and text tabs have a different height #65624

Closed
ciampo opened this issue Sep 24, 2024 · 12 comments
Closed

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

ciampo opened this issue Sep 24, 2024 · 12 comments
Labels
Needs Design Feedback Needs general design feedback. [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@ciampo
Copy link
Contributor

ciampo commented Sep 24, 2024

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 ciampo added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Package] Components /packages/components labels Sep 24, 2024
@ciampo
Copy link
Contributor Author

ciampo commented Sep 24, 2024

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

@jasmussen
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

ciampo commented Sep 25, 2024

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

@patil-vipul
Copy link

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

cc: @ciampo @jameskoster @jasmussen

@DaniGuardiola
Copy link
Contributor

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
Copy link
Contributor

This will be fixed by #65387

@ciampo
Copy link
Contributor Author

ciampo commented Oct 17, 2024

Closing as fixed in #65387

@ciampo ciampo closed this as completed Oct 17, 2024
@t-hamano
Copy link
Contributor

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
Copy link
Contributor Author

ciampo commented Oct 19, 2024

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

Otherwise I'll work on it on Monday

@ciampo
Copy link
Contributor Author

ciampo commented 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:

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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

6 participants