-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Accessibility: Try adding the keyboard navigation/edit mode #5709
Conversation
blocks/library/button/index.js
Outdated
@@ -129,13 +130,15 @@ class ButtonBlock extends Component { | |||
onSubmit={ ( event ) => event.preventDefault() }> | |||
<Dashicon icon="admin-links" /> | |||
<UrlInput | |||
autoFocus={ false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, the input was being focused when you tab to the button block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like an explanation worth attaching to the eslint-disable
comment. :)
@@ -65,10 +65,14 @@ class NavigableContainer extends Component { | |||
this.props.onKeyDown( event ); | |||
} | |||
|
|||
if ( this.props.disabled ) { | |||
return; | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a way to disable the tabbableContainer behavior without replacing the component (to avoid rerenders)
if ( TAB === keyCode ) { | ||
const eventToOffset = ( evt, container ) => { | ||
const { keyCode, shiftKey, target } = evt; | ||
if ( TAB === keyCode && target.parentElement === container ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tabbable container behavior triggers only if we're currently focusing one of its children.
🔥 thank you for working on this. I think the ingredients for something great are here. The behavior is a little bit confusing, it's easy to lose your place: In this, I start by tabbing through blocks. Enter then enters the block, where pressing the Tab key now cycles the block UI. Only until I press escape, where tab now switches between blocks again. How do I get back to cycling through the block UI? How would I know to press Enter first? Part of this confusion can be mitigated visually, by not showing the block toolbar or side UI in navigation mode, perhaps even highlighting the block in blue (like multi select) instead of showing the side UI. Another part of the confusion is that only tab switches between blocks. Why don't arrowkeys? Given that this is an editor, the arrowkeys are so crucial to moving up and down the content, it feels weird when that jumps straight into edit mode and starts navigating with the caret. It feels brittle and not as modal as I think navigation mode would have to be in order to work. Windows 10 has an interesting approach to keyboard navigation. Tab navigates between sections in a modal. Arrow keys then navigates items in that section: In the above GIF I'm first tabbing between the three columns of the start menu, then arrow-keying inside each section. Perhaps something similar can work for us?
The chief benefit of this approach would be it would not be a new mode we add on top of what we already have. It would essentially be a set of keyboard enhancements to our existing multi select mode. See also this very long document I wrote about that here: #4382 (comment) Notion.so works similarly to this, if you have questions about how this feels I'd recommend giving that a shot. The biggest challenge with this branch is that you don't really know that you're in a mode, so the behavior of Tab is unpredictable. It's also confusing because even in edit mode, you can tab through all the blocks, because in this mode, tab tabs through EVERYTHING. Immediate small steps to try would be:
Future steps to explore could be:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this feels like a solid start. I think it's worth keeping this PR small and address the following points later:
- I'm not a screen reader user, and hopefully @afercia will provide better insight, but I have some expectation that pressing Escape when in navigation mode could take me out of the "blocks loop"; perhaps non-intuitively, the only way to move focus back to the WP-Admin sidebar is to tab to the last block, press Enter to switch to edit mode, then tab out of the block list.
- Since pointer actions have no impact on the keyboard mode, behavior may be inconsistent for users who combine pointer and keyboard interactions. For instance, a user clicks into some text, edits, then tabs away; depending on their previous sequence of Enter/Escape input, tabbing may happen in either keyboard mode — seemingly randomly. This is naturally a minor point.
blocks/library/button/index.js
Outdated
@@ -129,13 +130,15 @@ class ButtonBlock extends Component { | |||
onSubmit={ ( event ) => event.preventDefault() }> | |||
<Dashicon icon="admin-links" /> | |||
<UrlInput | |||
autoFocus={ false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like an explanation worth attaching to the eslint-disable
comment. :)
renderBlockMenu={ renderBlockMenu } | ||
/> | ||
) ) } | ||
<TabbableContainer disabled={ this.state.keyboardMode !== 'navigation' } restrictNavigationToChildren> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect restrictNavigationToChildren
is a leftover artifact.
return; | ||
} | ||
|
||
this.focusFirstTextFieldOrContainer( -1 === initialPosition ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but I find readability to be improved by doing:
const isReverse = -1 === initialPosition;
this.focusFirstTextFieldOrContainer( isReverse );
This is not easy to get right. I agree we should leave this PR small because if we try to make everything right here, this will never land as it's too complex. So I'm proceeding by small steps and would love if you could tell me what the minimum required to land this. In my last commit I try to solve this issue:
The approach I took is to avoid cycling through blocks and navigate away when we reach the end. I'm not totally happy with the code though, it seems too complex for a small enhancement. Can't think of an alternative though. What do you think? |
Small PRs sound good, and hopefully better devs will chime in with code feedback. Ideally, though, each step along the way brings with it something useful, with as few regressions as possible. Here are some thoughts as to what would be the minimum amount we need necessary in order to progress. The chief challenge in this branch as is, is that it's impossible for a user to know when they are in navigation mode, and when they are in edit mode. Since the Tab button acts differently depending on mode, this will cause confusion as to when tab switches between blocks, and when it enters the movers/ellipsis/block toolbar. In that vein, perhaps in order to move on, the only change we need to do is to hide the mover, the block toolbar, and the ellipsis when in navigation mode. You'd only see the 1px border. Everything else could be intact and work as it used to, even exiting navigation mode by using the arrow keys. What do you think? |
When do you decide to show the movers and the block toolbar in that case since when you use the mouse, to click etc.. the mode is not changed? |
As soon as you hover a block, or click a block, you are in edit mode. Maybe just moving the mouse puts you in edit mode. |
Thanks for working on this, pretty excited after some first testing. I'd tend to agree with many of the considerations made here. Trying to proceed step by step, the main thing I see now is the "blocks loop" dilemma. As @mcsf pointed out, there's no way to exit the loop and I totally agree with @youknowriad: there shouldn't be a loop in the first place 🙂 One of the main goal of this experiment is to put the blocks in a natural tab order. Once on the last block (or the first when shift-tabbing) the expected behavior would be being able to continue tabbing the UI in a natural tab order. As per the expected interaction when using a screen reader, there's a big difference in terminology and "arrowing" is the default way to explore all content. Without entering into details about what browse mode and forms mode are, I think the best option is trying to emulate the interaction model users would expect in a form, where tabbing navigates through the form controls. I'm not fully convinced arrowing should navigate through blocks, that wouldn't be expected. Comparison with desktop UIs (Windows 10 start menu) is a bit misleading as they're built following an interaction model that is close to the one some ARIA widgets use (e.g. tabs or grids) but it's very different from the one expected in web forms. @joen I'd tend to agree in navigation mode the blocks UI shouldn't be displayed. Navigation mode should be just that: navigation :) I'd agree the "modes" should be made clear and communicated to users. For sure, keyboard interaction in a complex application requires documentation and an user guide (see #5420) but I'd tend to agree the modes should be communicated also visually.
I'd tend to say yes but I guess this could be addressed in a future iteration? See also the initial a11y recommendations in #297
Regarding:
But it is still possible when in Edit mode, right? Lastly: as a final step, there will be the need to give a role and properly label the container where focus lands in navigation mode. We already use a label for the blocks, I think that should be moved. Currently, when using a screen reader and landing in navigation mode on a block, all the content of the block gets announced because that container has no semantics, it's just a div with a
|
Thanks for your feedback all. Before trying to add the visual distinction suggested by @jasmussen between the two modes, I'm trying an alternative technical approach to this issue relying on the WritingFlow component in this branch https://github.com/WordPress/gutenberg/compare/try/writing-flow-edit-mode I'm thinking it's a better approach that scales better. I'm waiting for feedback here on this second approach before continuing. |
Nice work Riad. I tried the try/writing-flow-edit-mode branch, and it seems to work well there as well. One thing I noticed in that branch, is that arrowkey navigation doesn't exit edit mode and give you the caret like it does in this branch. I know I noted that wasn't desirable earlier on — but I feel like either the arrow-keys should exit navigation mode and enter edit mode with the caret, or the arrowkeys should work for navigating between blocks in edit mode. The more I think about it, though, the more I think (unlike my past assumption) that maybe it's good that the arrow key exits navigation mode. Unsure. But in either case, the arrowkeys should do something in navigation mode. Otherwise, seems good! |
Good catch and I did it on purpuse :). My idea was that moving the mouse will trigger edit mode, so maybe it's fine that in navigation mode, the arrows do nothing. That said, it's easily tweakable. We can either navigate the blocks like the tabs, keep the caret navigation or do nothing. your choice :). This flexibility is harder to achieve in the current branch though :) |
Love the sound of that. Then we should go with what seems best intellectually for now, and then tweak and adjust as we test it in master. 👍 👍 🔥 |
Nice work! Couple things: while we're here, can we please move the
they all correctly announce the type of the block, which is what we want: This is ideal because users will have an immediate feedback while navigating through the blocks. For example:
Instead, Firefox + NVDA 2018.1 do announce, for example, "Block: Paragraph" but they announce also the whole content of the block including the movers, formatting toolbar etc. A bit unfortunate, but seems to me a peculiar behavior of NVDA and can probably be improved using a more appropriate role. Will try to iterate on this. @youknowriad thanks very much. I've just noticed a small thing, please check if you can reproduce, when you have a chance:
Not sure what the best option is, but I'd tend to think the last "insertion point" should always appear. Of course, the behavior is different if the last block is, for example, an image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see last comment
The issue here is that the insertion point is part of the last block (inside the writing flow). What I do know is skip to the end when we reach the last block. But if I don't do this logic, when you reach the last block, you'll go through all its inner tabbable elements before reaching the sidebar (movers, toolbar...) |
Let me know what you think about the current state, I'd like to stop enhancements at this level to avoid complexifying more. |
Ah i Understand. I also realize now the modes are "global", meaning once you switch to edit mode, all blocks are in edit mode. Which is fine, I guess. Any thoughts about the second paragraph that gets skipped on Shift+Tab? I understand the need to not introduce further complexity at this point, I'd defer to @jasmussen any decision. |
@afercia Actually, I was able to fix both issues with the last commit. Always skip the inserter in navigation mode and focus the last paragraph. |
Ahh maybe I've catched one thing. Ideally, for keyboard users the default should always be "navigation mode. However, when using Shift+Tab, as soon as I press |
Actually, it's not the case but since it's hard to start tabbing without moving the mouse first, you end up being in edit mode before maybe? |
You're right, happens only in Safari. As soon as I press Shift, the block UI appears and I'm in edit mode. Can you reproduce? |
@afercia I can't seem to reproduce Safari Version 11.0.3 |
@afercia Wait I do reproduce now. safari 😱 |
e104018
to
0793684
Compare
I rebased this, let's try to get it to the finish line. Even if it doesn't benefit all the users for now, it's still an improvement as is.
I think we want to avoid a button per block as it's too cumbersome.
I'm leaning towards a global keyboard shortcut for now. What would be this shortcut? And do we know if the global shortcuts work on screenreaders? |
In the spirit of looking at alternate approaches, here's a very silly question. I expect the answer to be "no, that doesn't make sense", but I'm going to pose the question regardless just in case it inspires something else. What if you used page down and page up buttons to select blocks. 20 blocks on a page, press page down 20 times to get to the last one. Everything else is unchanged, you can still tab or use arrow keys. |
Or maybe "Ctrl + arrow" instead of "page up/down"? |
Hm, hold on please. If the click on the div is the only way to switch to edit mode, this results in the complete inability to edit a block for screen reader users on Windows. |
@afercia What's the way forward for you, we have several propositions here and it doesn't make sense to hold this more. |
@youknowriad I've just tested it again on Windows (Firefox + NVDA) and it really creates a huge barrier for screen reader users. When in browse mode, pressing Enter on the div doesn't work. As discussed at length, this is expected with Windows screen readers in browse mode. It's standard behavior and there's nothing to "fix". It's also one of the reasons why jsx-a11y warns to not use events on non-interactive elements. There are only two ways to make this work I can think of:
A few days ago we've also discussed a bit this issue with @mtias here's some point emerged:
|
Let's try and unstick this. Turns out Slack has a very cool version of this, that I think might work for us. It works like this:
Here's a GIF: If you noticed, once you started tabbing, a neat little "tip" modal was invoked (cc: @karmatosed) that suggests more keyboard shortcuts: I like this approach because:
I think Andrea's suggestion could make this work for us:
I think we can work with this, especially if #6773 lands first. Imagine if the following GIF showed you using up and down arrows, instead of me mousing over it: Although it looks like the block is selected/hovered, in fact the breadcrumb button is focused. And when you press Enter or space or whatever you need to press to invoke that button, well then you're editing that block. CC: @mtias |
Hm... it's an interesting idea. Not sure it will work, as the arrow navigation would need a keydown event on the blocks wrapper div and keyboard events don't work on non-interactive elements with Windows screen readers in browse mode.... |
Is there any way we can find that out? |
Well it's basically the same thing that happens in this PR. There would the need of a |
A word of caution — asking screen reader users to switch browse mode on and off is one of the issues that keeps screen reader users from Slack. In short, Slack's interface overall is inaccessible and should be used cautiously for pattern ideas. Related thread from March: https://twitter.com/LeonieWatson/status/974615133044359168 |
We've discussed this issue during WCEU contributor day with @aardrian and decided to try a new way:
Not fully sure it is possible from a technical point of view, but it's something definitely worth trying as a native button with a click event would always work even when a screen reader is running. |
@afercia @aardrian Per the new way suggested, would we be able to accomplish the proposed direction suggested in #5709 (comment) ? Let me know if you'd like me to explain that approach in more detail. |
@jasmussen with Windows screen readers, the expected behavior is (in short):
So the expected behavior with the arrow keys in browse mode is to navigate through all the content. In this mode, key presses are intercepted by the screen reader and the browser is unaware a keyboard event occurs. That's to say arrows to navigate through blocks won't work because the keydown event used for that won't work. For clarity, the Gutenberg Writing Flow arrows navigation works because the blocks have a To my knowledge there are only to options:
Overall, I think we should try to avoid "smart behaviors" of the application because they're hardly understandable by users. Instead, an UI control that must be intentionally activated by the user would be way more clear. |
Is there still a relevancy / desire to continue the work here? |
This is still blocked awaiting a response to mockups in #5694 (comment) |
Between arrow keys moving through blocks and #10545 giving us a keyboard-accessible (even with Firefox + NVDA) way to move throughout the post's blocks with an easy overview, I think the intent of this PR is covered. I think we should close it for now; please re-open if this is still something we want to look at, but there are issues with this approach that #10545 managed to bypass 🙂 |
refs #5694
Adding the navigation/edit mode can be very impacting for the edit flow, so it's best to try to do it by steps.
In this PR.
The drawback of the edit/navigation mode is that it's not possible to create an empty paragraph after the selected block by clicking "Enter", since "Enter" is the key used to move to edit mode. Is this acceptable?
Need help testing the different writing flow interactions to ensure there's no other regression.