WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.48k stars 4.18k forks source link

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

Closed afercia closed 4 years ago

afercia commented 4 years ago

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:

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:

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:

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

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

Editor version (please complete the following information):

talldan commented 4 years ago

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:

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 commented 4 years ago

I'm not sure the fix on #25127 is a good path forward because the "roving tabindex" interaction is broken anyways. See comment https://github.com/WordPress/gutenberg/pull/25127#issuecomment-700065305

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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: https://github.com/WordPress/gutenberg/pull/24021#issuecomment-698997467

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 commented 4 years ago

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 commented 4 years ago

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.