From 49b2a6e9cd95b74375f51fda87b87430073ec20e Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Mon, 18 May 2020 21:25:29 -0700 Subject: [PATCH] Pressable: Minimum Press Duration Summary: When a `Pressable` has a configured (or the default) `delayPressIn` and no (or the default) `delayPressOut`, tapping very quickly can lead to intantaneous invocation of `onPressIn` and `onPressOut`. The end result is that users may never experience any intended visual press feedback. This changes `Pressable` to accept (and be preconfigured with a default) **minimum press duration**. The minimum press duration ensures that even if the press is released before `delayPressIn` has elapsed, `onPressOut` will still wait the remaining time up to `minPressDuration` before firing. Note that setting a non-zero `delayPressOut` is insufficient because if a user holds down on a `Pressable` for longer than `delayPressIn`, we still want `onPressOut` to fire immediately when the press is released. Changelog: [General][Changed] - Added `minPressDuration` to `Pressable`. Reviewed By: TheSavior Differential Revision: D21614708 fbshipit-source-id: 502f3d8ad6a40e7762435b6df16809c8798dd92c --- Libraries/Pressability/Pressability.js | 20 ++- .../__tests__/Pressability-test.js | 119 +++++++++++++++++- 2 files changed, 137 insertions(+), 2 deletions(-) diff --git a/Libraries/Pressability/Pressability.js b/Libraries/Pressability/Pressability.js index 6a078eda849..37555d5df73 100644 --- a/Libraries/Pressability/Pressability.js +++ b/Libraries/Pressability/Pressability.js @@ -78,6 +78,11 @@ export type PressabilityConfig = $ReadOnly<{| */ delayPressOut?: ?number, + /** + * Minimum duration to wait between calling `onPressIn` and `onPressOut`. + */ + minPressDuration?: ?number, + /** * Called after the element loses focus. */ @@ -279,6 +284,7 @@ const DEFAULT_PRESS_RECT_OFFSETS = { right: 20, top: 20, }; +const DEFAULT_MIN_PRESS_DURATION = 130; /** * Pressability implements press handling capabilities. @@ -393,6 +399,7 @@ export default class Pressability { pageX: number, pageY: number, |}>; + _touchActivateTime: ?number; _touchState: TouchState = 'NOT_RESPONDER'; constructor(config: PressabilityConfig) { @@ -702,6 +709,7 @@ export default class Pressability { pageX: touch.pageX, pageY: touch.pageY, }; + this._touchActivateTime = Date.now(); if (onPressIn != null) { onPressIn(event); } @@ -710,7 +718,16 @@ export default class Pressability { _deactivate(event: PressEvent): void { const {onPressOut} = this._config; if (onPressOut != null) { - const delayPressOut = normalizeDelay(this._config.delayPressOut); + const minPressDuration = normalizeDelay( + this._config.minPressDuration, + 0, + DEFAULT_MIN_PRESS_DURATION, + ); + const pressDuration = Date.now() - (this._touchActivateTime ?? 0); + const delayPressOut = Math.max( + minPressDuration - pressDuration, + normalizeDelay(this._config.delayPressOut), + ); if (delayPressOut > 0) { this._pressOutDelayTimeout = setTimeout(() => { onPressOut(event); @@ -719,6 +736,7 @@ export default class Pressability { onPressOut(event); } } + this._touchActivateTime = null; } _measureResponderRegion(): void { diff --git a/Libraries/Pressability/__tests__/Pressability-test.js b/Libraries/Pressability/__tests__/Pressability-test.js index e523df9ceab..52c35d00c1f 100644 --- a/Libraries/Pressability/__tests__/Pressability-test.js +++ b/Libraries/Pressability/__tests__/Pressability-test.js @@ -232,6 +232,7 @@ const createMockPressEvent = ( describe('Pressability', () => { beforeEach(() => { jest.resetModules(); + jest.spyOn(Date, 'now'); jest.spyOn(HoverState, 'isHoverEnabled'); }); @@ -505,6 +506,7 @@ describe('Pressability', () => { handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); expect(config.onPress).toBeCalled(); + jest.runOnlyPendingTimers(); expect(config.onPressOut).toBeCalled(); }); }); @@ -578,7 +580,118 @@ describe('Pressability', () => { }); }); - // TODO: onPressOut tests + describe('onPressOut', () => { + it('is called after `onResponderRelease` before `delayPressIn`', () => { + const {config, handlers} = createMockPressability(); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + expect(config.onPressIn).not.toBeCalled(); + handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); + + expect(config.onPressOut).not.toBeCalled(); + jest.runOnlyPendingTimers(); + expect(config.onPressOut).toBeCalled(); + }); + + it('is called after `onResponderRelease` after `delayPressIn`', () => { + const {config, handlers} = createMockPressability(); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + jest.runOnlyPendingTimers(); + expect(config.onPressIn).toBeCalled(); + handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); + + expect(config.onPressOut).not.toBeCalled(); + jest.runOnlyPendingTimers(); + expect(config.onPressOut).toBeCalled(); + }); + + it('is not called after `onResponderTerminate` before `delayPressIn`', () => { + const {config, handlers} = createMockPressability(); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + handlers.onResponderTerminate( + createMockPressEvent('onResponderTerminate'), + ); + + expect(config.onPressOut).not.toBeCalled(); + jest.runOnlyPendingTimers(); + expect(config.onPressOut).not.toBeCalled(); + }); + + it('is not called after `onResponderTerminate` after `delayPressIn`', () => { + const {config, handlers} = createMockPressability(); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + jest.runOnlyPendingTimers(); + expect(config.onPressIn).toBeCalled(); + handlers.onResponderTerminate( + createMockPressEvent('onResponderTerminate'), + ); + + expect(config.onPressOut).not.toBeCalled(); + jest.runOnlyPendingTimers(); + expect(config.onPressOut).toBeCalled(); + }); + + it('is called after the minimum press duration by default', () => { + const {config, handlers} = createMockPressability(); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + jest.runOnlyPendingTimers(); + expect(config.onPressIn).toBeCalled(); + handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); + + jest.advanceTimersByTime(120); + expect(config.onPressOut).not.toBeCalled(); + jest.advanceTimersByTime(10); + expect(config.onPressOut).toBeCalled(); + }); + + it('is called after only after the remaining minimum press duration', () => { + const {config, handlers} = createMockPressability(); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + jest.runOnlyPendingTimers(); + expect(config.onPressIn).toBeCalled(); + // WORKAROUND: Jest does not advance `Date.now()`. + const touchActivateTime = Date.now(); + jest.advanceTimersByTime(120); + Date.now.mockReturnValue(touchActivateTime + 120); + handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); + + expect(config.onPressOut).not.toBeCalled(); + jest.advanceTimersByTime(10); + expect(config.onPressOut).toBeCalled(); + }); + + it('is called synchronously if minimum press duration is 0ms', () => { + const {config, handlers} = createMockPressability({ + minPressDuration: 0, + }); + + handlers.onStartShouldSetResponder(); + handlers.onResponderGrant(createMockPressEvent('onResponderGrant')); + handlers.onResponderMove(createMockPressEvent('onResponderMove')); + jest.runOnlyPendingTimers(); + expect(config.onPressIn).toBeCalled(); + handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); + + expect(config.onPressOut).toBeCalled(); + }); + }); describe('`onPress*` with movement', () => { describe('within bounds of hit rect', () => { @@ -611,6 +724,7 @@ describe('Pressability', () => { expect(config.onPressIn).toBeCalled(); expect(config.onPress).toBeCalled(); + jest.runOnlyPendingTimers(); expect(config.onPressOut).toBeCalled(); }); @@ -648,6 +762,7 @@ describe('Pressability', () => { handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); expect(config.onPress).toBeCalled(); + jest.runOnlyPendingTimers(); expect(config.onPressOut).toBeCalled(); }); }); @@ -676,6 +791,7 @@ describe('Pressability', () => { handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); expect(config.onPress).not.toBeCalled(); + jest.runOnlyPendingTimers(); expect(config.onPressOut).toBeCalled(); }); @@ -733,6 +849,7 @@ describe('Pressability', () => { handlers.onResponderRelease(createMockPressEvent('onResponderRelease')); expect(config.onPress).toBeCalled(); + jest.runOnlyPendingTimers(); expect(config.onPressOut).toBeCalled(); }); });