Add visible focus ring for keyboard-focused semantics nodes#186540
Open
BilalRehman08 wants to merge 2 commits into
Open
Add visible focus ring for keyboard-focused semantics nodes#186540BilalRehman08 wants to merge 2 commits into
BilalRehman08 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a visible focus ring for keyboard-navigated semantics nodes in the web engine to comply with WCAG 2.4.7. It adds logic to AccessibilityFocusManager to manage a focus ring element and defines its CSS styles in StyleManager. Feedback indicates that the focus ring implementation should be updated to support multi-view by making the ring element and its management methods instance-based rather than static. Additionally, the positioning logic needs to account for moving elements, and the visibility logic in the unmanage method should be conditional to avoid incorrectly hiding the ring.
flt-semantics nodes live inside a filter:opacity(0%) subtree, so a plain CSS outline on those elements is invisible. Added a singleton overlay div (.flt-focus-ring) as a sibling of flt-semantics-host inside flutter-view, outside the filtered subtree, repositioned via getBoundingClientRect() on every DOM focus event and hidden on blur. Fixes flutter#186044
- Make _focusRing an instance field instead of static so each view gets its own ring in multi-view setups - Convert _ensureFocusRing to an instance method using _owner directly - Guard _hideFocusRing in stopManaging to only hide when the element being unmanaged is actually the active element, preventing accidental hiding of a different view's focus ring during cleanup
5b738e6 to
98b73e6
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #186044
Problem
flt-semanticsnodes withtabindex="0"receive keyboard focus but show no visible indicator, failing WCAG 2.4.7 (Focus Visible). A plain CSSoutlinerule on those elements does not work because the rootflt-semanticsnode hasfilter: opacity(0%)applied — CSS filters composite the entire subtree, making any outline on descendant elements invisible too.Solution
Added a singleton overlay
div(.flt-focus-ring) appended as a direct child offlutter-view, outside theflt-semantics-hostsubtree and therefore unaffected by the opacity filter. On every DOM focus event,AccessibilityFocusManagerpositions this overlay over the focused element usinggetBoundingClientRect()and hides it again on blur or when the node stops being managed.Files changed:
engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart— focus ring show/hide logic inAccessibilityFocusManagerengine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/style_manager.dart—focusRingClassconstant + CSS rule (position: fixed,pointer-events: none,outline: 3px solid Highlight,z-index: 9999)engine/src/flutter/lib/web_ui/test/engine/view_embedder/style_manager_test.dart— test verifying focus ring CSS is appliedPre-launch Checklist
///).