Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Android TalkBack interactions with Pressable by adapting the internal Pressable state machine to the different event ordering produced when accessibility services are active.
Changes:
- Added a new native module API (
isAccessibilityEnabled) to detect accessibility state from JS. - Adjusted Android Pressable state machine event ordering when accessibility is enabled.
- Prevented an Android-only state-machine reset on
onTouchesUpwhen accessibility is enabled (to avoid breaking long-press flows under TalkBack).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-native-gesture-handler/src/v3/components/Pressable.tsx | Skips the Android onTouchesUp reset path when accessibility is enabled. |
| packages/react-native-gesture-handler/src/specs/NativeRNGestureHandlerModule.ts | Extends the TurboModule spec with isAccessibilityEnabled(): boolean. |
| packages/react-native-gesture-handler/src/components/Pressable/stateDefinitions.ts | Adds an Android accessibility-specific state config and selects it when accessibility is enabled. |
| packages/react-native-gesture-handler/src/RNGestureHandlerModule.web.ts | Adds a web stub for isAccessibilityEnabled (currently unimplemented). |
| packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt | Implements isAccessibilityEnabled() on Android via AccessibilityManager. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...e-handler/android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerModule.kt
Outdated
Show resolved
Hide resolved
| if ( | ||
| Platform.OS === 'android' && | ||
| !RNGestureHandlerModule.isAccessibilityEnabled() | ||
| ) { |
There was a problem hiding this comment.
This calls the native isAccessibilityEnabled() synchronously on every onTouchesUp. Since this value typically changes rarely, consider reading it once (e.g., on mount) and caching it in a ref/state to avoid repeated sync native calls on a hot interaction path.
packages/react-native-gesture-handler/src/components/Pressable/stateDefinitions.ts
Outdated
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/RNGestureHandlerModule.web.ts
Outdated
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/specs/NativeRNGestureHandlerModule.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (Platform.OS === 'android') { | ||
| if (isScreenReaderEnabled()) { | ||
| return getAndroidAccessibilityStatesConfig(handlePressIn, handlePressOut); | ||
| } | ||
| return getAndroidStatesConfig(handlePressIn, handlePressOut); |
There was a problem hiding this comment.
getStatesConfig selects the Android state-machine configuration based on the screen reader status at the time this function is called. In both Pressable implementations, the state machine config is set once in a useEffect that does not re-run when the screen reader status changes, so toggling TalkBack while the app is running can leave Pressable using the wrong event-order config until remount. Consider wiring screen-reader status as reactive state (subscribe in the component and re-run setStates when it changes) or making getStatesConfig accept the current status as a parameter from the component.
| export function isScreenReaderEnabled(): boolean { | ||
| if (isScreenReaderEnabledCache === null) { | ||
| isScreenReaderEnabledCache = RNGestureHandlerModule.isScreenReaderEnabled(); | ||
| } | ||
| return isScreenReaderEnabledCache; |
There was a problem hiding this comment.
isScreenReaderEnabled() calls RNGestureHandlerModule.isScreenReaderEnabled() on first use, but the Jest mock used in jestSetup.js (src/mocks/module.tsx) doesn’t currently define this new method. Any tests (or consumer test code) that call into this utility (directly or via Pressable on Android) will throw TypeError: ... isScreenReaderEnabled is not a function. Add a default mock implementation (e.g. returning false) to the mocked module to keep the test environment consistent with the new public API.
| isScreenReaderEnabled() { | ||
| // NO-OP | ||
| }, |
There was a problem hiding this comment.
isScreenReaderEnabled() is declared to return a boolean but this Windows stub currently returns undefined (no return statement). That can break callers like isScreenReaderEnabled() in src/utils.ts (cache becomes undefined) and may cause incorrect branching. Return an explicit boolean (likely false) to keep behavior predictable on Windows.
j-piasecki
left a comment
There was a problem hiding this comment.
Have you tried to approach this from the other side? i.e., why are the events in a different order when talkback is enabled? Maybe it's a problem with the native implementation (especially since the changes seems targeted to Android)
| let isScreenReaderEnabledCache: boolean | null = null; | ||
|
|
||
| AccessibilityInfo.addEventListener('screenReaderChanged', () => { | ||
| isScreenReaderEnabledCache = RNGestureHandlerModule.isScreenReaderEnabled(); |
There was a problem hiding this comment.
Why do we need a new method in the module when there's one in React Native, and you can also just get the value from the event here?
| val accessibilityManager = reactApplicationContext.getSystemService( | ||
| Context.ACCESSIBILITY_SERVICE, | ||
| ) as AccessibilityManager? | ||
|
|
||
| return accessibilityManager?.isTouchExplorationEnabled ?: false | ||
| } |
There was a problem hiding this comment.
Don't we already have this defined in some utils or extensions file?
Description
When TalkBack is activated, pressable cannot be clicked on android. This PR fixes the issue.
The reason was that Pressable requires a specific order of events. This order of events is hardcoded and found empirically. However, with talback enabled the order of events is different, I added a module function which queries native side whether or not talback is enabled and adjusts expected event order accordingly.
Moreover with talback enabled longPress gets both onPointerUp and onPointerDown before native side receives its events, thus we must not reset the state machine state on onPointerUp.
Test plan
Tested on the following example:
Details