Skip to content

Commit

Permalink
Pressable: Minimum Press Duration
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yungsters authored and facebook-github-bot committed May 19, 2020
1 parent c8ed2db commit 4aaf629
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 2 deletions.
20 changes: 19 additions & 1 deletion Libraries/Pressability/Pressability.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -279,6 +284,7 @@ const DEFAULT_PRESS_RECT_OFFSETS = {
right: 20,
top: 20,
};
const DEFAULT_MIN_PRESS_DURATION = 130;

/**
* Pressability implements press handling capabilities.
Expand Down Expand Up @@ -393,6 +399,7 @@ export default class Pressability {
pageX: number,
pageY: number,
|}>;
_touchActivateTime: ?number;
_touchState: TouchState = 'NOT_RESPONDER';

constructor(config: PressabilityConfig) {
Expand Down Expand Up @@ -702,6 +709,7 @@ export default class Pressability {
pageX: touch.pageX,
pageY: touch.pageY,
};
this._touchActivateTime = Date.now();
if (onPressIn != null) {
onPressIn(event);
}
Expand All @@ -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);
Expand All @@ -719,6 +736,7 @@ export default class Pressability {
onPressOut(event);
}
}
this._touchActivateTime = null;
}

_measureResponderRegion(): void {
Expand Down
119 changes: 118 additions & 1 deletion Libraries/Pressability/__tests__/Pressability-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ const createMockPressEvent = (
describe('Pressability', () => {
beforeEach(() => {
jest.resetModules();
jest.spyOn(Date, 'now');
jest.spyOn(HoverState, 'isHoverEnabled');
});

Expand Down Expand Up @@ -505,6 +506,7 @@ describe('Pressability', () => {
handlers.onResponderRelease(createMockPressEvent('onResponderRelease'));

expect(config.onPress).toBeCalled();
jest.runOnlyPendingTimers();
expect(config.onPressOut).toBeCalled();
});
});
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -611,6 +724,7 @@ describe('Pressability', () => {

expect(config.onPressIn).toBeCalled();
expect(config.onPress).toBeCalled();
jest.runOnlyPendingTimers();
expect(config.onPressOut).toBeCalled();
});

Expand Down Expand Up @@ -648,6 +762,7 @@ describe('Pressability', () => {
handlers.onResponderRelease(createMockPressEvent('onResponderRelease'));

expect(config.onPress).toBeCalled();
jest.runOnlyPendingTimers();
expect(config.onPressOut).toBeCalled();
});
});
Expand Down Expand Up @@ -676,6 +791,7 @@ describe('Pressability', () => {

handlers.onResponderRelease(createMockPressEvent('onResponderRelease'));
expect(config.onPress).not.toBeCalled();
jest.runOnlyPendingTimers();
expect(config.onPressOut).toBeCalled();
});

Expand Down Expand Up @@ -733,6 +849,7 @@ describe('Pressability', () => {
handlers.onResponderRelease(createMockPressEvent('onResponderRelease'));

expect(config.onPress).toBeCalled();
jest.runOnlyPendingTimers();
expect(config.onPressOut).toBeCalled();
});
});
Expand Down

0 comments on commit 4aaf629

Please sign in to comment.