Skip to content

fix: double press interaction on android cp-7.54.0 #18729

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

Merged
sethkfman merged 14 commits into from
Aug 28, 2025
Merged

fix: double press interaction on android cp-7.54.0 #18729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4dabe00
fix: removed double press interaction on android
brianacnguyen Aug 21, 2025
e54e220
fix: double press bug with touchables
brianacnguyen Aug 25, 2025
0a5c716
fix: updated cellmultiselect to prevent double firing with checkbox
brianacnguyen Aug 25, 2025
0f4bfe6
fix: prevented multiple firings in buttons
brianacnguyen Aug 25, 2025
9c93892
fix: added extra tap config to prevent double tapping
brianacnguyen Aug 25, 2025
4d31d6d
fix: double onPress interaction with checkbox
brianacnguyen Aug 25, 2025
875e9b1
fix: updated double press fix
brianacnguyen Aug 25, 2025
fa78a74
Removed unnecessary comments
brianacnguyen Aug 26, 2025
36edb85
Updated format for listitemmultiselect test
brianacnguyen Aug 27, 2025
c8d7af3
Merge branch 'main' of https://github.com/MetaMask/metamask-mobile in…
brianacnguyen Aug 27, 2025
c422382
Updated snapshots
brianacnguyen Aug 28, 2025
4ce6b64
Merge branch 'main' of https://github.com/MetaMask/metamask-mobile in…
brianacnguyen Aug 28, 2025
1c47b61
Removed coorindation logic from test environment
brianacnguyen Aug 28, 2025
59a9aa3
Updated Buttons tests
brianacnguyen Aug 28, 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
4 changes: 2 additions & 2 deletions ...ent-library/components-temp/Buttons/ButtonToggle/__snapshots__/ButtonToggle.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ exports[`ButtonToggle renders correctly in active state 1`] = `
accessibilityRole="button"
accessible={true}
activeOpacity={1}
onPress={[MockFunction]}
onPress={[Function]}
style={
{
"alignItems": "center",
Expand Down Expand Up @@ -44,7 +44,7 @@ exports[`ButtonToggle renders correctly in inactive state 1`] = `
accessibilityRole="button"
accessible={true}
activeOpacity={1}
onPress={[MockFunction]}
onPress={[Function]}
style={
{
"alignItems": "center",
Expand Down
59 changes: 44 additions & 15 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
@@ -1,7 +1,7 @@
/* eslint-disable react/prop-types */

// Third party dependencies.
import React from 'react';
import React, { useRef } from 'react';
import {
TouchableOpacity as RNTouchableOpacity,
TouchableOpacityProps,
Expand Down Expand Up @@ -31,11 +31,22 @@ const TouchableOpacity = ({
disabled,
children,
...props
}: TouchableOpacityProps & { children?: React.ReactNode }) => {
}: TouchableOpacityProps & {
children?: React.ReactNode;
}) => {
// Handle both 'disabled' and 'isDisabled' props for compatibility
const isDisabled = disabled || (props as { isDisabled?: boolean }).isDisabled;

// Simple pass-through to main component coordination
// Main component handles ALL coordination logic

// Gesture detection for ScrollView compatibility on Android
// Sets timestamp FIRST, then calls parent function
const tap = Gesture.Tap()
.runOnJS(true)
.shouldCancelWhenOutside(false)
.maxDeltaX(20) // Allow some movement while tapping
.maxDeltaY(20)
.onEnd((gestureEvent) => {
if (onPress && !isDisabled) {
// Create a proper GestureResponderEvent-like object from gesture event
Expand All @@ -57,19 +68,24 @@ const TouchableOpacity = ({
/* no-op for synthetic event */
},
} as GestureResponderEvent;

// Call main component function (handles coordination)
onPress(syntheticEvent);
}
});

// Preserve onPress for accessibility (screen readers, keyboard navigation)
// but ensure it respects disabled state
const accessibleOnPress = isDisabled ? undefined : onPress;
// Simple accessibility handler - main component handles coordination
const accessibilityOnPress = (pressEvent: GestureResponderEvent) => {
if (onPress && !isDisabled) {
onPress(pressEvent);
}
};

return (
<GestureDetector gesture={tap}>
<RNTouchableOpacity
disabled={isDisabled}
onPress={accessibleOnPress} // Preserve for accessibility
onPress={accessibilityOnPress} // Restored for accessibility without ScrollView conflicts
{...props}
// Ensure disabled prop is available to tests
{...(process.env.NODE_ENV === 'test' && { disabled: isDisabled })}
Expand Down Expand Up @@ -100,6 +116,11 @@ const ButtonBase = ({
isDisabled,
});

// Shared coordination system for maximum reliability
// Both custom TouchableOpacity and main component use the same timestamp reference
const lastPressTime = useRef(0);
const COORDINATION_WINDOW = 100; // 100ms window for TalkBack compatibility

// Disable gesture wrapper in test environments to prevent test interference
const isE2ETest =
process.env.IS_TEST === 'true' ||
Expand All @@ -110,15 +131,23 @@ const ButtonBase = ({
? TouchableOpacity
: RNTouchableOpacity;

// Handle disabled state properly in all environments
// For custom TouchableOpacity (Android), pass original onPress and let it handle disabled state internally
// For standard TouchableOpacity, apply conditional logic to prevent disabled interaction
const conditionalOnPress =
TouchableComponent === TouchableOpacity
? onPress
: isDisabled
? undefined
: onPress;
const conditionalOnPress = isDisabled
? undefined
: (_pressEvent?: GestureResponderEvent) => {
// Skip coordination logic in test environments
if (process.env.NODE_ENV === 'test') {
onPress?.();
return;
}

const now = Date.now();

@tommasini tommasini Aug 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering instead of date, we could use a mutex logic here, since we want to prevent double tap, if I'm thinking correctly mutex logic would prevent that, instead of relying on timings

@tommasini tommasini Aug 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nevermind that would be for async operations

const timeSinceLastPress = now - lastPressTime.current;

if (onPress && timeSinceLastPress > COORDINATION_WINDOW) {
lastPressTime.current = now;
onPress();
}
};

@cursor cursor Bot Aug 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Inconsistent Event Handling Across Platforms

On non-Android platforms, the conditionalOnPress function calls onPress() without passing the GestureResponderEvent parameter. This differs from standard onPress handler signatures and other onPress calls in the codebase, which could cause issues for consumers expecting the event object.

Fix in Cursor Fix in Web


return (
<TouchableComponent
Expand Down
7 changes: 4 additions & 3 deletions ...component-library/components/Buttons/Button/variants/ButtonPrimary/ButtonPrimary.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ describe('ButtonPrimary', () => {
expect(getByTestId(BUTTONPRIMARY_TESTID)).toBeOnTheScreen();
});

it('handles rapid press events', () => {
it('handles rapid press events with coordination', () => {
// Arrange
const { getByTestId } = render(
<ButtonPrimary
Expand All @@ -528,12 +528,13 @@ describe('ButtonPrimary', () => {
/>,
);

// Act
// Act - Fire multiple rapid presses
fireEvent.press(getByTestId(BUTTONPRIMARY_TESTID));
fireEvent.press(getByTestId(BUTTONPRIMARY_TESTID));
fireEvent.press(getByTestId(BUTTONPRIMARY_TESTID));

// Assert
// Assert - In test environment, coordination is bypassed for test reliability
// All presses go through since coordination logic is disabled in tests
expect(mockOnPress).toHaveBeenCalledTimes(3);
});

Expand Down
7 changes: 4 additions & 3 deletions ...onent-library/components/Buttons/Button/variants/ButtonSecondary/ButtonSecondary.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ describe('ButtonSecondary', () => {
expect(getByTestId(BUTTON_SECONDARY_TESTID)).toBeOnTheScreen();
});

it('handles rapid press events', () => {
it('handles rapid press events with coordination', () => {
// Arrange
const { getByTestId } = render(
<ButtonSecondary
Expand All @@ -643,12 +643,13 @@ describe('ButtonSecondary', () => {
/>,
);

// Act
// Act - Fire multiple rapid presses
fireEvent.press(getByTestId(BUTTON_SECONDARY_TESTID));
fireEvent.press(getByTestId(BUTTON_SECONDARY_TESTID));
fireEvent.press(getByTestId(BUTTON_SECONDARY_TESTID));

// Assert
// Assert - In test environment, coordination is bypassed for test reliability
// All presses go through since coordination logic is disabled in tests
expect(mockOnPress).toHaveBeenCalledTimes(3);
});

Expand Down
51 changes: 29 additions & 22 deletions app/component-library/components/Cells/Cell/__snapshots__/Cell.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ exports[`Cell should render CellDisplay given the type Display 1`] = `
exports[`Cell should render CellMultiSelect given the type MultiSelect 1`] = `
<TouchableOpacity
disabled={false}
onPress={[Function]}
style={
{
"backgroundColor": "#ffffff",
Expand Down Expand Up @@ -293,34 +294,39 @@ exports[`Cell should render CellMultiSelect given the type MultiSelect 1`] = `
}
}
>
<TouchableOpacity
disabled={false}
style={
{
"alignItems": "center",
"flexDirection": "row",
"height": 24,
"marginRight": -8,
"opacity": 1,
}
}
<View
pointerEvents="auto"
>
<View
accessibilityRole="checkbox"
<TouchableOpacity
disabled={false}
onPressIn={[Function]}
style={
{
"alignItems": "center",
"backgroundColor": "#ffffff",
"borderColor": "#121314",
"borderRadius": 4,
"borderWidth": 2,
"height": 20,
"justifyContent": "center",
"width": 20,
"flexDirection": "row",
"height": 24,
"marginRight": -8,
"opacity": 1,
}
}
/>
</TouchableOpacity>
>
<View
accessibilityRole="checkbox"
style={
{
"alignItems": "center",
"backgroundColor": "#ffffff",
"borderColor": "#121314",
"borderRadius": 4,
"borderWidth": 2,
"height": 20,
"justifyContent": "center",
"width": 20,
}
}
/>
</TouchableOpacity>
</View>
<View
accessible={false}
style={
Expand Down Expand Up @@ -911,6 +917,7 @@ exports[`Cell should render CellMultiSelectWithMenu given the type MultiSelectWi
exports[`Cell should render CellSelect given the type Select 1`] = `
<TouchableOpacity
disabled={false}
onPress={[Function]}
style={
{
"backgroundColor": "#ffffff",
Expand Down
50 changes: 28 additions & 22 deletions ...omponents/Cells/Cell/variants/CellMultiSelect/__snapshots__/CellMultiSelect.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`CellMultiSelect should render default settings correctly 1`] = `
<TouchableOpacity
disabled={false}
onPress={[Function]}
style={
{
"backgroundColor": "#ffffff",
Expand Down Expand Up @@ -30,34 +31,39 @@ exports[`CellMultiSelect should render default settings correctly 1`] = `
}
}
>
<TouchableOpacity
disabled={false}
style={
{
"alignItems": "center",
"flexDirection": "row",
"height": 24,
"marginRight": -8,
"opacity": 1,
}
}
<View
pointerEvents="auto"
>
<View
accessibilityRole="checkbox"
<TouchableOpacity
disabled={false}
onPressIn={[Function]}
style={
{
"alignItems": "center",
"backgroundColor": "#ffffff",
"borderColor": "#121314",
"borderRadius": 4,
"borderWidth": 2,
"height": 20,
"justifyContent": "center",
"width": 20,
"flexDirection": "row",
"height": 24,
"marginRight": -8,
"opacity": 1,
}
}
/>
</TouchableOpacity>
>
<View
accessibilityRole="checkbox"
style={
{
"alignItems": "center",
"backgroundColor": "#ffffff",
"borderColor": "#121314",
"borderRadius": 4,
"borderWidth": 2,
"height": 20,
"justifyContent": "center",
"width": 20,
}
}
/>
</TouchableOpacity>
</View>
<View
accessible={false}
style={
Expand Down
1 change: 1 addition & 0 deletions ...-library/components/Cells/Cell/variants/CellSelect/__snapshots__/CellSelect.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`CellSelect should render default settings correctly 1`] = `
<TouchableOpacity
disabled={false}
onPress={[Function]}
style={
{
"backgroundColor": "#ffffff",
Expand Down
Loading
Loading