fix: android flicker on a11y tree traversal#3635
Open
isekovanic wants to merge 2 commits into
Open
Conversation
Contributor
SDK Size
|
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
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.
🎯 Goal
This PR fixes an Android only full screen flash that occurs when closing the message context menu, unfortunately introduced alongside the focus trap work in #3634. Fortunately though it was caught early.
The flash is the Android framework rewalking the wrapped subtree's accessibility nodes when
importantForAccessibilityflips back off onOverlayA11yShield. That rewalk lands on the same frame asreact-native-teleport's reparent back of the message subtree and the closing portal hosts, and is visible as a fullscreen flicker. The gallery doesn't show it for two reasons: it doesn't use the teleport mechanism on close and its fullscreen overlay visually masks the same window.The native rewalk isn't avoidable while the prop toggles - but most users don't need the prop to toggle at all. The trap exists to keep
TalkBack/Switch Access/Voice Accessfocus inside the overlay; everyone else gains nothing from it and shouldn't pay for it.🛠 Implementation details
Three layer gate on whether the shield's
importantForAccessibilityever flips:accessibility.enabled). Moved to theOverlayProvidercall site so the shield isn't even mounted in the tree when the integrator hasn't opted intoa11y. Apps that don't passaccessibility={{ enabled: true }}toOverlayProvidersee zero overhead and no extraView, no hook subscriptions, no native prop ever touched. This naturally means that if we change theaccessibility.enabledconfiguration on the fly we'll see a remount but we anyway do not recommend doing this nor is this something intended to go back and forth.a11yservice running. The newuseAccessibilityServiceEnabledhook at mirrors the shape of the existinguseScreenReaderEnabledexcept it covers all services. Wraps Android'sAccessibilityInfo.isAccessibilityServiceEnabled- which reports anya11yservice, not just screen readers, so the trap covers TalkBack, Switch Access, Voice Access, Select to Speak, etc. RN does not emit a dedicated change event for this signal, so the hook repolls onscreenReaderChangedand onAppStateforeground transitions (the realistic path for a user togglingAccessibilitysettings and returning to the app).All three are checked in the new private
useShouldActivateTraphook inOverlayA11yShield.tsx. The wrapper stays unconditionally mounted on Android (within theaccessibility.enabledbranch); only theimportantForAccessibilityprop toggles. Mounting the wrapper unconditionally was deliberate - gating the wrapper itself on per overlay state remounts the entire chat subtree on every menu open/close, which manifests as a visible chat tree reset. Also remounting when screen readers get enabled/disabled feels off, so for now we'll stick to this.If we think about it we only need the trap for when
a11yservices are enabled, not particularly in the normal scenarios. Its sole purpose is anyway to not let youra11ycursor get away from the overlays as compensation for the lack ofaccessibilityViewIsModalas we have for iOS.Also adds
pointerEvents='none'on the wrapper for good measure so it can never intercept a touch from sibling overlays.🎨 UI Changes
iOS
Android
🧪 Testing
☑️ Checklist
developbranch