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

The image block's editing tools break the toolbar keyboard accessibility #24766

Closed
afercia opened this issue Aug 24, 2020 · 7 comments · Fixed by #25127
Closed

The image block's editing tools break the toolbar keyboard accessibility #24766

afercia opened this issue Aug 24, 2020 · 7 comments · Fixed by #25127
Assignees
Labels
[Block] Image Affects the Image Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Aug 24, 2020

Describe the bug
Related: #24676 and #24021

Accessing the image block editing tools not only triggers a focus loss as reported in #24676 but also breaks the toolbar keyboard accessibility. The block toolbar can't be used with a keyboard any longer.

As mentioned in #24021, replacing parts of the toolbar "on the fly" also breaks the toolbar expected behavior, specifically:

  • arrows navigation
  • the "roving tabindex"

I didn't look in depth at the code but I think both features rely on the which buttons (and their number) are initially rendered within the toolbar. Replacing part of those buttons with another UI breaks the toolbar implementation itself. To test: try to "manually" re-focus the toolbar and at that point various weird behavior happen:

  • arrows navigation is broken
  • some buttons within the toolbar can be focused by pressing the Tab key: this shouldn't happen, as only one button should be focusable via tabbing
  • at some point, arrowing isn't constrained within the toolbar and focus is moved to the post title or somewhere else.

Besides the general usability and accessibility concerns mentioned in #24021 I'd tend to think this design pattern that replaces part of the buttons within the toolbar isn't ideal and needs to be reconsidered. I'll propose to discuss this issue, #24021, and the proposed design in the next weekly accessibility meeting.

To reproduce
Steps to reproduce the behavior:

  • edit a post with an image block
  • click the image
  • press Shift + Tab to move focus to the block toolbar
  • use the arrow keys to navigate the toolbar buttons
  • once on the "Crop" button, press Enter
  • part of the toolbar UI gets replaced with another UI; the image editing tools
  • focus is lost: there's no focused control
  • modern browsers implement the so called sequential focus navigation starting point so that pressing Tab after a focus loss tries to start the tab sequence from the closest focusable control but this isn't guaranteed to work properly, especially with assistive technologies
  • after the focus loss, press Tab: using Chrome, focus goes to the "Aspect Ratio" button because of the browser "sequential focus navigation starting point" feature
  • this is unexpected because navigation within the toolbar should work only with arrow keys
  • use the arrow keys to focus other buttons in the toolbar: depending on the pressed arrow key (left or right), focus goes to the post title or the following block thus existing the selected block

Also the "roving tabindex" implementation appears to be broken:

  • tab again from the image to the toolbar while it displays the image editing buttons
  • observe that more than one button can be focused by pressing Tab
    • expected: only one button within the toolbar should have a tabindex=0 and thus be focusable with the Tab key
  • use the arrow keys: observe the "Aspect ratio" button gets skipped

Expected behavior
The expected toolbar behavior to work and match the ARIA toolbar pattern, where:

  • only one button is focusable by pressing the Tab key
  • navigation through the buttons works by arrow keys

Editor version (please complete the following information):

  • WordPress version: 5.5
  • Does the website has Gutenberg plugin installed, or is it using the block editor that comes by default? both
  • If the Gutenberg plugin is installed, which version is it? 8.8.0
@afercia afercia added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block [a11y] Keyboard & Focus labels Aug 24, 2020
@talldan
Copy link
Contributor

talldan commented Aug 26, 2020

I think we touched upon it recently in brief discussion on #24021, but there are similarities between the two interfaces and there was some desire to understand if this UI could be treated as a popup menu.

Implementation wise:

  • The Crop button uses the aria-haspopup="menu" attribute and the aria-expanded state appropriately (as per Fix the usage of aria-haspopup  #24796).
  • Tabbing and arrow key navigation is constrained in the 'popup menu'.
  • Escape can be used to dismiss the menu and return focus to the visible again Crop button.

One issue I notice is that other popup menus in the toolbar tend to use vertical arrow key navigation, while this one is displayed horizontally and uses horizontal arrow key navigation. That might break expectations. How would a screenreader user know which set of arrow keys to use, is there a good way to solve that?

Visually, another issue might be that there's not anything to delineate that navigation is now constrained to the toolbar controls shown in this image:
image-toolbar

#24021 goes further by not making other parts of the toolbar visible, and the same approach could potentially be used on the image block.
Screen Shot 2020-07-31 at 9 28 43 AM

For #24012, as the UI contains an input field, I imagine it would need to be represented as a dialog and use the tab key instead of arrow keys?

@afercia
Copy link
Contributor Author

afercia commented Sep 28, 2020

I'm not sure the fix on #25127 is a good path forward because the "roving tabindex" interaction is broken anyways. See comment #25127 (comment)

Also, the accessibility team discussed an alternative solution for all toolbars that was proposed by @enriquesanchez which should be a decent compromise between the given design and the accessibility needs. @enriquesanchez do you think this issue should be reopened or are you keeping track of the new proposal on another issue?

@diegohaz
Copy link
Member

I'm not opposed to re-open this issue. Though the PR I made was a legitimate fix for a regression on the image block toolbar that was making it work differently from the other toolbars. The PR fixes many issues you described here, like these:

  • arrows navigation is broken
  • some buttons within the toolbar can be focused by pressing the Tab key: this shouldn't happen, as only one button should be focusable via tabbing
  • this is unexpected because navigation within the toolbar should work only with arrow keys
  • use the arrow keys to focus other buttons in the toolbar: depending on the pressed arrow key (left or right), focus goes to the post title or the following block thus existing the selected block
  • observe that more than one button can be focused by pressing Tab
    • expected: only one button within the toolbar should have a tabindex=0 and thus be focusable with the Tab key
  • use the arrow keys: observe the "Aspect ratio" button gets skipped

I would say the main problem discussed in this issue is fixed. As I can see, there are separate issues for focus loss (#24676) and focus restoration (#24469). Please let me know if I'm missing anything.

@diegohaz
Copy link
Member

Note that there are toolbars with items that change on the fly but don't result in a focus loss because the change is triggered by an action outside of the toolbar (for example, clicking to upload an image). The PR also ensures that those newly added buttons won't break the keyboard navigation in the future when the user moves the focus onto the toolbar.

@afercia
Copy link
Contributor Author

afercia commented Sep 28, 2020

@diegohaz thanks, I do realize the PR fixed some of the issues. From an expected interaction perspective it doesn't fix the roving tabindex pattern though, and it can't be fixed since some of the elements are changed on the fly. I mean that it is the design pattern that breaks it. This is the reason why hte accessibility team explored an alternative solution. For reference: #24021 (comment)

Also, from as semantic perspective, the presence of a form or input field in other toolbars is an anti-pattern as a toolbar is supposed to only contain "controls". I couldn't find a definition of "controls" in the specs but to me they're form controls (even checkboxes as in the ARIA Authoring Practices example) that "do something". Certainly not form controls meant for user input. A toolbar isn't the place for user input. Since for those cases we need to find a different pattern anyways, I guess we'll need to change the Image editing toolbar as well.

@diegohaz
Copy link
Member

This is the reason why hte accessibility team explored an alternative solution

I'm aware of that. And I totally agree with that solution. That's pretty similar to what I proposed back in July:

Please, correct me if I'm wrong. But, from what I understand from this issue, the "toolbar" that contains the text field is not really a toolbar. It seems to me more like a popover with a form inside. And it temporarily hides the block toolbar while it's open. Just like other dropdown buttons, but hiding the toolbar. Thus, arrow key navigation wouldn't be necessary here. User should be free to use Tab to navigate through the elements in this popover.

This is not an alternative to #25127 though. Both solutions are addressing different problems, even though they seem related. They aren't mutually exclusive.

Currently, this issue is in the state of "can't reproduce" for most things, and this could be confusing for people coming here. The other issues that aren't fixed by #25127 already have their own issues as far as I can see (please, correct me if I'm wrong). So it's probably better to discuss it there.

As for the "popup" pattern on the Image block, I think it should be discussed in #24676.

@enriquesanchez
Copy link
Contributor

do you think this issue should be reopened or are you keeping track of the new proposal on another issue?

@afercia The alternative solution is being explored and tracked on #24021. I personally think it's fine to keep this one closed.

@priethor priethor added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [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