Skip to content

Commit 1f8b698

Browse files
yungstersgrabbou
authored andcommittedMar 18, 2020
Pressability: Fix Missing onLongPress Gestures
Summary: The current implementation of `Pressability` has a bug related to `onLongPress`. When a user starts a press gesture, we keep track of the activation position (occurs after waiting `delayPressIn` milliseconds). If the touch moves away from that position by more than 10dp, we rule out the long press gesture. This means no matter how long you hold down the press, even if you move it back to within 10dp, we will not fire `onLongPress`. However, there is currently a bug where we never reset the cached activation position. This means that after the first press gesture, all subsequent long press gestures must start within 10dp of that first press gesture. This leads to seemingly intermittent missing long press gestures. This fixes the bug by ensuring that whenever a press gestures is terminated (either via a cancel or release), we reset the activation position. Changelog: [General][Fixed] - Fixed Pressability to properly fire `onLongPress`. Reviewed By: TheSavior Differential Revision: D20410075 fbshipit-source-id: e4727b7a9585ce3ea39481fc13e56b6b91740c8c
1 parent f6a8452 commit 1f8b698

File tree

2 files changed

+91
-2
lines changed

2 files changed

+91
-2
lines changed
 

‎Libraries/Pressability/Pressability.js

+1
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ export default class Pressability {
602602
event: PressEvent,
603603
): void {
604604
if (isTerminalSignal(signal)) {
605+
this._touchActivatePosition = null;
605606
this._cancelLongPressDelayTimeout();
606607
}
607608

‎Libraries/Pressability/__tests__/Pressability-test.js

+90-2
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,97 @@ describe('Pressability', () => {
395395
jest.advanceTimersByTime(1);
396396
expect(config.onLongPress).toBeCalled();
397397
});
398-
});
399398

400-
// TODO: onLongPressShouldCancelPress tests
399+
it('is called if touch moves within 10dp', () => {
400+
mockUIManagerMeasure();
401+
const {config, handlers} = createMockPressability();
402+
403+
handlers.onStartShouldSetResponder();
404+
handlers.onResponderGrant(createMockPressEvent('onResponderGrant'));
405+
handlers.onResponderMove(
406+
createMockPressEvent({
407+
registrationName: 'onResponderMove',
408+
pageX: 0,
409+
pageY: 0,
410+
}),
411+
);
412+
413+
jest.advanceTimersByTime(130);
414+
handlers.onResponderMove(
415+
// NOTE: Delta from (0, 0) is ~9.9 < 10.
416+
createMockPressEvent({
417+
registrationName: 'onResponderMove',
418+
pageX: 7,
419+
pageY: 7,
420+
}),
421+
);
422+
423+
jest.advanceTimersByTime(370);
424+
expect(config.onLongPress).toBeCalled();
425+
});
426+
427+
it('is not called if touch moves beyond 10dp', () => {
428+
mockUIManagerMeasure();
429+
const {config, handlers} = createMockPressability();
430+
431+
handlers.onStartShouldSetResponder();
432+
handlers.onResponderGrant(createMockPressEvent('onResponderGrant'));
433+
handlers.onResponderMove(
434+
createMockPressEvent({
435+
registrationName: 'onResponderMove',
436+
pageX: 0,
437+
pageY: 0,
438+
}),
439+
);
440+
441+
jest.advanceTimersByTime(130);
442+
handlers.onResponderMove(
443+
createMockPressEvent({
444+
registrationName: 'onResponderMove',
445+
// NOTE: Delta from (0, 0) is ~10.6 > 10.
446+
pageX: 7,
447+
pageY: 8,
448+
}),
449+
);
450+
451+
jest.advanceTimersByTime(370);
452+
expect(config.onLongPress).not.toBeCalled();
453+
});
454+
455+
it('is called independent of preceding long touch gesture', () => {
456+
mockUIManagerMeasure();
457+
const {config, handlers} = createMockPressability();
458+
459+
handlers.onStartShouldSetResponder();
460+
handlers.onResponderGrant(createMockPressEvent('onResponderGrant'));
461+
handlers.onResponderMove(
462+
createMockPressEvent({
463+
registrationName: 'onResponderMove',
464+
pageX: 0,
465+
pageY: 0,
466+
}),
467+
);
468+
469+
jest.advanceTimersByTime(500);
470+
expect(config.onLongPress).toHaveBeenCalledTimes(1);
471+
handlers.onResponderRelease(createMockPressEvent('onResponderRelease'));
472+
473+
// Subsequent long touch gesture should not carry over previous state.
474+
handlers.onStartShouldSetResponder();
475+
handlers.onResponderGrant(createMockPressEvent('onResponderGrant'));
476+
handlers.onResponderMove(
477+
// NOTE: Delta from (0, 0) is ~10.6 > 10, but should not matter.
478+
createMockPressEvent({
479+
registrationName: 'onResponderMove',
480+
pageX: 7,
481+
pageY: 8,
482+
}),
483+
);
484+
485+
jest.advanceTimersByTime(500);
486+
expect(config.onLongPress).toHaveBeenCalledTimes(2);
487+
});
488+
});
401489

402490
describe('onPress', () => {
403491
it('is called even when `measure` does not finish', () => {

0 commit comments

Comments
 (0)
Please sign in to comment.