Add block toolbar as a navigable region #56560
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ellatrix
added
[Type] Enhancement
ellatrix
requested review from
jeryj and
alexstine
and removed request for
jeryj
|
Size Change: +22 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
ellatrix
commented
Nov 27, 2023
| <BlockToolbar hideDragHandle={ isFixed } /> | ||
| </NavigableToolbar> | ||
| <div role="region" tabIndex="-1"> | ||
| <NavigableToolbar |
There was a problem hiding this comment.
Note that NavigableToolbar already has role=toolbar, which I didn't want to override.
There was a problem hiding this comment.
Yeah, I see what you mean. It does feel weird to have a region wrapping the toolbar if the only purpose is to have one toolbar within it. Could we modify the region code to allow the block toolbar to be considered a region navigation? Or is that breaking the flexibility/intention of that code too much?
There was a problem hiding this comment.
I'm not sure, I assume the region role is needed? Cc @alexstine
jeryj
reviewed
Nov 27, 2023
There was a problem hiding this comment.
I'm not sure about the usefulness of this. On one hand, I think it's a good idea to give people more ways to quickly reach a useful part of the editor. On the other, if someone wants to interact with the block toolbar, they should use Alt + F10, as it will take them directly there. If we introduce this and someone learns this as their pattern to interact with the toolbar, it would feel like a bad UX.
- Ctrl + ` to reach the toolbar region
- Tab to interact with the toolbar
- Escape to return to editing
Once you start navigating via Ctr + `, it feels disconnected to get back to where you were editing since you have to tab into the block toolbar and then press escape.
Maybe we should also consider merging regions with the Alt+F10 shortcut?
Reserving alt+F10 to always go directly to the toolbar feels useful to keep separate. I like being able to go directly to the toolbar when editing.
One issue I found in testing is if the Block Toolbar is accessible but not visible (such as when typing in a paragraph), the region isn't available when it should be accessible via the region navigation.
There's pretty significant changes in #56335, so may be worth waiting til that is sorted. That said, I don't know if the direction it's going will get merged.
Overall, I'm not sure where to go with this. I'm open to it if others find it useful though.
|
I'm not sure this should be done. The whole purpose of navigable regions is to make available to all keyboard users a feature that screen readers navigation provides for ARIA landmarks. ARIA landmarks are meant to be used for the main sections of a page. Besides structural HTML tags that have native semantics of landmark (main, nav, aside, footer, etc.), some ARIA roles are mapped to ARIA landmarks. One of them is the That said, all navigable regions in the editor are ARIA landmarks, so far. Screen readers provide dedicated ways to nqvigate through landmarks. For example:
This is a very convenient way to allow screen reader users to jump through the main sections of a page, as long as all perceivable content of the page lives within landmarks. Screen reader users can just use the native screen reader tools to navigate landmarks. The whole purpose of the navigable region feature was to make this native screen readers feature available to other keyboard users. Basically, it 'duplicates' the native screen readers feature. As such, the native feature based on ARIA landmarks must always match the equivalent Editor feature based on navigable regions. Technically, we opted to use the For example, this is treated like an ARIA landmark:
This isn't:
So, we would need to label the region that wraps the toolbar to make it an ARIA landmark. And here comes the point: ARIA landmarks are not supposed to dynamically appear and disappear from a page. Instead, they are meant to mark the main sections of a page in a way that users can orient themselves, have an idea of the structure of the page, and navigate through these sections.
The block toolbar appears and disappears, depending on the user flow. That would be an ARIA landmark that is added or removed continuously and that would defeat the purpose of a landmark. One more important point: The block toolbar lives in the main document, while blocks live in the iframed document. As such, screen readers (at least VoiceOver) will not detect the toolbar landmark when the block is selected because it lives in another document. WHen the block is selected, that's the moment where users would need to find the toolbar and this wouldn't work for native screen readers landmark navigation. It would only work with the Editor shortcuts. I tested this by adding an aria-label to the toolbar region in this PR. Screenshot: focus is in the iframed document (the block) and VoiceOver doesn't see any landmark:
Screenshot: focus is in the main document (the block toolbar) and VoiceOver sees all landmarks:
|
|
Okay, so I guess Voiceover landmark navigation is entirely useless period. If inside the iFrame, you couldn't even move to the sidebar as it is outside of the iFrame. Wow. 😞 Good thing we have the code to handle this already. I guess we'll have to close this, I was totally unaware of the Voiceover bug with regions. I knew the appearing/disappearing would need to be a discussion later but we can't solve for Apple bugs. We'll keep brainstorming in the other PR. There's a way to make this work, we'll find it eventually. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds the block toolbar and inline rich text toolbar (for example for image captions) as a region so you can navigate to it directly with ctrl+`.
Why?
See #56474 (comment)
How?
Just makes it focusable and adds the region attribute. We discussed that navigating regions should focus the first element, which I will attempt in a follow-up PR. Maybe we should also consider merging regions with the
Alt+F10shortcut?Testing Instructions
Pressing ctrl+` should cycle through the block toolbar as well.
Testing Instructions for Keyboard
Screenshots or screencast