Skip to content

Testing enhanced gesture approach #21369

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+59 −26
Open

Testing enhanced gesture approach #21369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc9d9bc
Testing enhanced gesture approach
brianacnguyen Oct 17, 2025
76cefb2
Added extra vertical condition for bottomsheet gesture
brianacnguyen Oct 17, 2025
b518f47
Fixed offset issue
brianacnguyen Oct 17, 2025
183c4cf
fixed more offset errors
brianacnguyen Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...ry/components/BottomSheets/BottomSheet/foundation/BottomSheetDialog/BottomSheetDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ const BottomSheetDialog = forwardRef<
<PanGestureHandler
enabled={isInteractable}
onGestureEvent={gestureHandler}
shouldCancelWhenOutside={false}
activeOffsetY={[-10, 10]}
activeOffsetX={[-50, 50]}
>
<Animated.View
onLayout={updateSheetHeight}
Expand Down
28 changes: 19 additions & 9 deletions app/component-library/components/Buttons/Button/foundation/ButtonBase/ButtonBase.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,22 @@ export const TouchableOpacity = ({
// Handle both 'disabled' and 'isDisabled' props for compatibility
const isDisabled = disabled || (props as { isDisabled?: boolean }).isDisabled;

// Track accessibility state - start with null to indicate "unknown"
// Track accessibility state - start with false as default to ensure gesture handler works
const [isAccessibilityEnabled, setIsAccessibilityEnabled] = useState<
boolean | null
>(null);
>(false);

useEffect(() => {
// Check initial accessibility state
AccessibilityInfo.isScreenReaderEnabled().then(setIsAccessibilityEnabled);
AccessibilityInfo.isScreenReaderEnabled()
.then(setIsAccessibilityEnabled)
.catch((error) => {
// Log the error for debugging
console.warn('AccessibilityInfo.isScreenReaderEnabled failed:', error);
// Fallback to false - assume accessibility is OFF
// This ensures gesture handler will work in ScrollViews
setIsAccessibilityEnabled(false);
});

// Listen for accessibility changes
const subscription = AccessibilityInfo.addEventListener(
Expand All @@ -61,14 +69,16 @@ export const TouchableOpacity = ({
return () => subscription?.remove();
}, []);

// Gesture detection for ScrollView compatibility on Android
// Native gesture handler to prevent interruption from other gestures (BottomSheet pan, etc.)
const native = Gesture.Native().disallowInterruption(true);

// Gesture detection for ScrollView and BottomSheet compatibility on Android
const tap = Gesture.Tap()
.runOnJS(true)
.shouldCancelWhenOutside(false)
.maxDeltaX(20) // Allow some movement while tapping
.maxDeltaY(20)
.requireExternalGestureToFail() // Wait for other gestures to fail before activating
.maxDuration(300) // Tight constraint: must complete within 300ms
.maxDuration(200) // Shorter duration for better responsiveness
.minPointers(1)
.onEnd(
(
Expand Down Expand Up @@ -116,12 +126,12 @@ export const TouchableOpacity = ({
}

return (
<GestureDetector gesture={tap}>
<GestureDetector gesture={Gesture.Simultaneous(native, tap)}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Gesture Configuration Blocks Tap Gestures

The updated gesture configuration, combining Gesture.Simultaneous(native, tap) with native.disallowInterruption(true) and removing .requireExternalGestureToFail(), creates a conflict. This prevents tap gestures from executing, leading to unresponsive buttons and list items.

Additional Locations (2)

Fix in Cursor Fix in Web

<RNTouchableOpacity
disabled={isDisabled}
onPress={
isAccessibilityEnabled !== false && !isDisabled ? onPress : undefined
} // Use TouchableOpacity onPress when accessibility is ON or UNKNOWN (safer for accessibility users)
isAccessibilityEnabled === true && !isDisabled ? onPress : undefined
} // Use TouchableOpacity onPress only when accessibility is explicitly ON (safer for accessibility users)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Accessibility Race Condition Disables Touch Events

The updated accessibility logic introduces a critical issue where TouchableOpacity's onPress is disabled on mount. This happens because isAccessibilityEnabled now initializes to false and the onPress condition requires it to be true, causing the gesture handler to take over prematurely. This race condition can lead to unresponsive components during initialization. Additionally, if AccessibilityInfo.isScreenReaderEnabled() fails, isAccessibilityEnabled is explicitly set to false, which permanently disables onPress and breaks accessibility for screen reader users.

Additional Locations (2)

Fix in Cursor Fix in Web

{...props}
>
{children}
Expand Down
28 changes: 19 additions & 9 deletions app/component-library/components/List/ListItemMultiSelect/ListItemMultiSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,22 @@ const TouchableOpacity = ({
}) => {
const isDisabled = disabled || (props as { isDisabled?: boolean }).isDisabled;

// Track accessibility state - start with null to indicate "unknown"
// Track accessibility state - start with false as default to ensure gesture handler works
const [isAccessibilityEnabled, setIsAccessibilityEnabled] = useState<
boolean | null
>(null);
>(false);

useEffect(() => {
// Check initial accessibility state
AccessibilityInfo.isScreenReaderEnabled().then(setIsAccessibilityEnabled);
AccessibilityInfo.isScreenReaderEnabled()
.then(setIsAccessibilityEnabled)
.catch((error) => {
// Log the error for debugging
console.warn('AccessibilityInfo.isScreenReaderEnabled failed:', error);
// Fallback to false - assume accessibility is OFF
// This ensures gesture handler will work in ScrollViews
setIsAccessibilityEnabled(false);
});

// Listen for accessibility changes
const subscription = AccessibilityInfo.addEventListener(
Expand All @@ -55,14 +63,16 @@ const TouchableOpacity = ({
return () => subscription?.remove();
}, []);

// Gesture detection for ScrollView compatibility on Android
// Native gesture handler to prevent interruption from other gestures (BottomSheet pan, etc.)
const native = Gesture.Native().disallowInterruption(true);

// Gesture detection for ScrollView and BottomSheet compatibility on Android
const tap = Gesture.Tap()
.runOnJS(true)
.shouldCancelWhenOutside(false)
.maxDeltaX(20) // Allow some movement while tapping
.maxDeltaY(20)
.requireExternalGestureToFail() // Wait for other gestures to fail before activating
.maxDuration(300) // Tight constraint: must complete within 300ms
.maxDuration(200) // Shorter duration for better responsiveness
.minPointers(1)
.onEnd(
(
Expand Down Expand Up @@ -110,12 +120,12 @@ const TouchableOpacity = ({
}

return (
<GestureDetector gesture={tap}>
<GestureDetector gesture={Gesture.Simultaneous(native, tap)}>
<RNTouchableOpacity
disabled={isDisabled}
onPress={
isAccessibilityEnabled !== false && !isDisabled ? onPress : undefined
} // Use TouchableOpacity onPress when accessibility is ON or UNKNOWN (safer for accessibility users)
isAccessibilityEnabled === true && !isDisabled ? onPress : undefined
} // Use TouchableOpacity onPress only when accessibility is explicitly ON (safer for accessibility users)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Accessibility State Causes Component Unresponsiveness

Initializing isAccessibilityEnabled to false and changing the TouchableOpacity onPress condition to isAccessibilityEnabled === true removes a safe fallback. This causes components to rely solely on the gesture handler when accessibility state is false or unknown, leading to unresponsive buttons and list items, especially during initial loading or for accessibility users.

Additional Locations (2)

Fix in Cursor Fix in Web

{...props}
>
{children}
Expand Down
26 changes: 18 additions & 8 deletions app/component-library/components/List/ListItemSelect/ListItemSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,19 @@ const TouchableOpacity = ({
// Track accessibility state - start with null to indicate "unknown"
const [isAccessibilityEnabled, setIsAccessibilityEnabled] = useState<
boolean | null
>(null);
>(false);

useEffect(() => {
// Check initial accessibility state
AccessibilityInfo.isScreenReaderEnabled().then(setIsAccessibilityEnabled);
AccessibilityInfo.isScreenReaderEnabled()
.then(setIsAccessibilityEnabled)
.catch((error) => {
// Log the error for debugging
console.warn('AccessibilityInfo.isScreenReaderEnabled failed:', error);
// Fallback to false - assume accessibility is OFF
// This ensures gesture handler will work in ScrollViews
setIsAccessibilityEnabled(false);
});

// Listen for accessibility changes
const subscription = AccessibilityInfo.addEventListener(
Expand All @@ -54,14 +62,16 @@ const TouchableOpacity = ({
return () => subscription?.remove();
}, []);

// Gesture detection for ScrollView compatibility on Android
// Native gesture handler to prevent interruption from other gestures (BottomSheet pan, etc.)
const native = Gesture.Native().disallowInterruption(true);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Gesture Handler Recreated on TouchableOpacity Render

The native gesture handler is recreated on every render of the TouchableOpacity component. This causes unnecessary performance overhead and can lead to gesture state loss, potentially resulting in unresponsive button behavior.

Additional Locations (2)

Fix in Cursor Fix in Web


// Gesture detection for ScrollView and BottomSheet compatibility on Android
const tap = Gesture.Tap()
.runOnJS(true)
.shouldCancelWhenOutside(false)
.maxDeltaX(20) // Allow some movement while tapping
.maxDeltaY(20)
.requireExternalGestureToFail() // Wait for other gestures to fail before activating
.maxDuration(300) // Tight constraint: must complete within 300ms
.maxDuration(200) // Shorter duration for better responsiveness
.minPointers(1)
.onEnd(
(
Expand Down Expand Up @@ -109,12 +119,12 @@ const TouchableOpacity = ({
}

return (
<GestureDetector gesture={tap}>
<GestureDetector gesture={Gesture.Simultaneous(native, tap)}>
<RNTouchableOpacity
disabled={isDisabled}
onPress={
isAccessibilityEnabled !== false && !isDisabled ? onPress : undefined
} // Use TouchableOpacity onPress when accessibility is ON or UNKNOWN (safer for accessibility users)
isAccessibilityEnabled === true && !isDisabled ? onPress : undefined
} // Use TouchableOpacity onPress only when accessibility is explicitly ON (safer for accessibility users)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Accessibility Regression in Initial Render

The change to initialize isAccessibilityEnabled as false and update the TouchableOpacity onPress condition to isAccessibilityEnabled === true introduces an accessibility regression. During initial render, before the screen reader status is determined, users with screen readers will incorrectly use the gesture handler instead of the accessible TouchableOpacity.

Additional Locations (2)

Fix in Cursor Fix in Web

{...props}
>
{children}
Expand Down
Loading