-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: Add initial support for screen readers (experimental) #9280
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
feat: Add initial support for screen readers (experimental) #9280
Conversation
There might need to be a bit of branch-fu in order to make the tests work correctly between this new long-lived core branch and the equivalent version in the keyboard navigation repo. |
Next steps here:
|
core/block_svg.ts
Outdated
aria.setState(svgPath, aria.State.ROLEDESCRIPTION, 'block'); | ||
aria.setRole(svgPath, aria.Role.TREEITEM); | ||
this.recomputeAriaLabel(); | ||
svgPath.tabIndex = -1; | ||
this.currentConnectionCandidate = null; | ||
|
||
this.doInit_(); |
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 aria bit needs to be after doInit_()
for the block to have fields.
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 catch. The dataflow of when things happen has changed each time the code was moved, so I missed this and somehow missed the voice output being incomplete for blocks.
Addresses a reviewer comment.
d0ad934
into
google:add-screen-reader-support-experimental
The basics
The details
Resolves
Fixes part of #8207
Fixes part of #3370
Proposed Changes
This introduces initial broad ARIA integration in order to enable at least basic screen reader support when using keyboard navigation.
Largely this involves introducing ARIA roles and labels in a bunch of places, sometimes done in a way to override normal built-in behaviors of the accessibility node tree in order to get a richer first-class output for Blockly (such as for blocks and workspaces).
Reason for Changes
ARIA is the fundamental basis for configuring how focusable nodes in Blockly are represented to the user when using a screen reader. As such, all focusable nodes requires labels and roles in order to correctly communicate their contexts.
The specific approach taken in this PR is to simply add labels and roles to all nodes where obvious with some extra work done for
WorkspaceSvg
andBlockSvg
in order to represent blocks as a tree (since that seems to be the best fitting ARIA role per those available: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles). The custom work specifically for blocks includes:One note on some of the labels being nonsensical (e.g. 'DoNotOverride?'): this was done intentionally to try and ensure all focusable nodes (that can be focused) have labels, even when the specifics of what that label should be aren't yet clear. More components had these temporary labels until testing revealed how exactly they would behave from a screen reader perspective (at which point their roles and labels were updated as needed). The temporary labels act as an indicator when navigating through the UI, and some of the nodes can't easily be reached (for reasons) and thus may never actually need a label. More work is needed in understanding both what components need labels and what those labels should be, but that will be done beyond this PR.
Test Coverage
No tests are added to this as it's experimental and not a final implementation.
The keyboard navigation tests are failing due to a visibility expansion of
connectionCandidate
inBlockDragStrategy
. There's no way to avoid this breakage, unfortunately. Instead, this PR will be merged and then google/blockly-keyboard-experimentation#684 will be finalized and merged to fix it. There's some additional work that will happen both in that branch and in a later PR in core Blockly to integrate the two experimentation branches as part of #9283 so that CI passes correctly for both branches.Documentation
No documentation is needed at this time.
Additional Information
This work is experimental and is meant to serve two purposes:
This code should never be merged into
develop
as it stands. Instead, it will be redesigned with maintainability, testing, and correctness in mind at a future date (see google/blockly-keyboard-experimentation#673).