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

[WP 6.7] Tabs: Unify tab height to 48px/40px #66295

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Oct 22, 2024

Fix the issue discovered in this comment

What?

This PR standardizes the tab height to 48px/40px in the Tabs component.

Why?

This issue is caused by a fluid height being applied to the tabs to prevent overlapping focus styles. In #62027, the vertical padding of the tabs was changed from 3px to space( 4 ) = 16px. This may cause the tab height to exceed 40px or 48px depending on the tab content.

How?

All of these issues have been resolved in the trunk branch. However, since the trunk branch contains various improvements and changes, it is not possible to backport directly from the trunk branch to the wp/6.7 branch.

As I understand it, the Tabs component expected in WP 6.7 is as follows.

  • Horizontal tab (icon):
    • The height is basically 48px.
    • When the tab content is long, the tab height changes fluidly.
    • Focus outlines do not overlap.
  • Vertical tab:
    • the height should be 40px (See #63446)

To achieve these, I will submit the minimum changes to the wp/6.7 branch.

Testing Instructions

  • Horizontal Tab:
    • Open the site editor.
    • Open the editor sidebar.
    • Tabs (Template, Block) height should still be 48px.
    • Select a block that has Settings and Styles tabs.
    • The settings and style tabs height should now be 48px.
  • Horizontal Tab (fluid height):
    • Change the site language to Japanese.
    • Enable "Show button text labels" setting.
    • Open the site editor and select a navigation block.
    • Focus a tab in the block sidebar.
    • The focus outline should not overlap any text.
  • Vertical Tab:
    • Select the Patterns tab from the main inserter.
    • The pattern category button height should still be 40px.
    • Open the settings modal.
    • The tab panel height should now be 40px.

Screenshots

Horizontal tab (icon)

Before After
image image
image image

Horizontal tab (fluid height)

Before After
image image

Vertical tab

Before After
image image
image image

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components labels Oct 22, 2024
@t-hamano t-hamano self-assigned this Oct 22, 2024
@t-hamano t-hamano changed the title [WP 6.7] Tabs: Unify tab height to 48px [WP 6.7] Tabs: Unify tab height to 48px/40px Oct 22, 2024
Copy link

github-actions bot commented Oct 22, 2024

Flaky tests detected in aa452d7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11452376965
📝 Reported issues:

@t-hamano t-hamano requested a review from a team October 22, 2024 03:18
@t-hamano t-hamano marked this pull request as ready for review October 22, 2024 03:18
Copy link

github-actions bot commented Oct 22, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Thank you!

@ciampo ciampo merged commit a98d8ab into wp/6.7 Oct 22, 2024
65 checks passed
@ciampo ciampo deleted the wp6.7/tab-height-focus branch October 22, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants