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

Locked block toolbar layout broken when 'Show button text labels' is enabled #49865

Closed
afercia opened this issue Apr 17, 2023 · 6 comments · Fixed by #50944
Closed

Locked block toolbar layout broken when 'Show button text labels' is enabled #49865

afercia opened this issue Apr 17, 2023 · 6 comments · Fixed by #50944
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Apr 17, 2023

Description

When the 'Show button text labels' preference is enabled, a locked block toolbar doesn't look right.

There's no separation between the block switcher button and the unlock button. Visually, it's pretty confusing. Also the paddign on the left is incorrect.

Step-by-step reproduction instructions

  • Lock a paragraph block:
    • From the block toolbar > Options > Lock > Lock all > Apply
  • The block toolbar now shows the 'unlock' button after the block switcher:

Screenshot 2023-04-17 at 15 47 43

  • Go to the editor Options > Preferences, and enable 'Show button text labels'.
  • The block toolbar now shows text labels instead of icons.
  • Observe there's no sepaaration between the first two buttons. Also, the left padding looks bigger than expected:

Screenshot 2023-04-17 at 15 48 24

  • It happens also with 'top toolbar' enabled:

Screenshot 2023-04-17 at 15 51 47

For reference, here's how the block toolbar looks when the block is not locked:

Screenshot 2023-04-17 at 15 49 37

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 [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor labels Apr 17, 2023
@annezazu
Copy link
Contributor

@tellthemachines curious if you could dive in here as I know you've done some fixes on the text label experience <3

@joedolson
Copy link
Contributor

This is coming from this CSS:

.block-editor-block-lock-toolbar .components-button.has-icon { padding-left: 0 !important; }

It's removing the left padding on the lock button, which you can see has compressed spacing both when using icons and using text; but it makes a much bigger difference to comprehension when viewing as text.

Separately - is it necessary for that button to contain the block name? That context should already be apparent, since the neighboring button gives the context. Seems unnecessarily verbose.

@Mamaduka Mamaduka self-assigned this May 24, 2023
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 25, 2023
@Mamaduka
Copy link
Member

Created PR to resolve styling issues.

Separately - is it necessary for that button to contain the block name? That context should already be apparent, since the neighboring button gives the context. Seems unnecessarily verbose.

I like the idea and am happy to create PR if everyone agrees on the change.

@afercia
Copy link
Contributor Author

afercia commented May 26, 2023

Separately - is it necessary for that button to contain the block name? That context should already be apparent, since the neighboring button gives the context. Seems unnecessarily verbose.

I'd agree to try to remove the block name. However, I think that button needs a few more improvements so yes I'd recommend a separate PR.

Regardless whether it's icon or label, when a block is locked that button gets disabled with both a disabled attribute and aria-disabled="true".

Screenshot 2023-05-26 at 10 25 23 Screenshot 2023-05-26 at 10 24 55

That's not ideal because the button can't be navigated to with the keyboard. As such, there's no contextual information for screen reader users (although screen readers do provide alternative ways to get to that button).

In several places in the Editor we do use a different pattern to keep disabled controls discoverable. See Focusability of disabled controls in the ARIA practices.

To recap:

  • The block toolbar is an ARIA toolbar.
  • Expected keyboard interaction: navigation through the toolbar buttons is by arrow keys.
  • However, the 'Paragraph' button is not focusable because of the disabled attribute: the button gets skipped when arrowing.
  • Screen reader users will miss that context.
  • To remediate:
  • The button should only use aria-disabled="true" and be 'noop-ed`. We should make sure that arrow navigation works on this button.
  • When the block is locked, the button should also look disabled. Currently, it doesn't.
  • The icon button should still display its tooltip.
  • The tooltip should not be displayed when 'Show button text labels' is enabled.

@Mamaduka
Copy link
Member

I created a separate PR to make the button label less verbose - #51083.

@afercia, the block switcher button can be disabled for other reasons as well, so it's not tied to block locking. If you prefer, we can open a separate issue.

When the block is locked, the button should also look disabled. Currently, it doesn't.

The button also serves as a visual clue for the block type; this is probably why disabled styles aren't applied.

@afercia
Copy link
Contributor Author

afercia commented Jun 13, 2023

Thanks @Mamaduka! I will create a separate issue for the disabled state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants