-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Add screen reader support experimental (DO NOT MERGE) #9532
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
Draft
BenHenning
wants to merge
58
commits into
main
Choose a base branch
from
add-screen-reader-support-experimental
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or 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
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## 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` and `BlockSvg` 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: - Overriding the role description to be 'block' rather than 'tree item' (which is the default). - Overriding the position, level, and number of sibling counts since those are normally determined based on the DOM tree and blocks are not laid out in the tree the same way they are visually or logically (so these computations were incorrect). This is also the reason for a bunch of extra computation logic being introduced. 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` in `BlockDragStrategy`. There's no way to avoid this breakage, unfortunately. Instead, this PR will be merged and then RaspberryPiFoundation/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: - Provide a foundation for testing and iterating the core screen reader experience in Blockly. - Provide a reference point for designing a long-term solution that accounts for all requirements collected during user testing. 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 RaspberryPiFoundation/blockly-keyboard-experimentation#673).
…(experimental) (#9284) ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9283 ### Proposed Changes Updates the keyboard navigation plugin workflow to point to the screen reader experimentation branch. ### Reason for Changes This ensures that both the plugin (via RaspberryPiFoundation/blockly-keyboard-experimentation#684) and core Blockly correctly link their experimental screen reader branches against each other to ensure CI passes correctly for both. ### Test Coverage No new tests are needed here. ### Documentation N/A ### Additional Information Similar to #9280, this PR will never be merged directly into `develop` so the workflow changes will remain isolated to the experimental branch.
chore: merge develop into add-screen-reader Merge pull request #9352 from google/develop
* fix: Improve representation of `FlyoutButtons` in screenreaders. * fix: Remove aria-selected.
chore: merge develop into add-screen-reader
* fix: Use field ARIA labels for field-only blocks. * chore: Make the linter happy.
* fix: improve screenreader output for workspace comments * fix: run npm run messages to fully add new message * fix: remove useless bubble label * fix: use roledescription instead of description
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9386 Fixes part of #9293 ### Proposed Changes Addresses #9386 through a number of changes: - Ensures the flyout contents are reevaluated for ARIA changes whenever they themselves change (since previously `WorkspaceSvg` only recomputed its ARIA properties when one of its blocks self-registered which doesn't account for labels or buttons). - Collapsible categories are now correctly wrapped by a group (since groups of tree items must be in a parent group). - Updates toolbox ARIA computations to be on content changes rather than having items self-specify recomputing the ARIA properties. This mitigates an issue with collapsible categories not updating the toolbox contents and thus being omitted. - Introduced a separate pathway for computing tree info for flyout workspaces (vs. for the main workspace) in order to account for `FlyoutButton`s. - Updated `FlyoutButton` to use a nested structure of `treeitem` then `button` in order to actually count correctly in the tree and still be an interactive button. The readout experience is actually better now on ChromeVox, as well, since it reads out _both_ 'Button' and 'Tree item' which is interesting. It seems screen readers are designed to look for this pattern but it only works if set up in a very particular way. ### Reason for Changes Most of the changes here fixed incidental problems noticed while trying to address #9386 (such as the variables category not correctly accounting for the 'Create variable' button in the count, or not having the correct labels). Much of the count issues in the original issue were caused by a combination of missing some flyout items, and trying to compute the labels too early (i.e. before the categories were fully populated). ### Test Coverage Since this is an experimental change, no new tests were added. ### Documentation No documentation changes are directly needed here. ### Additional Information None.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #8206 Fixes #8210 Fixes #8213 Fixes #8255 Fixes #8211 Fixes #8212 Fixes #8254 Fixes part of #9301 Fixes part of #9304 ### Proposed Changes This PR completes the remaining ARIA roles and properties needed for all core fields. Specifically: - #8206: A better name needed to be used for the checkbox value, plus there was an ARIA property missing for actually representing the checkbox state. The latter needed to be updated upon toggling the checkbox, as well. These changes bring checkbox fields in compliance with the ARIA checkbox pattern documented here: https://www.w3.org/WAI/ARIA/apg/patterns/checkbox/. - #8210: This one required a lot of changes in order to adapt to the ARIA combobox pattern documented here: https://www.w3.org/WAI/ARIA/apg/patterns/combobox/. Specifically: - Menus needed to have a unique ID that's also exposed in order to link the combobox element to its menu when open. - ARIA's `activedescendant` proved very useful in ensuring that the current dropdown selection is correctly read when the combobox has focus but its menu isn't opened. - The default properties available for options (label and value) aren't very good for readout, so a custom ARIA property was added for much clearer option readouts. This is only demonstrated for the math arithmetic block for now. - The text element is normally hidden for ARIA but it's useful in conjunction with `activedescendant` to represent the current value selection. - Images have been handled here as well (partly as part of #8255) by leveraging their alt text for readouts. This actually seems to work quite well both for current value and selection. - #8213: Much of the improvements here come from the combobox (`FieldDropdown`) improvements explained above. However one additional bit was done to provide an explicit 'Variable <name>' readout for the purpose of clarity. This demonstrates some contextualization of the value of the field which may be a generally useful pattern to copy in other field contexts. - #8255: Image fields have been refined since they were redundantly specifying 'image' when an `image` ARIA role is already being used. Now only the alt text is supplied along with the role context. Note that images need special handling since they can sometimes be navigable (such as when they have click handlers). - #8211: Text input fields have had their labeling improved like all other fields, and the field's value is now exposed via its `text` element since this will show up as a `StaticText` node in the accessibility tree and automatically be read as part of the field's value. - #8212: This gets the same benefits as the previous point since those improvements were included for both text and number input. However, existing `valuemin` and `valuemax` ARIA properties have been removed. It seems these are really only useful when introducing a slider mechanism (see https://www.w3.org/WAI/ARIA/apg/patterns/slider/) and from testing seems to not really be utilized for the basic text input that `FieldNumber` currently uses. It may be the case that this is a better pattern to use in the future, but it's more likely that other custom fields could benefit from more specific patterns like slider rather than `FieldNumber` being changed in that way. - #8254 and part of #9304: Field labels have been completely removed from the accessibility node tree since they can never be navigated to (as #8254 explains all labels will be included as part of the block's ARIA label itself for readout parity with navigation options). Note that it doesn't cover external fields (such as those supplied in blockly-samples), nor does it fully set up the infrastructure work for those. Ultimately that work needs to happen as part of #9301. Beyond the role work above, this PR also introduces some fundamental work for #9301. Specifically: - It demonstrates how block definitions could be used to introduce accessibility label customizations (in this case for the options of the arithmetic operator block's drop-down field, plus the block itself). - It sets up some central label computation for all fields, though more thought is needed on whether this is sufficient for custom fields outside of core Blockly and on how to properly contextualize labels for field values. Core Blockly's fields are fairly simple for representing values which is why that aspect of #9301 didn't need to be solved in this PR. Note that the field labeling here is being used to improve all of the fields above, but also it tries to aggressively fall back to the _next best_ label to be used (though it's possible to run out of options which is why fields still need contextually-specific fallbacks). ### Reason for Changes Generally the initial approach for implementing labels is leveraging as specific ARIA roles as exist to directly represent the element. This PR is completing that work for all of core Blockly's built-in fields, and laying some of the groundwork for generalizing this support for custom fields. Having specific roles does potentially introduce inconsistencies across screen readers (though should improve consistency across sites for a single screen reader), and expectations for behaviors (like shortcuts) that may need to be ignored or only partially supported (#9313 is discussing this). ### Test Coverage Only manual testing has been completed since this is experimental work. Video demonstrating most of the changes: [Screen recording 2025-10-01 4.05.35 PM.webm](https://github.com/user-attachments/assets/c7961caa-eae0-4585-8fd9-87d7cbe65988) ### Documentation N/A -- Experimental work. ### Additional Information This has only been tested on ChromeVox.
* fix: set activedescendant correctly on toolbox * fix: dont manually set posinset for toolbox categories * fix: dont set activedescendant on toolbox at all
…a textual label. (#9401)
Read value-inputs and fields in place and recursively. Announce block shape, number of inputs and number of children where appropriate. Co-authored-by: Matt Hillsdon <matt.hillsdon@microbit.org>
* fix: Improve narration and navigation of C-shaped blocks. * chore: Satisfy the linter. * chore: Refactor and comment `getBlockNavigationCandidates()`. * refactor: Reduce code duplication in `LineCursor`. * fix: Add missing case when labeling connections.
Also, use regions for identifiying toolbox, workspace, and flyout.
* fix: Miscellaneous improvements for screenreader support. * fix: Include field name in ARIA label. * fix: Update block ARIA labels when inputs are shown/hidden. * fix: Make field row label generation more robust.
* fix: Make up/previous navigation consistent with down/next. * fix: Don't visit nested input blocks when moving up/previous.
…tal-branch chore: Merge develop into experimental branch (experimental)
…der-issues ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9301 Fixes #9312 Fixes #9313 Fixes part of #9304 ### Proposed Changes This introduces a variety of specific changes to resolve several issues for screen reader work, including introducing fundamental support for field labeling. Specifically: - Field labels have been simplified to only use their custom defined ARIA name otherwise they are null (and thus should be ignored for readout purposes) which wraps up the remaining high-level work for #9301 (#9450 tracks more specific follow-up work to improve upon what's been established at this point). The PR also introduces an ARIA override for number inputs in math blocks so that the readout is correct for them. - Bubble labeling is more explicit now which is useful for mutators (#9312), warnings, and comments. The general improvement for bubbles wraps up the remaining work for #9313 as well since the core issue was resolved in #9351. By default a bubble has no ARIA label. - #9304 is partly being addressed here with the change to field images: they are no longer being added to the accessibility node tree unless they are actually navigable (that is, clickable). Part of #9304's goal is to remove extraneous nodes. - Finally, a typo was fixed for 'replaceable blocks' since these were not reading out correctly. This was noticed in passing and isn't directly related to the other issues. ### Reason for Changes This PR is largely being used as a basis for one particularly significant issue: #9301. Field labeling has undergone several iterations over the past few months and the team seems comfortable sticking with a "do as little as possible" approach when determining the label, thus justifying the need for expecting more specific customization (i.e. #9450). To this end it's important to be clear that getting fields to a good state is not actually "done" but the need to track it as a large incomplete thing has ended. Note that one important part of #9301 was updating field plugins to be accessible--this largely seems unnecessary as-is as it will be completely dependent on the needs of future user tests. The long-term plan will need to account for making all fields in `blockly-samples` accessible (per #9307). Some of the terminology used here (e.g. for bubbles) will likely need to change after user testing, but it's important to establish that _something_ correct is communicated even if the terminology may require scaffolding and/or refinement. It's important to note that while non-clickable field images are no longer in the node graph, their ARIA presence still exists as part of the fluent block labeling solution. That is, `FieldImage`'s alt text is used as part of constructing a fluent block label (sometimes to confusing effect--see #9452). ### Test Coverage No tests needed since these are experimental changes and do not change existing test behaviors. ### Documentation No documentation changes are needed for these experimental changes. ### Additional Information None.
…to-latest-develop chore: Update screen reader branch to latest develop Bypassing branch protection because this commit fixes broken a broken build.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9466 ### Proposed Changes Ensures `FieldInput`'s ARIA labels are recomputed when an invalid value is attempted to be set. ### Reason for Changes Previously the `FieldInput` would continuously update its ARIA label as the value changed, including for invalid values. If the editor was cancelled this would correctly revert but if an invalid value was attempted to be saved then it would cancel the update but not correct the ARIA label. ### Test Coverage No new tests are needed for this experimental change. This has been manually verified locally using `FieldNumber` in Core's advanced playground. ### Documentation No documentation changes are needed for this experimental change. ### Additional Information None.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9461 ### Proposed Changes Remove image-specific ARIA properties (role and label) for the current selected item in `FieldDropdown`. ### Reason for Changes This intentionally changes a behavior introduced in #9384 because the alternative ARIA behavior works better. As the images in #9461 show the actual focus outline when the current drop-down value has ARIA properties goes to that element (separate from active focus in this case) since the screen reader will read out those properties. However, this makes the reader lose context on the combo box itself. This basically goes away from leveraging active descendant (though that's still necessary for correct combo box implementation) and just forces the label in a way that keeps the combo box context. Text-based combo boxes already do this, so this change simply brings image-based items in alignment with text combo box behaviors. ### Test Coverage No new automated tests are needed for this experimental change. Manual testing was done via core Blockly's advanced playground (using the 'test blocks' toolbox). ### Documentation No documentation changes are needed. ### Additional Information None.
## The basics - [ ] ~I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)~ (I can't really due to the nature of this change--I have to rely on tests and external testing) ## The details ### Resolves Fixes part of #9456 (remainder will be in a change in the keyboard navigation plugin) ### Proposed Changes Introduce and use new `Block` function for retrieving a configurably constrained singleton field for a given block. The constraints allow for some level of configuring (such as whether to isolate to only full or editable blocks). The existing simple reporter function has been retrofitted to use this new function, instead. ### Reason for Changes This expanded support fixes the underlying use case. Separately, this change reveals two noteworthy details: 1. There's inconsistency in the codebase as to when the singleton field needs to be editable, a full-block field, both, and neither. It would be ideal to make this consistent. Interestingly, the documentation for `isSimpleReporter` seems to have been wrong since it wasn't actually fulfilling its contract of returning an editable field (this has been retained for callsites except where the check was already happening). 2. There's a possible recursion case now possible between `getSingletonFullBlockField` and `isFullBlockField` due to `FieldInput`'s `isFullBlockField` depending on `isSimpleReporter`. Ideally this would be changed in the future to avoid that potential recursion risk (possibly as part of #9307). ### Test Coverage No new automated tests are needed for this experimental work. Manual testing mainly comprised of cursory navigation and readout checks for single-field blocks to make sure nothing breaks. More thorough testing is difficult in core since the specific situation of multiple fields don't have a corresponding block to use in the playground to verify. Automated tests are also being heavily relied on for correctness since all of the nuance behind the simple reporter cases would require a deeper testing pass. ### Documentation No new documentation needed for this experimental work. ### Additional Information None.
* feat: Add comma-separation to ARIA labels * fix: remove unneeded complexity of returned comma-separated string
* fix: Don't include count of inputs in block ARIA label * fix: Handle single input case
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9451 ### Proposed Changes Removes the ARIA `region` role for both flyouts and workspaces that are within mutators. ### Reason for Changes The use of the `region` role only adds confusion and slightly messes up region announcements for screen readers. `generic` has been used instead since it's the default container role (e.g. for `div`) and seems sufficient for what needs to be described in this case. Note that the delayed initialization for the flyout role is due to flyout initialization happening a bit later than its workspace DOM creation (so it doesn't seem possible to check for mutator status yet). There might be ways of doing this a bit more cleanly as part of #9307. ### Test Coverage No automated tests are needed for this experimental change. Manual testing comprised of navigating between the main workspace, the main workspace's toolbox and flyout, and a mutator workspace and flyout to validate that no unusual region readouts were happening. The accessibility node tree was also analyzed to verify that `generic` is correctly being applied as the role for the mutator workspace and flyout. ### Documentation No new documentation is needed for this experimental change. ### Additional Information This doesn't fully resolve all region issues, but it resolves the main ones (especially when combined with #9483 for NVDA). The main remaining problem at this point is that the main workspace itself is usually not read out as a region and it's not clear why. I suspect it has something to do with focus manager and how it automatically moves focus, but I'm not entirely sure what specific mechanism is causing the problem since both toolbox and flyout do something similar and don't have the same issue (flyout is particularly noteworthy since it's a workspace in itself). There may be some other focus oddities happening to cause the difference but, for now, this seems reasonable. If testing or user feedback find that the lack of consistent region readout is problematic for the main workspace then a new issue can be opened and investigated separately.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9464 ### Proposed Changes Adds ARIA labels for `Toolbox` and `Flyout` ('s `Workspace`) containers to complement their existing region labels. ### Reason for Changes In some cases certain screen readers (like NVDA) don't read out region labels. Previously, only the region labels provided context for currently being within the toolbox or flyout, so adding additional labels on the ARIA `tree` containers themselves ensures that these contexts are always read if the tree's context is read. This adds redundancy with the region output if both are read, but ChromeVox at least adds some delay between finishing the tree's context and beginning the region's context and this seems to mesh well together. The fully read context now feels like: "here's an item, it's the toolbox tree, and that tree is within the toolbox region." Even though both regions only contain a single tree it doesn't feel dimorphic. ### Test Coverage No new automated tests are needed for this experimental change. This has been manually tested with ChromeVox to demonstrate the dual tree + region output for both toolbox and flyout: [Screen recording 2025-11-26 2.52.22 PM.webm](https://github.com/user-attachments/assets/aa43ae81-9da6-4c79-b0fc-120146892aae) ### Documentation No new documentation is needed for this experimental change. ### Additional Information This was only tested on ChromeVox.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes part of RaspberryPiFoundation/blockly-keyboard-experimentation#764 Fixes part of #9450 (infrastructure needs) ### Proposed Changes Introduces support for two new "where am I?" shortcuts for helping to provide location context for users: - `I`: re-reads the current selected block with full verbosity (i.e. also includes the block's field types with their values in the readout). - `shift+I`: reads the current selected block's parent with full verbosity. Note that this includes some functional changes to `Field` to allow for more powerful customization of a field's ARIA representation (by splitting up value and type), though a field's value defaults potentially to null which will be ignored in the final ARIA computed label. This seems necessary per the discussion here: https://github.com/RaspberryPiFoundation/blockly/pull/9470/files#r2541508565 but more consideration may be needed here as part of #9307. Some limitations in the new shortcuts: - They will not read out anything if a block is not selected (e.g. for fields and icons). - They read out input blocks when the input block is selected. - They cannot read out anything while in move mode (due to the behavior here in the plugin which automatically cancels moves if an unknown shortcut is pressed: https://github.com/RaspberryPiFoundation/blockly-keyboard-experimentation/blob/a36f3662b05c2ddcd18bde8745777fff8dc3df31/src/actions/mover.ts#L166-L191). - The readout is limited by the problems of dynamic ARIA announcements (per #9460). ### Reason for Changes RaspberryPiFoundation/blockly-keyboard-experimentation#764 provides context on the specific needs addressed here. ### Test Coverage Self tested. No new automated tests needed for experimental work. ### Documentation No new documentation needed for experimental work. ### Additional Information This was spun out of #9470 with the intent of getting shortcuts initially working checked in even if the entirety of the experience is incomplete.
…9508) * feat: Use custom ARIA roledescriptions for different block types * fix: Don't include 'begin stack' in block ARIA labels in the flyout * fix: Fix visibility of `computeAriaLabel`
chore: Merge `main` into `add-screen-reader-support-experimental`
* feat: Use custom ARIA roledescriptions for different block types * fix: Denote shadow blocks in the label rather than role description
* feat: Make navigation looping configurable * chore: Add TODO to clean up API
* feat: Add support for playing beeps * chore: Fix typo
* feat: Include parent category in verbose block descriptions * fix: Loosen typechecking to break import cycle * chore: Make the linter happy
* fix: read parent hierarchy with shift+i * fix: use a default argument instead of optional
* feat: Improve annonucements for unconstrained. * chore: Lint fixes.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9495 ### Proposed Changes Changes a bunch of ARIA role & label management to ensure that `Flyout` acts like a list rather than a tree. ### Reason for Changes `Flyout`s are always hierarchically flat so it doesn't make sense to model them as a tree. Instead, a menu is likely a better fit per https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/menu_role: > A `menu` generally represents a grouping of common actions or functions that the user can invoke. The `menu` role is appropriate when a list of menu items is presented in a manner similar to a menu on a desktop application. Submenus, also known as pop-up menus, also have the role `menu`. However, there are important caveats that need to be considered and addressed: - As discussed below, menus introduce some unexpected compatibility issues with VoiceOver so this PR presently uses `list` and `listitem`s as a slightly more generic enumerating alternative for menus. - Menus (and to some extent lists) are stricter\* than trees in that they seem to impose a requirement that `menuitem`s cannot contain interactive elements (they are expected to be interactive themselves). This has led to a few specific changes: - Block children are now hidden when in the flyout (since they aren't navigable anyway). - Flyout buttons are themselves now the `menuitem` rather than their container parent, and they no longer use the role button. - Menus aren't really expected to contain labels but it isn't inherently disallowed. This is less of an issue with lists. - Because everything must be a `listitem` (or a few more specific alternatives) both blocks and buttons lack some context. Since not all `Flyout` items can be expected to be interactive, buttons and blocks have both had their labels updated to include an explicit indicator that they are buttons and blocks, respectively. Note that this does possibly go against convention for buttons in particular but it seems fine since this is an unusual (but seemingly correct) utilization of a `list` element. - To further provide context on blocks, the generated label for blocks in the `Flyout` is now its verbose label rather than the more compact form. \* This is largely a consequence of a few specific attributes of `menuitem` and `menu`s as a whole and very likely also applies to `tree`s and `treeitem`s (and `list`s and `listitems`s). However, now seemed like a good time to fix this especially in case some screen readers get confused rather than ignore nested interactive controls/follow semantic cloaking per the spec. Demo of it working on VoiceOver (per @gonfunko -- note this was the `menu` variant rather than the `list` variant of the PR):  ### Test Coverage This has been manually tested with ChromeVox. No automated tests are needed as part of this experimental change. ### Documentation No new documentation changes are needed for this experimental change. ### Additional Information None.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9304 ### Proposed Changes Fixes non-visual parent roles and rendered connection navigation policy. ### Reason for Changes Other PRs have made progress on removing extraneous accessibility nodes with #9446 being essentially the last of these. Ensuring that parent/child relationships are correct is the last step in ensuring that the entirety of the accessibility node graph is correctly representing the DOM and navigational structure of Blockly. This can have implications and (ideally) improvements for certain screen reader modes that provide higher-level summarization and sometimes navigation (bypassing Blockly's keyboard navigation) since it avoids an incorrect flat node structure and instead ensures correct hierarchy and ordering. It was discovered during the development of the PR that setting `aria-owns` properties to ensure that all focusable accessibility nodes have the correct parent/child relationships (particularly for blocks) isn't actually viable per the analysis summarized in this comment: #9449 (comment). At a high level introducing these relationships seems to actually cause problems in both ChromeVox and Voiceover. Part of the analysis discovered that nodes set with the `presentation` role aren't going to behave correctly due to the spec ignoring that role if any children of such elements are focusable, so this PR does change those over to `generic` which is more correct. They are still missing in Chrome's accessibility node viewer, and `generic` _seems_ to introduce slightly better `group` behaviors on VoiceOver (that is, it seems to reduce some of the `group` announcements which VoiceOver is known for over-specifying). Note that some tests needed to be updated to ensure that they were properly rendering blocks (in order for `RenderedConnection.canBeFocused()` to behave correctly) in the original implementation of the PR. Only one test actually changed in behavior because it seemed like it was incorrect before--the particular connection being tested wasn't actually navigable and the change to `canBeFocused` actually enforces that. These changes were kept even though the behaviors weren't needed anymore since it's still a bit more correct than before. Overall, #9304 is closed here because the tree seems to be about as good as it can get with current knowledge (assuming no other invalid roles need to be fixed, but that can be addressed in separate issues as needed). ### Test Coverage No automated tests are needed for this since it's experimental but it has been manually tested with both ChromeVox and Voiceover. ### Documentation No documentation changes are needed for these experimental changes. ### Additional Information Note that there are some limitations with this approach: text editors and listboxes (e.g. for comboboxes) are generally outside of the hierarchy represented by the Blockly workspace. This is an existing issue that remains unaffected by these changes, and fixing it to be both ARIA compliant and consistent with the DOM may not be possible (though it doesn't seem like there's a strong requirement to maintain DOM and accessibility node tree hierarchical relationships). The analysis linked above also considered introducing a top-level `application` role which might change some of the automated behaviors of certain roles but this only seemed to worsen local testing with ChromeVox so it was excluded.
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.
The basics
The details
Resolves
Fixes
Proposed Changes
Reason for Changes
Test Coverage
Documentation
Additional Information