feat: add debounce functionality to TouchableRipple#4828
feat: add debounce functionality to TouchableRipple#4828BohdanKhv wants to merge 1 commit intocallstack:mainfrom
Conversation
- Add optional debounce prop to prevent rapid successive presses - Implement debounce logic in both web and native TouchableRipple components - Add comprehensive test interface in TouchableRipple example - Update documentation with usage examples - Maintain backward compatibility with existing code
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| expect(onPress).toHaveBeenCalledTimes(2); | ||
|
|
||
| jest.useRealTimers(); | ||
| }); |
There was a problem hiding this comment.
Bug: Unmocked Time Improves Debounce Testing Reliability
The debounce test uses jest.useFakeTimers() and jest.advanceTimersByTime(), but the debounce implementation relies on Date.now(). Since Date.now() isn't mocked by Jest's fake timers by default, advanceTimersByTime() doesn't affect the debounce logic's internal clock. This makes the test unreliable and unable to accurately verify the debounce behavior.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a debounce feature to the TouchableRipple component to prevent rapid successive presses. The debounce functionality uses a time-based approach that ignores press events occurring within a specified time window after the first press.
- Adds optional
debounceprop (in milliseconds) to TouchableRipple component - Implements debouncing logic using
Date.now()and a ref to track last press time - Includes comprehensive test coverage for debounced and non-debounced behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/components/TouchableRipple/TouchableRipple.tsx | Added debounce prop, implementation logic, and type annotations for PressableStateCallbackType |
| src/components/TouchableRipple/TouchableRipple.native.tsx | Added debounce prop and implementation logic for native platform |
| src/components/tests/TouchableRipple.test.tsx | Added tests for debounce functionality with fake timers |
| example/src/Examples/TouchableRippleExample.tsx | Enhanced example with interactive debounce demonstration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(onPress).toHaveBeenCalledTimes(1); | ||
|
|
||
| // Fast forward time past debounce window | ||
| jest.advanceTimersByTime(400); |
There was a problem hiding this comment.
The test advances timers by 400ms, but the debounce implementation uses Date.now() which is not affected by jest.advanceTimersByTime(). This test will likely fail because Date.now() returns real time, not fake timer time. Mock Date.now() using jest.spyOn(Date, 'now') and control its return value, or use jest.setSystemTime() if using modern fake timers.
| onPressOut={handlePressOut} | ||
| disabled={disabled} | ||
| style={(state) => [ | ||
| style={(state: PressableStateCallbackType) => [ |
There was a problem hiding this comment.
[nitpick] Adding explicit type annotation here is unnecessary since TypeScript can infer the type from the Pressable component's props. This change adds no value and increases verbosity. Consider removing the type annotation unless there's a specific type inference issue.
| ]} | ||
| > | ||
| {(state) => | ||
| {(state: PressableStateCallbackType) => |
There was a problem hiding this comment.
[nitpick] Adding explicit type annotation here is unnecessary since TypeScript can infer the type from the Pressable component's children render prop. This change adds no value and increases verbosity. Consider removing the type annotation unless there's a specific type inference issue.
| {(state: PressableStateCallbackType) => | |
| {state => |
| onPress={debouncedOnPress} | ||
| > | ||
| {({ pressed }) => ( | ||
| {({ pressed }: { pressed: boolean }) => ( |
There was a problem hiding this comment.
[nitpick] Adding inline type annotation for a destructured parameter is unnecessarily verbose. TypeScript can infer this type from the Pressable component's children render prop. Consider removing the type annotation unless there's a specific type inference issue.
| {({ pressed }: { pressed: boolean }) => ( | |
| {({ pressed }) => ( |
|
Hey @BohdanKhv, thank you for your pull request 🤗. The documentation from this branch can be viewed here. |
2 participants
Motivation
Had to write extra code just to implement this in my apps
Note
Add optional debounce to
TouchableRipple(web/native), wire it toonPress, and update examples and tests.debounceprop toTouchableRipple(web:src/components/TouchableRipple/TouchableRipple.tsx, native:src/components/TouchableRipple/TouchableRipple.native.tsx).debouncedOnPressusingDate.now()+ ref; hook intoPressableviaonPress={debouncedOnPress}.PressableStateCallbackTypeon web.example/src/Examples/TouchableRippleExample.tsxwith a scrollable demo showcasing basic ripple and a debounce test (normal vs debounced buttons, counters, reset).src/components/__tests__/TouchableRipple.test.tsxto cover debounced vs non-debounced presses and existing behaviors.Written by Cursor Bugbot for commit c77ad85. This will update automatically on new commits. Configure here.