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

Unify visual separator between mover buttons when show button label is on #58999

Closed
t-hamano opened this issue Feb 14, 2024 · 4 comments · Fixed by #59158
Closed

Unify visual separator between mover buttons when show button label is on #58999

t-hamano opened this issue Feb 14, 2024 · 4 comments · Fixed by #59158
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Edit Post /packages/edit-post [Package] Edit Site /packages/edit-site [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@t-hamano
Copy link
Contributor

Description

As mentioned in #49556, there should be a visual separator between mover buttons when "show button label" is on. However, depending on the browser size, block orientation, and whether the top toolbar is enabled, this separator may not display correctly.

Desktop view and horizontal layout

  • Separator should be vertical.

image

Desktop view, horizontal layout, and top toolbar enabled

  • Separator should be vertical.

image

Tablet view

  • Separator color should be gray instead of black.

image

Tablet view and horizontal layout

  • Separator should be vertical.
  • Separator color should be gray instead of black.

image

Mobile view

  • There is no separator itself.

image

Step-by-step reproduction instructions

  • Open the post editor or site editor.
  • Turn on "Show button text labels" from Options > Preferences > Accessibility.
  • Select the block and check the toolbar.

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress version: 6.5-beta1-57631
  • Gutenberg version: 17.7.0-rc.1

Regarding this issue, #49556 and #57640 are related, but not all issues were introduced in WP6.5. It may not be a necessary task for WP6.5, but if we can fix it, I think it's worth backporting.

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

@t-hamano t-hamano 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] Edit Post /packages/edit-post [Package] Edit Site /packages/edit-site labels Feb 14, 2024
@t-hamano t-hamano self-assigned this Feb 14, 2024
@t-hamano
Copy link
Contributor Author

@afercia

Is a visual separator still necessary when mover buttons are horizontally aligned? I would appreciate any advice from an accessibility perspective 🙏

mover-button

@afercia
Copy link
Contributor

afercia commented Feb 14, 2024

Ouch I missed these buttons change to 'Move left' and 'Move right' for blocks that can be rearranged horizontally 🤦🏽

Is a visual separator still necessary when mover buttons are horizontally aligned?

Generally, from an a11y perspective, I don't like buttons with no borders. Ideally, all buttons should have borders, which would make separators pointless.

I'd say that for now we should make them look consistently with other buttons groups where separators are only used between groups while buttons don't have separators. For example in a Heading block:

Screenshot 2024-02-14 at 12 19 37

The format buttons don't have separators becouse they belong to the same group.
From a logical perspective, Move left / Move right belong to.a group so they should look consistently.

Aside: I don't understand why 'Change level' and 'Align text' are in the same group. Their purpose and functionality is different and they shouldn't be in the same group. I see they are in the same group also with icon buttons...

Screenshot 2024-02-14 at 12 24 59

@t-hamano
Copy link
Contributor Author

Thank you for your reply.

In that case, do you think the following is the best solution at this point?

  • If the mover button is horizontal, the separator is not always displayed.
  • If the mover button is vertical, always show a separator so that the button is not misunderstood as having multiple lines of text (#57640 mentioned it)

@afercia
Copy link
Contributor

afercia commented Feb 14, 2024

Yes I think for now that's the best path forward 👍🏽

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] Edit Post /packages/edit-post [Package] Edit Site /packages/edit-site [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants