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

Block gallery: fix tab order #14821

Closed
wants to merge 1 commit into from
Closed

Block gallery: fix tab order #14821

wants to merge 1 commit into from

Conversation

oandregal
Copy link
Member

Fixes #14814

Before After
Peek 2019-04-04 16-08-before Peek 2019-04-04 16-16-after

@oandregal oandregal self-assigned this Apr 4, 2019
@oandregal oandregal added [Block] Gallery Affects the Gallery Block - used to display groups of images [Type] Bug An existing feature does not function as intended labels Apr 4, 2019
@oandregal oandregal added this to the 5.5 (Gutenberg) milestone Apr 4, 2019
@oandregal
Copy link
Member Author

cc @jasmussen

@oandregal oandregal changed the title Gallery block: fix tab order Block gallery: fix tab order Apr 4, 2019
@jasmussen
Copy link
Contributor

GIF:

tab order

This is a VAST improvement! Much much nicer.

I'm seeing one small issue and one "maybe-issue". When you tab backwards — "shift tab" — you don't enter captions or the "remove" button, this only works when moving forwards. Is this something fixable? I love how simple the markup change was, but it also suggests we're only "inverting" the problem. Which is better, to be sure, but it also doesn't feel like the final solution.

I would expect the following behavior:

  1. [Tab] — select first image
  2. [Tab] — select "remove" button from first image
  3. [Tab] — select caption
  4. [Tab] — select next image (etc)
  5. [Shift + Tab] — select caption of previous image
  6. [Shift + Tab] — select remove button from previous image
  7. [Shift + Tab] — select previous image

The other issue is that when I tab out of the gallery entirely, the last-focused image keeps its selection. I.e. tab from paragraph, into gallery, then into the next paragraph, and the last image of the gallery will still have a focus rectangle.

Both issues would be nice to fix, but if it's a massive amount of work compared to the relatively small code change of this PR, it's okay to merge this one and track the other issues to look at separately. What do you think?

@oandregal
Copy link
Member Author

When you tab backwards — "shift tab" — you don't enter captions or the "remove" button, this only works when moving forwards

I actually thought that was an improvement! This was my rationale: forward you explore every control but backward you are probably looking to reposition yourself in a different image. For example, in a gallery with 5 images:

  • navigate to the next image takes 3 tabs, it's 12 to get to the last image from the first one. When we add the move forward/backward controls it will be 5 tabs to the next and 20 to the last. Adding controls makes navigation within the gallery more expensive keyboard wise.

  • navigate backward takes 1 tab, and it'll take 4 to get the first from the last one (no matter the number of controls).

Note that, currently, the caption's toolbar controls are only accessible when the caption is selected and you tab backward. Keeping that behavior and adding the caption and controls to the tab backward mechanism: it'd take 7 tabs to select the previous image, and 28 to get to the first one. It'd be 9 and 36 whith the move forward/backward controls. It is even more expensive to go backward than forward for every control added.

Does this rationale make sense? Or is it a given that we should tab through every control when navigating forward and backward?

The other issue is that when I tab out of the gallery entirely, the last-focused image keeps its selection.

Yeah, I'm tracking it at #14823 and started looking into it yesterday. It's a bit of a different problem (concerns the bookeeping mechanism we use).

@jasmussen
Copy link
Contributor

I actually thought that was an improvement! This was my rationale: forward you explore every control but backward you are probably looking to reposition yourself in a different image. For example, in a gallery with 5 images:

I can see where you're coming from, and it is possible I'm mistaken here. But my understanding is that consistency is more important for screenreaders, than saving a few tab stops when navigating from item to item. Imagine tabbing forward through the block toolbar, and when shift tabbing you're always taken back to the first item. It's not a 1:1 comparison, but the point is, I think tabbing should always tab through every item, forwards and backwards, that shift is just a shift in direction.

Note that, currently, the caption's toolbar controls are only accessible when the caption is selected and you tab backward. Keeping that behavior and adding the caption and controls to the tab backward mechanism: it'd take 7 tabs to select the previous image, and 28 to get to the first one. It'd be 9 and 36 whith the move forward/backward controls. It is even more expensive to go backward than forward for every control added.

I can't quite parse that, but on the face of it, it sounds like there's an additional issue here. The way I'm calculating it is that for each thumbnail, there are three tab stops: the thumbnail itself, the remove button, and the caption button. Which means it's 3 tab stops per thumbnail, going either direction.

I can understand the desire to create fewer keyboard presses to select items, I know that Windows uses arrow keys for that. So we could potentially explore using the arrow-keys to jump from thumbnail to thumbnail without ever "entering" the thumbnail.

Adding the accessibility label to ask for additional advice.

@jasmussen jasmussen added the Needs Accessibility Feedback Need input from accessibility label Apr 5, 2019
@jasmussen
Copy link
Contributor

Also to be clear, this PR is an improvement over master, and thank you again for working on it. I'm more asking whether we should create an additional ticket to make the tab order unified in both directions.

@oandregal
Copy link
Member Author

Note that, currently, the caption's toolbar controls are only accessible when the caption is selected and you tab backward.

For reference:

Peek 2019-04-05 14-44

Note how, in master, TAB goes to image then caption, but SHIFT+TAB goes from caption to every control in the caption toolbar, then to remove button, then image, etc.

@oandregal
Copy link
Member Author

I'm confused 😕 Sometimes, in master, SHIFT+TAB doesn't go through the controls in the caption's toolbar:

Peek 2019-04-05 15-07

@jasmussen
Copy link
Contributor

It sounds like layers of bugs on top of each other :|

To be clear, THIS PR is okay to go. It's a big improvement over what's in master.

I'm only questioning whether we need to create additional tickets for this, and it sounds like we do...

@afercia
Copy link
Contributor

afercia commented Apr 12, 2019

tabbing should always tab through every item, forwards and backwards, that shift is just a shift in direction.

Ideally, yes this is definitely correct! Screen reader users can't perceive if an element appears or disappears visually. They legitimately expect to find the same elements regardless of the tabbing/arrowing direction.

Worth noting the fact some elements are not available when shift-tabbing happens in many other places in Gutenberg and it's because these elements are not rendered yet. In most of the cases, they are rendered only when their container gets selected and... focus is already there.

Noted also in #14931 (comment) regatding the "Remove image" button:

make sure the button appears both when tabbing forward and when tabbing backwards

@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed Needs Accessibility Feedback Need input from accessibility labels Apr 12, 2019
@oandregal
Copy link
Member Author

Going to close this while we figure a better alternative. I appreciate your time and feedback!

@oandregal oandregal closed this Apr 23, 2019
@oandregal oandregal deleted the fix/gallery-focus branch April 23, 2019 08:25
@oandregal oandregal removed this from the 5.6 (Gutenberg) milestone Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [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 this pull request may close these issues.

Block gallery: wrong tab order
4 participants