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

View Page and Toggle Zoom Out buttons should have width `auto` when showIconLabels is true #65491

Closed afercia closed 1 month ago

afercia commented 1 month ago

Description

See https://github.com/WordPress/gutenberg/pull/65183 See https://github.com/WordPress/gutenberg/issues/65311 See https://github.com/WordPress/gutenberg/pull/65404

When enabling the 'Show button text labels' preference, most buttons in the top bar and block toolbars should have a width auto to show the text labels in a single line. View Page and Toggle Zoom Out don't. Screenshot:

Screenshot 2024-09-19 at 15 51 53

It would be great to have a more reliable mechanism to set the width auto to icon buttons that reveal their text when showIconLabels is true.

Also, it would be great that any new design and UI change is designed taking into account showIconLabels and tested for it. Cc @WordPress/gutenberg-design

The 'Zoom out` feature is now not flagged behind an experiment any longer so this appeear something to fix soon.

Additionally, I'd like to remind everyone that the verb 'Toggle' must be avoided as it's not well translatable in many languages. See https://github.com/WordPress/gutenberg/discussions/42492 See https://core.trac.wordpress.org/ticket/34753

Cc @WordPress/gutenberg-core

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

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

afercia commented 1 month ago

One more issue with the 'Toggoel Zoom Out' is that buttons should not show a tooltip with the same text of the visible text. In fact, for most buttons the tooltip is conditional and doesn't show up when the label is visible.

Screenshots; In the Site Editor and Post Editor, the tooltip just repeats the visible text:

Post editor:

Screenshot 2024-09-19 at 16 13 44

Site editor:

Screenshot 2024-09-19 at 16 14 12

afercia commented 1 month ago

Now that https://github.com/WordPress/gutenberg/pull/65573 has been merged, I submitted a quick PR to remove the verb Toggle,

Besides making the string better translatable, shortening the label helps not breaking the layout when the 'Show button text labels' preference is enabled. Screenshot with the WP admin language set to English:

Screenshot 2024-09-24 at 13 33 48

However, other languages provide longer strings so the layout isn'g great yet. I stand corrected with my previous description as it seems the widht auto is set correctly on this buttons when showIconLabels is true. It appears it's the grid layout of the container that doesn't take into account longer strings so it's a broader issue that should be addressed on the CSS side.

Screenshots in other languages:

Screenshot 2024-09-24 at 13 35 17

Screenshot 2024-09-24 at 13 36 02

aaronrobertshaw commented 1 month ago

I stand corrected with my previous description as it seems the widht auto is set correctly on this buttons when showIconLabels is true.

I can confirm the buttons in the editor header have width: auto already applied when showIconLabels is set to true.

Image

It appears it's the grid layout of the container that doesn't take into account longer strings so it's a broader issue that should be addressed on the CSS side.

This looks to be because the buttons in question don't enforce white-space: nowrap.

Buttons for pinned plugin sidebars are made tertiary buttons which come with white-space: nowrap set. They also get some padding that buttons like Zoom out and View don't. Maybe we could get some design direction on exactly how we want all the buttons here to end up.

Image

The possibility of an unknown number of pinned plugins could make a completely clean solution tricky. For now though creating a bunch of test puglin sidebars, it just reduces the available space for the center document bar.

The document tools on the left of the editor header mostly get hidden at smaller viewports, so do not seem as susceptible to wrapping.

I drafted a quick PR that simply adds white-space: nowrap for buttons in the editor header when showIconLabels is true. It seems to do the job.

While testing this, I noticed the buttons in the Global Styles sidebar's header are also wrapping and overlapping when showIconLabels is true. The available real estate there is unlikely to change and I have some vague recollections around components in the editor needing to remain unaware of the showIconLabels preference. That could prevent doing something like; if the preference is enabled, move the Style Book, and revisions buttons into the more menu there. Figured it was worth raising in case others have ideas while looking at this issue.

afercia commented 1 month ago

@aaronrobertshaw thanks for looking into this.

Personally, I think the only way to improve showIconLabels is that any new design should provide a design for this preference too. The current state of the UI when showIconLabels is true is honestly a poor state. Designing for it should be an integral part of the design phase.

This looks to be because the buttons in question don't enforce white-space: nowrap

Yes that's one of the issues. It would be interesting to understand why the default button doesn't use white-space: nowrap while the primary, secondary, and tertiary variants do. I do understand the link variant should allow text to wrap, but why the default does? GIF to illustrate: Cc @WordPress/gutenberg-components

Image

The possibility of an unknown number of pinned plugins could make a completely clean solution tricky. For now though creating a bunch of test puglin sidebars, it just reduces the available space for the center document bar.

Regarding, the document bar, I'm proposing to entirely rethink it. Its implementation is less than ideal from an usability and accessibility perspective and it is a problem when the viewport is small. I don't think hiding controls and content is the best approach to responsive design in the first place. See https://github.com/WordPress/gutenberg/issues/65238

While testing this, I noticed the buttons in the Global Styles sidebar's header are also wrapping and overlapping when showIconLabels is true. The available real estate there is unlikely to change and I have some vague recollections around components in the editor needing to remain unaware of the showIconLabels preference.

That is a known issue and it's reported on https://github.com/WordPress/gutenberg/issues/61761 I don't think introducing exceptions to showIconLabels would be a good option. Rather, there is a broader issue with all the panels headers. For better usability and accessibility, all panels should have a title. That was first tried on https://github.com/WordPress/gutenberg/pull/61553. From there, I created a new issue https://github.com/WordPress/gutenberg/issues/63251 where I'm proposing to add an optional sub-header to all panels. The sub-header may contain the buttons when showIconLabels is true. That proposal was inspired by the solution implemented for the LinkControl where the buttons go in a new line when showIconLabels is true, see https://github.com/WordPress/gutenberg/pull/61726

ciampo commented 1 month ago

@afercia moved the text wrapping conversation to a separate issue: https://github.com/WordPress/gutenberg/issues/66049

I have some vague recollections around components in the editor needing to remain unaware of the showIconLabels preference

@aaronrobertshaw yeah, basically code in @wordpress/components shouldn't rely on WordPress-specific settings. We could think about adding a more generic functionality to Button that is not tied to the settings — and then the consumer of the Button component would be in change of "connecting" the WordPress setting to the hypothetical Button prop

getdave commented 1 month ago

👋 Thanks for all the discussion and work on this.

Looking at this with a 6.7 lense for a second, I could be wrong but it seems like there are a lot of moving parts and fixes being proposed that could expand in scope.

Would it be better to defer this work until 6.8 cycle to allow a more comprehensive solution to be devised as a components level?

I definitely support the effort to improve the current situation but if the complexity is high it's probably not a good fit for this late in the release cycle.

What do you think? Is it possible to "de scope" this to a simple fix or is it a larger problem that could do with more time to resolve?

Update: I just noticed https://github.com/WordPress/gutenberg/pull/66038#issuecomment-2407108661 is a proposal for a quick fix. So the above might be a moot point.

afercia commented 1 month ago

@getdave Personally I'd vote for fixing https://github.com/WordPress/gutenberg/pull/66038 for 6.7 and look at the broader issue for 6.8.

afercia commented 1 month ago

Would it be better to defer this work until 6.8 cycle to allow a more comprehensive solution to be devised as a components level?

Reminder: to address in a more holistically way the implementation of showIconLabels I created https://github.com/WordPress/gutenberg/issues/61763. That is five months ago, already. I also proposed a potential approach in https://github.com/WordPress/gutenberg/pull/61824. While that may not be the best approach, I'd appreciate more focus on that issue and any alternative option. Thanks.