Skip to content

fix(button): prevent false ripples during scroll deceleration (#4911) #5133

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
Priyanshu0007 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(button): prevent false ripples during scroll deceleration (#4911) #5133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44e1129
fix(button): prevent false ripples during scroll deceleration (#4911)
Priyanshu0007 Sep 26, 2026
459e4a5
fix(button): address review comments and strengthen responder tests
Priyanshu0007 Oct 31, 2026
70af55f
fix(button): tag unstable_pressDelay native-only and add fast-tap test
Priyanshu0007 Nov 3, 2026
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
8 changes: 8 additions & 0 deletions src/components/Button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ export type Props = Omit<ViewProps, 'style'> & {
* The number of milliseconds a user must touch the element before executing `onLongPress`.
*/
delayLongPress?: number;
/**
* Delay in ms, from the start of the touch, before the press feedback (ripple) is shown (Native-only).
* Quick taps shorter than this delay will still fire `onPress` but might not show the ripple effect.
* Recommended values for buttons in scrollable lists are between 50ms and 100ms.

@MikitasK MikitasK Nov 2, 2026

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.

[nit] could we tag this native-only, like background is marked "(Android)" above?
react-native-web doesn't implement unstable_pressDelay, so does nothing on web

*/
unstable_pressDelay?: number;

@MikitasK MikitasK Oct 30, 2026

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.

could we check how this lands against #5097 ? that PR is actively rewriting this file & already covers all 3 places this PR edits, so it'll definitely conflict.
since 6.x is moving Button to Expressive, what do you think about rebasing on it?

@MikitasK MikitasK Oct 30, 2026 •
edited
Loading

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.

[blocker] another thing to flag: Build package CI step will fail for this PR (I reproduced it locally)

API leak guard in scripts/typescript-output-lint.ts sees unstable_pressDelay in generated Button.d.ts & flags it as unexpected RN prop.
Now that Button defines it, could we just remove it from unusedViewProps at scripts/typescript-output-lint.ts ?

@Priyanshu0007 Priyanshu0007 Oct 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Makes total sense to coordinate with #5097. I'm happy to either rebase this branch directly onto feat/button-material-3-expressive now, or wait until #5097 lands in main and rebase then whichever makes review and merging easier for you. What's the current timeline for #5097?

/**
* Style of button's inner content.
* Use this prop to apply custom height and width, to set a custom padding or to set the icon on the right with `flexDirection: 'row-reverse'`.
Expand Down Expand Up @@ -179,6 +185,7 @@ const Button = ({
onPressOut,
onLongPress,
delayLongPress,
unstable_pressDelay,
style,
theme: themeOverrides,
uppercase: uppercaseProp,
Expand Down Expand Up @@ -327,6 +334,7 @@ const Button = ({
onPressIn={hasPassedTouchHandler ? handlePressIn : undefined}
onPressOut={hasPassedTouchHandler ? handlePressOut : undefined}
delayLongPress={delayLongPress}
unstable_pressDelay={unstable_pressDelay}

@MikitasK MikitasK Oct 30, 2026

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.

what do you think about fast-tap case?
with a delay set, tap shorter than it still fires onPress but with no visible feedback at all - close to the mirror image of original complaint.
could we document the trade-off on the prop? and/or suggest a smaller value maybe?

@Priyanshu0007 Priyanshu0007 Oct 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done,
I've added a note explaining this trade-off (that fast taps shorter than the delay will still trigger onPress but might not show the ripple animation) and recommended using small values (e.g. 50–100ms) to strike the right balance between scroll cancellation and touch feedback.

aria-label={ariaLabel}
accessibilityHint={accessibilityHint}
role={role}
Expand Down
100 changes: 99 additions & 1 deletion src/components/__tests__/Button.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { StyleSheet } from 'react-native';
import { describe, expect, it, jest } from '@jest/globals';
import { userEvent } from '@testing-library/react-native';

import { render, screen } from '../../test-utils';
import { act, fireEvent, render, screen } from '../../test-utils';
import { pink500, white } from '../../theme/colors';
import { DarkTheme, LightTheme } from '../../theme/schemes';
import { tokens } from '../../theme/tokens';
Expand Down Expand Up @@ -98,6 +98,104 @@ it('renders active button if only onLongPress handler is passed', async () => {
expect(screen.getByTestId('active-button')).toBeEnabled();
});

const createResponderEvent = () => ({
persist: () => {},
nativeEvent: { timestamp: Date.now() },
currentTarget: {
measure: (callback: any) => {
callback(0, 0, 100, 100, 0, 0);
},
},
});

it('suppresses onPressIn when touch is terminated before unstable_pressDelay', async () => {
const onPressIn = jest.fn();
const onPress = jest.fn();

await render(
<Button
testID="delay-button"
unstable_pressDelay={100}
onPressIn={onPressIn}
onPress={onPress}
>
Delay Button
</Button>
);

const button = screen.getByTestId('delay-button');

await fireEvent(button, 'responderGrant', createResponderEvent());

@MikitasK MikitasK Nov 2, 2026

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.

could we add one more test for the most common case - quick tap that actually completes?

right now 2 tests cover:

  1. touch ends before the delay via responderTerminate -> no ripple, no press
  2. touch is held past the delay -> ripple shows, press fires

what's missing is in-between: user taps for less than the delay & lifts finger normally (responderRelease).

That's the everyday fast tap, and it's the case new docblock promises works: onPress fires, only ripple is skipped.
so could we add: grant -> advanceTimersByTime(60) -> responderRelease, then expect onPress to have been called & onPressIn not to have been called

await act(() => {
jest.advanceTimersByTime(50);
});
await fireEvent(button, 'responderTerminate', createResponderEvent());
await act(() => {
jest.advanceTimersByTime(100);
});

expect(onPressIn).not.toHaveBeenCalled();
expect(onPress).not.toHaveBeenCalled();
});

it('triggers onPressIn after unstable_pressDelay when touch is not terminated', async () => {
const onPressIn = jest.fn();
const onPress = jest.fn();

await render(
<Button
testID="delay-button"
unstable_pressDelay={100}
onPressIn={onPressIn}
onPress={onPress}
>
Delay Button
</Button>
);

const button = screen.getByTestId('delay-button');

await fireEvent(button, 'responderGrant', createResponderEvent());
expect(onPressIn).not.toHaveBeenCalled();

await act(() => {
jest.advanceTimersByTime(100);
});

expect(onPressIn).toHaveBeenCalledTimes(1);

await fireEvent(button, 'responderRelease', createResponderEvent());
expect(onPress).toHaveBeenCalledTimes(1);
});

it('triggers onPress for fast taps shorter than unstable_pressDelay', async () => {
const onPressIn = jest.fn();
const onPress = jest.fn();

await render(
<Button
testID="delay-button"
unstable_pressDelay={100}
onPressIn={onPressIn}
onPress={onPress}
>
Delay Button
</Button>
);

const button = screen.getByTestId('delay-button');

await fireEvent(button, 'responderGrant', createResponderEvent());
await act(() => {
jest.advanceTimersByTime(60);
});
expect(onPressIn).not.toHaveBeenCalled();

await fireEvent(button, 'responderRelease', createResponderEvent());
expect(onPress).toHaveBeenCalledTimes(1);
expect(onPressIn).toHaveBeenCalledTimes(1);
});

it('renders button with color', async () => {
const tree = (
await render(<Button textColor={pink500}>Custom Button</Button>)
Expand Down
100 changes: 99 additions & 1 deletion src/components/__tests__/TouchableRipple.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { GestureResponderEvent } from 'react-native';
import { describe, expect, it, jest } from '@jest/globals';
import { userEvent } from '@testing-library/react-native';

import { render, screen } from '../../test-utils';
import { act, fireEvent, render, screen } from '../../test-utils';
import TouchableRipple from '../TouchableRipple/TouchableRipple.native';

describe('TouchableRipple', () => {
Expand Down Expand Up @@ -44,6 +44,104 @@ describe('TouchableRipple', () => {
expect(onPress).not.toHaveBeenCalled();
});

const createResponderEvent = () => ({
persist: () => {},
nativeEvent: { timestamp: Date.now() },
currentTarget: {
measure: (callback: any) => {
callback(0, 0, 100, 100, 0, 0);
},
},
});

it('suppresses onPressIn when touch is terminated before unstable_pressDelay', async () => {
const onPressIn = jest.fn();
const onPress = jest.fn<(event: GestureResponderEvent) => void>();

await render(
<TouchableRipple
testID="delay-ripple"
unstable_pressDelay={100}
onPressIn={onPressIn}
onPress={onPress}
>
<Text>Button</Text>
</TouchableRipple>
);

const button = screen.getByTestId('delay-ripple');

await fireEvent(button, 'responderGrant', createResponderEvent());
await act(() => {
jest.advanceTimersByTime(50);
});
await fireEvent(button, 'responderTerminate', createResponderEvent());
await act(() => {
jest.advanceTimersByTime(100);
});

expect(onPressIn).not.toHaveBeenCalled();
expect(onPress).not.toHaveBeenCalled();
});

it('triggers onPressIn after unstable_pressDelay when touch is not terminated', async () => {
const onPressIn = jest.fn();
const onPress = jest.fn<(event: GestureResponderEvent) => void>();

await render(
<TouchableRipple
testID="delay-ripple"
unstable_pressDelay={100}
onPressIn={onPressIn}
onPress={onPress}
>
<Text>Button</Text>
</TouchableRipple>
);

const button = screen.getByTestId('delay-ripple');

await fireEvent(button, 'responderGrant', createResponderEvent());
expect(onPressIn).not.toHaveBeenCalled();

await act(() => {
jest.advanceTimersByTime(100);
});

expect(onPressIn).toHaveBeenCalledTimes(1);

await fireEvent(button, 'responderRelease', createResponderEvent());
expect(onPress).toHaveBeenCalledTimes(1);
});

it('triggers onPress for fast taps shorter than unstable_pressDelay', async () => {
const onPressIn = jest.fn();
const onPress = jest.fn<(event: GestureResponderEvent) => void>();

await render(
<TouchableRipple
testID="delay-ripple"
unstable_pressDelay={100}
onPressIn={onPressIn}
onPress={onPress}
>
<Text>Button</Text>
</TouchableRipple>
);

const button = screen.getByTestId('delay-ripple');

await fireEvent(button, 'responderGrant', createResponderEvent());
await act(() => {
jest.advanceTimersByTime(60);
});
expect(onPressIn).not.toHaveBeenCalled();

await fireEvent(button, 'responderRelease', createResponderEvent());
expect(onPress).toHaveBeenCalledTimes(1);
expect(onPressIn).toHaveBeenCalledTimes(1);
});

describe('on iOS', () => {
Platform.OS = 'ios';

Expand Down
Loading