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

Keyboard activation of undo buttons in toolbar creates inconsistent focus behaviour #3486

Closed
ephox-mogran opened this issue Nov 15, 2017 · 6 comments
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@ephox-mogran
Copy link
Contributor

Issue Overview

So Gutenberg's history is managed by changing redux state. This works really well if the undo and redo buttons are fired by the keyboard shortcuts (e.g. Ctrl + Z), where moving through the page history can shift the focus to the corresponding block correctly.

However, if the user has navigated to the toolbar, and pressed Space / Enter to active the undo and redo buttons, the focus becomes very inconsistent

  1. It can stay on the button itself. This happens if the undo puts you in the PostTitle. This is likely because the PostTitle focus state is not encapsulated in the redux state.
  2. It can jump to the block where the last history change was saved. This is likely because the block focus is often encapsulated in the redux state.

Note, with (1), if the undo button becomes disabled because you get back to the start of the undo history, the focus is lost, and the focus returns to the body, breaking keyboard navigation.

With (2), this prevents a user from making multiple undo operations in a row. You could rather easily argue that most people would be using the Ctrl + Z shortcut, though (note, however, that this is a different operation from clicking / keyboard activating that button though ... as it goes through TinyMCE). The behaviour just seems to be inconsistent. We need to decide what should happen.

a) Should keyboard activating the undo / redo buttons always transfer focus back to whatever block previously had focus? What if something else had focus in the block?
b) Should keyboard activating the undo / redo buttons always keep focus on the button. Would it work differently then if you clicked on it with the mouse?

Steps to Reproduce (for bugs)

Situation 1

  1. Create a new post
  2. Focus the post title
  3. Type "One word"
  4. Press Alt + F10 (or equivalent)
  5. Navigate with the arrow keys to undo
  6. Press Space to trigger Undo repeatedly until the button gets disabled

Expected Behavior

The focus stays somewhere in the toolbar, or it jumps straight to the PostTitle immediately.

Current Behavior

The focus stays in the toolbar until the last undo, and then focus is just lost.

Situation 2

  1. Create a new post
  2. Focus the "Write Your Story" block
  3. Type "One two three four five"
  4. Press Alt + F10 (or equivalent)
  5. Navigate with the arrow keys to undo
  6. Press Space to trigger Undo

Expected Behavior

The focus stays somewhere in the toolbar, or it jumps to the last editable focused in the history.

Note: I also thought that pressing that button would trigger an undo in TinyMCE instead, but it doesn't. This means that Ctrl + Z and activating that button have completely different meanings. Is there a better way we can handle that?

Current Behavior

The focus jumps to the last editable focused in the history, so this can be fine as long as it is consistent.

@ephox-mogran ephox-mogran added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Nov 15, 2017
@ephox-mogran
Copy link
Contributor Author

@afercia what is your preference here? Should triggering an undo via the keyboard via the toolbar shift the focus away from the toolbar?

@afercia
Copy link
Contributor

afercia commented Nov 28, 2017

Re:

Note, with (1), if the undo button becomes disabled because you get back to the start of the undo history, the focus is lost, and the focus returns to the body, breaking keyboard navigation.

I'd say in this case there should be:

  • an audible indication that the user reached the start of the undo history
  • focus should be moved to a reasonable place, maybe the "Redo"?
  • same for when the Redo end is reached, should focus be moved to "Undo"?

a) Should keyboard activating the undo / redo buttons always transfer focus back to whatever block previously had focus?

Well, from an accessibility perspective, focus should stay on the buttons, for the reasons you've already explained 🙂

@afercia
Copy link
Contributor

afercia commented Jul 16, 2018

Haven't followed closely but I've heard the undo/redo has been recently improved. From an a11y perspective, it's best to keep focus on the buttons, which is the current behavior. Unsure what's left to do here but maybe this could be closed? /Cc @mtias

@aaronjorbin
Copy link
Member

@afercia Could you please give this another quick test and if you are happy with it, close things out?

@aaronjorbin aaronjorbin added this to the Merge: Accessibility milestone Oct 7, 2018
@afercia
Copy link
Contributor

afercia commented Oct 27, 2018

The problem is still there. When there are no more Undo or Redo steps, the related button (which is the focused element until then) gets disabled with a disabled HTML attribute and it's not focusable any longer. At that point, technically there's a focus loss. Focus losses should be avoided at all costs. I can think of two options:

  • not disable the button and just "noop" it – styling and aria-disabled can be used to communicate it doesn't do anything
  • disable the button but then manage focus programmatically to move it in the most logical place

@tofumatt tofumatt self-assigned this Nov 1, 2018
@tofumatt
Copy link
Member

tofumatt commented Nov 1, 2018

Just FYI:

keeping the button actually disabled requires a lot of focus management and extra actions. I tried it and it was a bit of a mess. I'll try to get this going with aria-disabled and a no-op (which should actually work), otherwise I'll punt this as a later feature. For keyboard users (whose focus we care a lot about), I'd imagine tabbing to the toolbar is not a common use-case over using Ctrl+Z, which doesn't mess with focus.

Still, I'll try the second option quickly first 😄

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).
Projects
None yet
Development

No branches or pull requests

4 participants