Repository navigation
[A11y][Web] Support isAccessibilityFocusBlocked flag on web - #182789
hannah-hyj wants to merge 2 commits into
Conversation
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request correctly implements support for the isAccessibilityFocusBlocked flag on the web platform by mapping it to the aria-hidden attribute in the SemanticRole base class. This ensures that nodes and their descendants are properly hidden from the accessibility tree when requested. The implementation follows existing patterns in the web engine and includes a relevant test case. However, please ensure that the PR description and pre-launch checklist are fully completed as required by the repository's contribution guidelines.
…cs.dart Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
|
||
| if (semanticsObject.isFlagsDirty) { | ||
| if (semanticsObject.flags.isAccessibilityFocusBlocked) { | ||
| setAttribute('aria-hidden', 'true'); |
There was a problem hiding this comment.
this will hide the entire subtree, not just this node. If this is set by AccessibilityFocusBlockType.blockNode this will be broken i believe
There was a problem hiding this comment.
Good point, @chunhtai.
Also, a few things stand out from the aria-hidden docs:
Warning: Do not use aria-hidden="true" on focusable elements.
They don't explain why, but this warning is highlighted in red to indicate it's important. For focusable nodes, it could be better to use the disabled or inert attribute.
Using aria-hidden="false" will not re-expose the element to assistive technology if any of its parents specify aria-hidden="true".
Once aria-hidden: true is set, there's no way to unhide the children.
There was a problem hiding this comment.
Warning: Do not use aria-hidden="true" on focusable elements.
My understanding is a focusable element should not be a11y unfocusable. We have the same check in the framework side, A semantics node should not be focusable and a11y unfocusable , but it can be unfocusable but a11y focusable. So it's wont be a problem.
Once aria-hidden: true is set, there's no way to unhide the children.
That's unfortunate. on the framework side we block a11y focus on node or subtree. I think we should either 1. annotate on the framework side that web only support block a11y focus for a subtree, blocking a node will block its children too or 2. just don't support block a11y focus on web. using the old way (dropping semantics nodes from the tree) in framework instead.
I prefer option 1 on this as blocking a subtree is the more common use case and the function to block a subtree is enough to replace BlockSemantics.
This is the design doc for isAccessibilityFocusBlocked to replace BlockSemantics: https://docs.google.com/document/d/1niJnShCR3-PIMYo1a9lWdVySBqr_wmXYPvBXntvcc0c/edit?tab=t.0
|
Hey @hannah-hyj! Is this still something we want to land? |
|
Closing this PR as I don't have immediate plan to land it. |
|
Any plans on getting back to this, or should a new issue be opened? |
|
I created a new issue for this: #191484 |
... using aria-hidden.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.