Writing flow: A11Y enhancements for useTabNav #41645
Conversation
alexstine
requested review from
Mamaduka,
youknowriad,
getdave,
aristath,
talldan and
tellthemachines
alexstine
added
[Type] Bug
|
Size Change: +994 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
|
I am sure there will be tons of failed E2E tests and maybe even some unit tests. I will start working on those slowly but wanted to get some technical feedback. I know Alt+F10 is a harder shortcut but Shift+Tab is too crucial for navigating dynamic blocks. |
…nt. Remove the before tabindex to skip the block toolbar and jump to title.
| @@ -10,11 +10,11 @@ import { useRef } from '@wordpress/element'; | |||
| /** | |||
| * Internal dependencies | |||
| */ | |||
| import { isInSameBlock } from '../../utils/dom'; | |||
There was a problem hiding this comment.
This is needed to ensure a successful check later. More info shortly.
packages/block-editor/src/components/writing-flow/use-tab-nav.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/writing-flow/use-tab-nav.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/writing-flow/use-tab-nav.js
Outdated
Show resolved
Hide resolved
| const { tagName } = element; | ||
| const checkForInputTextarea = isInputOrTextArea( element ); | ||
| const checkContentEditable = |
There was a problem hiding this comment.
The default value of checking contenteditable="true" is false for this function but I needed this extended to handle tabbing in blocks such as the table block.
There was a problem hiding this comment.
Do we need this option? Can't we just make it the default? This function was added to check form elements in placeholders. Not sure why it was ever moved to the dom package as an API.
I think we're also missing links here.
Why can't we just use check the tabbable API (findNext) and skip over blocks?
There was a problem hiding this comment.
Oh yes, I do not think we will be able to do this. Seems like this backfires because focus.tabbable will always have somewhere to go since blocks have tabindex of 0. Need to ensure it is a form field or something we want to move to. Unless you see another way around this...
| @@ -200,7 +200,7 @@ describe( 'Order of block keyboard navigation', () => { | |||
| 'Multiple selected blocks' | |||
| ); | |||
|
|
|||
| await pressKeyWithModifier( 'shift', 'Tab' ); | |||
| await pressKeyWithModifier( 'alt', 'F10' ); | |||
There was a problem hiding this comment.
New shortcut to focus the toolbar.
| @@ -35,7 +35,7 @@ async function testBlockToolbarKeyboardNavigation( | |||
| await expectLabelToHaveFocus( 'Move up' ); | |||
| await page.keyboard.press( 'Tab' ); | |||
| await expectLabelToHaveFocus( currentBlockLabel ); | |||
| await pressKeyWithModifier( 'shift', 'Tab' ); | |||
| await focusBlockToolbar(); | |||
There was a problem hiding this comment.
This just executes Alt+F10 for us.
There was a problem hiding this comment.
Shift+Tab now works again, right? So we can revert these lines?
| await pageUtils.pressKeyWithModifier( 'shift', 'Tab' ); | ||
| // and finally Bold. | ||
| await pageUtils.pressKeyWithModifier( 'shift', 'Tab' ); | ||
| // Navigate to Image. |
There was a problem hiding this comment.
I'm not honestly sure how this ever worked. This needs bad accessibility help. Had no idea a double toolbar could exist.
alexstine
changed the title
… instead without needing to worry about moving back to the block.
| @@ -753,4 +753,42 @@ describe( 'Writing Flow', () => { | |||
| ); | |||
| expect( paragraphs ).toEqual( [ 'first', 'second' ] ); | |||
| } ); | |||
|
|
|||
| it( 'Allows block wrapper focus when there are no previous focus elements', async () => { | |||
There was a problem hiding this comment.
This ensures Shift+Tab can focus the block wrapper if there are no previous elements. Then Shift+Tab will not work for anything before the block wrapper. Tab can move back in to the block.
| await expect( getAriaLabel ).toEqual( 'Block: Image' ); | ||
| } ); | ||
|
|
||
| it( 'Allows block settings sidebar focus when there are no next focus elements', async () => { |
There was a problem hiding this comment.
This ensures you can use Tab key to access the sidebar. However, you must tab through the block first until there are no more tabbable elements.
|
I've left a comment on the parent PR. I'm not convinced we should allow tabbing in between blocks. #38311 (comment) |
|
Okay @ellatrix . This seems stable enough for testing. I restored the ability to Shift+Tab to the block toolbar. However, in blocks that still have wrappers, you will land on the wrapper first. Do you anticipate that being a problem? Up Arrow can always get you to a block wrapper, thought I should logically allow Shift+Tab to do the same. In the future, could add descriptions saying what to do in this wrapper area. Something like this I think.
Anyway, that was kind of the idea but I do not want to distract from this PR's goal. |
That's fine! I'll test the PR today. |
|
Some case I'll want to be testing:
|
|
Ok, so one accessibility problem I'm experiencing is that there's no way to access the inline toolbar anymore for blocks that define that (caption in an image block, cite in a quote). Other than that, it's working well. It's just a little weird that tab doesn't navigate through the direct inner blocks. I'm not sure how that would even work though. I understand that you can access them through navigation mode, but it's not really clear. |
Yeah, not sure how to work around that right now. We'll need to revisit this down the line. My biggest hope is to get rid of the block selection button and replace it with opening the list view. Seems like that could be a better experience long term and screen readers will not need to manually change modes since list view has better ARIA spec currently. |
|
Thanks for all the work you've put in here @alexstine ! Testing this branch, the issue Ella mentioned above with tabbing into the caption toolbar seems to be fixed. I'll try to do some more testing across other blocks in the next couple of days. This is a big change so best make sure everything is working as expected. So far it looks good though! I pushed a tentative fix for an e2e test failure that seems to be due to flakiness (I can't consistently reproduce it locally). Hopefully that improves matters on CI. I also opened #43564 to report keyboard navigation inconsistency across toolbars. |
|
Thanks for all the work on this. While reverse tabbing into the formatting toolbar from a caption now works, we're introducing some inconsistency: you can tab into the caption from the image wrapper, but when reverse tabbing you tab into the toolbar. I'm also still wondering how you can select inner blocks from a parent block, if this is supposed to be an alternative to navigating by arrow keys. Maybe we should allow the block list element to be tabbed into with instructions to enter navigation mode to select inner blocks as a label? |
|
I think we should try this under the existing option that disables arrow key navigation across elements. |
You cannot do that because the editor does not follow semantic code layout. That would be super nice, but I doubt it happens any time soon. If accessibility was introduced from the start instead of this late in the game, this is what I would have done.
The problem is there is too much dynamic movement in Gutenberg. The problem really begins with block selecting and sidebar/formatting toolbars constantly dynamically updating. There would not be a need for this whole big focus discussion if the editor just followed basic flows of a website.
I really want navigation mode gone because it has caused a lot of bugs between NVDA and JAWS on Windows. However, this PR is something I have worked on but still cannot get it in due to a delay in upgrading React versions. I'd like to land it, but again, it does not appear it will happen soon. Unless it already has and I just did not know/no one revisited the PR. I do agree with you on the tab/shift+tab issue with the image caption field. That must be why I disabled that toolbar in the first place. This whole PR seems to be quite a nightmare now. 😞 I think I was trying to fix a structural problem with more hacking. However, I just do not have time to contribute at this level anymore. |
What do you mean with headings to mark blocks? I'm fine with moving forward with this PR if we enable this only for the option that blocks arrow key navigation for now. If you don't have time to continue work on this, shall I take it over and tweak some things? Would you still be able to test and review? |
That would be awesome. Yes, I can still review, I just do not have much time for code writing these days now that I am not working with WordPress. I think the point I am trying to get across is the fact Gutenberg relies on a lot of custom keyboard support. When you use a screen reader on Windows, there are two different modes. One mode allows you to send all keyboard input to the application. This is called edit/focus mode. The other mode uses a virtual cursor to allow screen readers to follow the natural order of the DOM. The problem is, React/VUE are frameworks that can dynamically change the DOM and screen readers just have not caught up. We are now left with two very big problems.
My problem with number 2 is WordPress used to be so easy. Now, editing in WordPress is like learning to use Slack or Teams. A completely new interaction model with tons of keyboard shortcuts to remember. When really, it would be a huge investment, but better off to just allow Gutenberg to act like a normal website. This all sounds quite foreign to you I am sure because Voiceover does a much better job as it does not have to switch between modes constantly. However, Voiceover is not liked due to the amount of keyboard gymnastics required to do basic functions compared to Windows. I dug up this older video by @diegohaz who explains this really well. Here is the related issue where all of this stuff started coming to light. I am not saying to stop using JS completely, but JS is meant to make certain parts dynamic. Not whole apps. I mean, you may be able to use JS this way, but the end result is an inaccessible mess that is highly depend it on which OS you use. |
…ble option unstable
|
@alexstine I modified your PR so that Tab is only enabled in blocks when the keep caret in block is set to true. Could you review the changes? Then we can merge this. |
|
@ellatrix I'll review later today, thanks! Do you think we should hold off on this until #45157 gets closer? It will fix our problem with shift+tab landing on the inline toolbar and region shortcut could be used for this. Then we could have dedicated navigation keys. I do enjoy your idea there regardless of my thoughts of the keyboard shortcuts chosen. Might be good to get this one in as a good enhancement until the other PR lands. Thoughts? Thanks. |
|
That sounds good if you prefer that! |
What?
This is a fairly complex refactor of writing flow for accessibility. Specifically the tabbing functionality in useTabNav.
Why?
See the following discussion for details. Essentially, Shift+Tab/Tab keys should be reserved for navigating a blocks content, not triggering actions such as toolbar focus or sidebar jump (to be addressed in another PR).
#38311
How?
Testing Instructions
Screenshots or screencast