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

Global styles panel header buttons text overlaps when 'Show button text labels' is enabled #61761

Open
afercia opened this issue May 17, 2024 · 9 comments
Labels
[Feature] Show button text labels A preference in the Post and Site Editor that makes buttons show text instead of icons [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Edit Site /packages/edit-site [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented May 17, 2024

Description

When the 'Show button text labels' preference is enabled, the text of the buttons in the Styles panel header overlaps, making thte text barely readable.

Screenshot from trunk:

trunk

This appears to be a regression from WordPress 6.5, where the text is readable. Screenshot:

wp 6 5

I think the recent addition of the is-compact CSS class and styling to the buttons hasn't been tested for 'Show button text labels'. Regardless, the buttons text should never overlap.

Worth noting the horizontal space in the panel header is very limited. Placing there a title, 3 action buttons and a close button doesn't seem ideal in the first place.

On the other hand, the 'Show button text labels' preference seems to be often neglected and missed during the design and testing phase. I'd love to see this feature being treated as a first-class citizen in this project at the point that I'd think anydesign for new feature should provide design and mockups for 'Show button text labels' as well.

Note: I created a new GitHub label [Feature] Show button text labels, similarly to other used preferences like [Feature] Distraction Free to better track issues related to it. Thre have been several issues and PRs to remediate to breakages of buttons with this prefrence enabled that are all scattered around and difficult to search for.

Step-by-step reproduction instructions

  • Go to the Site editor > Options > Preferences > Accessibility, and enable the 'Show button text labels' preference.
  • Open the Styles panel.
  • Observe the buttons text overlaps.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Type] Regression Related to a regression in the latest release [Package] Edit Site /packages/edit-site [Feature] Show button text labels A preference in the Post and Site Editor that makes buttons show text instead of icons labels May 17, 2024
@jasmussen
Copy link
Contributor

What design feedback are you looking for?

@afercia
Copy link
Contributor Author

afercia commented May 21, 2024

@jasmussen as I see it:

  • The root problem is that all those button shouldn't be placed there in the first place.
  • I'd appreciate the design team to provide design and mockups also for the 'Show button text labels' preference, when relevant, as this is an integral part of the interface.

Too many buttons in a panel header that shoud likely only contain the title and the X close button:

Screenshot 2024-05-21 at 10 04 39

@jasmussen
Copy link
Contributor

Ah, I agree, the text-labels only mode is in a poor state, the block toolbars especially need a lot of attention, and many missing pieces in the inspector. I'm asking because this seemed to report mainly a regression, which is hopefully easy to fix in time for 6.6. Is this meant to be a larger issue around the expanded topics? In that case, how do you feel about dropdowns for handling overflow?

@afercia
Copy link
Contributor Author

afercia commented May 22, 2024

I think the only real solution to this problem is to ust stop placing buttons in the panels header.
See also #61553 Ideally, the panels header should only contain:

  • The panel title
  • The Close button, if any

Other controls should not be placed there because the available space is not enough when 'Show buttons text labels' is enabled. The length of the visibel text is really unpredictable. As such, the UI of the panels should provide another spot where visibel text can be fully readable and flow as necessary.

I think this specific regression can be remediated in time for 6.6 but a real solution would be to make any desing take into account the 'Show buttons text labels' preference.

the text-labels only mode is in a poor state

Unfortunately, yes. I'd love to see it prioritized, added to design guidelines and be reviewed for any issue/PR that touches the related sections of the UI.

@afercia
Copy link
Contributor Author

afercia commented May 27, 2024

Of course, the problem is even more noticeable with languages other than English, as they often use longer strings. E.g.: Italian:

Screenshot 2024-05-27 at 15 54 29

Not sure a CSS fix would be sufficient in this case. Wondering whether the panels should have a 'subheader'.

@afercia
Copy link
Contributor Author

afercia commented Jun 10, 2024

In #61726 the LinkControl preview has been improved to show the text buttons in a second row, when the 'Show buttons text label' preference is enabled. Animated GIF to illustrate:

linkcontrol show button text label

In a similar way, I would like to propose to change the panel's header to show a 'subheader' when the preference is enabled and render the text buttons there.

@DaniGuardiola
Copy link
Contributor

@jasmussen would it be okay to take the "subheader" approach when the setting is enabled, as suggested by @afercia? I personally find it to be a reasonable compromise.

@jasmussen
Copy link
Contributor

At this point, anything is better than what we have, surely. But it's not clear to me that adding a subheader would solve the issue at hand here. If anything, allowing the header to wrap would be more scalable, which, of course, would be a terrible experience.

That's why we need to be better at handling overflow in a more general sense. Take this example, the block toolbar is cropped on both sides of the viewport, that's not going to be useful, but the solution here isn't to limit what plugins can add to a block toolbar, because even if we set a max of n buttons there, they could still provide long descriptive actions. Even then, there are translations, where german adds on average, 20%.

So we need some way to handle overflow, and while it can be on a case by case basis, I'd recommend instead thinking of these primitives as part of the WordPress component system, and then providing tools and best practices for how to leverage them. Even if we handle all the edge-cases in the block editor, that won't benefit plugins.

@afercia
Copy link
Contributor Author

afercia commented Jul 8, 2024

Looking into this issue a bit more in depth, I think there are two separate issues:

1
The size of the buttons in the complementary area header changed in #61331 and it's now compact. That's OK. However, I'm not sure why the Close button is now small.

  • The different size is very evident when focusing the buttons. See screenshot below.
  • Szes that provide a bigger target size (as much as possible) should be always preferred.

For consistency and better usability, I'd make the X Close button compact, not sure why it needs to be small.
Cc @richtabor

Screenshot: the Styles panel and the Plugins panel:

inconsistent size

2
The second issue is that the compact and small sizes of the Button component are the only ones to use a fixed width. See the related CSS:

&.is-compact {
height: $button-size-compact;
&.has-icon:not(.has-text) {
padding: 0;
width: $button-size-compact;
min-width: $button-size-compact;
}
}
&.is-small {
height: $button-size-small;
line-height: 22px;
padding: 0 8px;
font-size: 11px;
&.has-icon:not(.has-text) {
padding: 0;
width: $button-size-small;
min-width: $button-size-small;
}
}

The fixed width is only applied when the buttons use an icon and don't contain text. However, the current implementation of 'Show button text labels' doesn't use text. It uses CSS generated content. By setting a fixed width, these sizes aren't compatible with 'Show button text labels'.

I'm not sure why a fixed width is necessary in the first place. It appears the first fixed width was added long time ago for the small size in #19344

I'd tend to think all fixed widths should be removed, they seem unnecessary to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Show button text labels A preference in the Post and Site Editor that makes buttons show text instead of icons [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Edit Site /packages/edit-site [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

No branches or pull requests

3 participants