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 1 commit 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
1 commit
Select commit
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
6 changes: 6 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,10 @@ 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 is engaged and ripple is triggered.

@MikitasK MikitasK Oct 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[nit] I tested it out & seems like press still fires - only feedback is delayed
so maybe we could reword it to smth like "...before the press feedback (ripple) is shown"?

*/
unstable_pressDelay?: number;

@MikitasK MikitasK Oct 30, 2026

Copy link
Copy Markdown

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

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 ?

/**
* 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 +183,7 @@ const Button = ({
onPressOut,
onLongPress,
delayLongPress,
unstable_pressDelay,
style,
theme: themeOverrides,
uppercase: uppercaseProp,
Expand Down Expand Up @@ -327,6 +332,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

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?

aria-label={ariaLabel}
accessibilityHint={accessibilityHint}
role={role}
Expand Down
5 changes: 4 additions & 1 deletion src/components/TouchableRipple/TouchableRipple.native.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ const TouchableRipple = ({
...rest
}: Props) => {
const theme = useInternalTheme(themeOverrides);
const { rippleEffectEnabled } = React.useContext<Settings>(SettingsContext);
const { rippleEffectEnabled, ripplePressDelay } =
React.useContext<Settings>(SettingsContext);

const { onPress, onLongPress, onPressIn, onPressOut } = rest;

Expand Down Expand Up @@ -90,6 +91,7 @@ const TouchableRipple = ({
return (
<Pressable
{...rest}
unstable_pressDelay={rest.unstable_pressDelay ?? ripplePressDelay}
ref={ref}
disabled={disabled}
style={[useForeground && styles.overflowHidden, style]}
Expand All @@ -103,6 +105,7 @@ const TouchableRipple = ({
return (
<Pressable
{...rest}
unstable_pressDelay={rest.unstable_pressDelay ?? ripplePressDelay}
ref={ref}
disabled={disabled}
style={[borderless && styles.overflowHidden, style]}
Expand Down
4 changes: 3 additions & 1 deletion src/components/TouchableRipple/TouchableRipple.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ const TouchableRipple = ({
typeof calculatedRippleColor === 'string'
? color(calculatedRippleColor).fade(0.5).rgb().string()
: calculatedRippleColor;
const { rippleEffectEnabled } = React.useContext<Settings>(SettingsContext);
const { rippleEffectEnabled, ripplePressDelay } =
React.useContext<Settings>(SettingsContext);

const { onPress, onLongPress, onPressIn, onPressOut } = rest;

Expand Down Expand Up @@ -276,6 +277,7 @@ const TouchableRipple = ({
return (
<Pressable
{...rest}
unstable_pressDelay={rest.unstable_pressDelay ?? ripplePressDelay}
ref={ref}
onPressIn={handlePressIn}
onPressOut={handlePressOut}
Expand Down
13 changes: 13 additions & 0 deletions src/components/__tests__/Button.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,19 @@ it('renders active button if only onLongPress handler is passed', async () => {
expect(screen.getByTestId('active-button')).toBeEnabled();
});

it('renders button with unstable_pressDelay and triggers onPress', async () => {

@MikitasK MikitasK Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[blocker] could we strengthen these?

I tried running this test after reverting changes in Button.tsx. Result - test passed, so seems like nothing here actually guards the delay.
userEvent.press resolves regardless of it. also RNL 14 removed UNSAFE_getByType that's why prop assertion isn't available either.

so wdyt about driving RN's responder state machine with fake timers asserting pressIn is suppressed on RESPONDER_TERMINATED?

const onPress = jest.fn();

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

await userEvent.press(screen.getByTestId('delay-button'));
expect(onPress).toHaveBeenCalledTimes(1);
});

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

import { SettingsContext } from '../../core/settings';
import { render, screen } from '../../test-utils';
import TouchableRipple from '../TouchableRipple/TouchableRipple.native';

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

it('triggers onPress when unstable_pressDelay is provided', async () => {
const onPress = jest.fn<(event: GestureResponderEvent) => void>();
await render(
<TouchableRipple unstable_pressDelay={100} onPress={onPress}>
<Text>Button</Text>
</TouchableRipple>
);

await userEvent.press(screen.getByText('Button'));

expect(onPress).toHaveBeenCalledTimes(1);
});

it('triggers onPress when ripplePressDelay is provided in SettingsContext', async () => {
const onPress = jest.fn<(event: GestureResponderEvent) => void>();
await render(
<SettingsContext.Provider
value={{ rippleEffectEnabled: true, ripplePressDelay: 80 }}
>
<TouchableRipple onPress={onPress}>
<Text>Button</Text>
</TouchableRipple>
</SettingsContext.Provider>
);

await userEvent.press(screen.getByText('Button'));

expect(onPress).toHaveBeenCalledTimes(1);
});

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

Expand Down
1 change: 1 addition & 0 deletions src/core/settings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export type Settings = {
testID,
}: IconProps) => React.ReactNode;
rippleEffectEnabled?: boolean;
ripplePressDelay?: number;

@MikitasK MikitasK Oct 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could we keep this PR to per-component unstable_pressDelay prop & leave global ripplePressDelay for separate discussion?

#4911 is about a button inside scrolling list, but this setting changes every TouchableRipple-based component app-wide the moment someone sets it on PaperProvider.
it's also not fully global: Card uses its own Pressable with unstable_pressDelay={0} so pressable Card in a list would still misbehave.

for what it's worth, #3806 (same problem) was solved at the call site rather than with library-wide default. the wiring itself looks fine & consistent with rippleEffectEnabled - mainly wanted to keep this PR's surface area small

};

export const SettingsContext = React.createContext<Settings>({
Expand Down
Loading