React events: fix press end event dispatching (#15500)
This patch fixes an issue related to determining whether the end event occurs within the responder region. Previously we only checked if the event target was within the responder region for moves, otherwise we checked if the target was within the event component. Since the dimensions of the child element can change after activation, we need to recalculate the responder region before deactivation as well if the target is not within the event component.
Nicolas Gallagher committed
Apr 25, 2019 at 13:00 UTC
0b3431170565a8d346912380f7476df60714ab99
3 files changed
+262
-53
packages/react-events/src/Press.js
+77
-20
@@ -52,10 +52,16 @@ type PressState = {
52
isPressWithinResponderRegion: boolean,
53
longPressTimeout: null | Symbol,
54
pointerType: PointerType,
55
- pressTarget: null | Element | Document,
55
+ pressTarget: null | Element,
56
pressEndTimeout: null | Symbol,
57
pressStartTimeout: null | Symbol,
58
- responderRegion: null | $ReadOnly<{|
58
+ responderRegionOnActivation: null | $ReadOnly<{|
59
+ bottom: number,
60
+ left: number,
61
+ right: number,
62
+ top: number,
63
+ |}>,
64
+ responderRegionOnDeactivation: null | $ReadOnly<{|
65
bottom: number,
66
left: number,
67
right: number,
@@ -312,7 +318,7 @@ function calculateDelayMS(delay: ?number, min = 0, fallback = 0) {
318
}
319
320
// TODO: account for touch hit slop
315
-function calculateResponderRegion(target, props) {
321
+function calculateResponderRegion(target: Element, props: PressProps) {
322
const pressRetentionOffset = {
323
...DEFAULT_PRESS_RETENTION_OFFSET,
324
...props.pressRetentionOffset,
@@ -352,15 +358,33 @@ function isPressWithinResponderRegion(
358
nativeEvent: $PropertyType<ReactResponderEvent, 'nativeEvent'>,
359
state: PressState,
360
): boolean {
355
- const {responderRegion} = state;
361
+ const {responderRegionOnActivation, responderRegionOnDeactivation} = state;
362
const event = (nativeEvent: any);
363
+ let left, top, right, bottom;
364
+
365
+ if (responderRegionOnActivation != null) {
366
+ left = responderRegionOnActivation.left;
367
+ top = responderRegionOnActivation.top;
368
+ right = responderRegionOnActivation.right;
369
+ bottom = responderRegionOnActivation.bottom;
370
+
371
+ if (responderRegionOnDeactivation != null) {
372
+ left = Math.min(left, responderRegionOnDeactivation.left);
373
+ top = Math.min(top, responderRegionOnDeactivation.top);
374
+ right = Math.max(right, responderRegionOnDeactivation.right);
375
+ bottom = Math.max(bottom, responderRegionOnDeactivation.bottom);
376
+ }
377
+ }
378
379
return (
359
- responderRegion != null &&
360
- (event.pageX >= responderRegion.left &&
361
- event.pageX <= responderRegion.right &&
362
- event.pageY >= responderRegion.top &&
363
- event.pageY <= responderRegion.bottom)
380
+ left != null &&
381
+ right != null &&
382
+ top != null &&
383
+ bottom != null &&
384
+ (event.pageX >= left &&
385
+ event.pageX <= right &&
386
+ event.pageY >= top &&
387
+ event.pageY <= bottom)
388
);
389
}
390
@@ -408,7 +432,8 @@ const PressResponder = {
432
pressEndTimeout: null,
433
pressStartTimeout: null,
434
pressTarget: null,
411
- responderRegion: null,
435
+ responderRegionOnActivation: null,
436
+ responderRegionOnDeactivation: null,
437
ignoreEmulatedMouseEvents: false,
438
};
439
},
@@ -469,7 +494,11 @@ const PressResponder = {
494
}
495
496
state.pointerType = pointerType;
472
- state.pressTarget = target;
497
+ state.pressTarget = getEventCurrentTarget(event, context);
498
+ state.responderRegionOnActivation = calculateResponderRegion(
499
+ state.pressTarget,
500
+ props,
501
+ );
502
state.isPressWithinResponderRegion = true;
503
dispatchPressStartEvents(context, props, state);
504
context.addRootEventTypes(rootEventTypes);
@@ -519,26 +548,34 @@ const PressResponder = {
548
case 'touchmove': {
549
if (state.isPressed) {
550
// Ignore emulated events (pointermove will dispatch touch and mouse events)
522
- // Ignore pointermove events during a keyboard press
551
+ // Ignore pointermove events during a keyboard press.
552
if (state.pointerType !== pointerType) {
553
return;
554
}
555
527
- if (state.responderRegion == null) {
528
- state.responderRegion = calculateResponderRegion(
529
- getEventCurrentTarget(event, context),
556
+ // Calculate the responder region we use for deactivation, as the
557
+ // element dimensions may have changed since activation.
558
+ if (
559
+ state.pressTarget !== null &&
560
+ state.responderRegionOnDeactivation == null
561
+ ) {
562
+ state.responderRegionOnDeactivation = calculateResponderRegion(
563
+ state.pressTarget,
564
props,
565
);
566
}
533
- if (isPressWithinResponderRegion(nativeEvent, state)) {
534
- state.isPressWithinResponderRegion = true;
567
+ state.isPressWithinResponderRegion = isPressWithinResponderRegion(
568
+ nativeEvent,
569
+ state,
570
+ );
571
+
572
+ if (state.isPressWithinResponderRegion) {
573
if (props.onPressMove) {
574
dispatchEvent(context, state, 'pressmove', props.onPressMove, {
575
discrete: false,
576
});
577
}
578
} else {
541
- state.isPressWithinResponderRegion = false;
579
dispatchPressEndEvents(context, props, state);
580
}
581
}
@@ -551,18 +588,38 @@ const PressResponder = {
588
case 'mouseup':
589
case 'touchend': {
590
if (state.isPressed) {
554
- // Ignore unrelated keyboard events
591
+ // Ignore unrelated keyboard events and verify press is within
592
+ // responder region for non-keyboard events.
593
if (pointerType === 'keyboard') {
594
if (!isValidKeyPress(nativeEvent.key)) {
595
return;
596
}
597
+ // If the event target isn't within the press target, check if we're still
598
+ // within the responder region. The region may have changed if the
599
+ // element's layout was modified after activation.
600
+ } else if (
601
+ state.pressTarget != null &&
602
+ !context.isTargetWithinElement(target, state.pressTarget)
603
+ ) {
604
+ // Calculate the responder region we use for deactivation if not
605
+ // already done during move event.
606
+ if (state.responderRegionOnDeactivation == null) {
607
+ state.responderRegionOnDeactivation = calculateResponderRegion(
608
+ state.pressTarget,
609
+ props,
610
+ );
611
+ }
612
+ state.isPressWithinResponderRegion = isPressWithinResponderRegion(
613
+ nativeEvent,
614
+ state,
615
+ );
616
}
617
618
const wasLongPressed = state.isLongPressed;
619
dispatchPressEndEvents(context, props, state);
620
621
if (state.pressTarget !== null && props.onPress) {
565
- if (context.isTargetWithinElement(target, state.pressTarget)) {
622
+ if (state.isPressWithinResponderRegion) {
623
if (
624
!(
625
wasLongPressed &&
packages/react-events/src/__tests__/Press-test.internal.js
+184
-32
@@ -189,10 +189,22 @@ describe('Event responder: Press', () => {
189
);
190
ReactDOM.render(element, container);
191
192
+ ref.current.getBoundingClientRect = () => ({
193
+ top: 50,
194
+ left: 50,
195
+ bottom: 500,
196
+ right: 500,
197
+ });
198
+
199
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
200
jest.advanceTimersByTime(499);
201
expect(onPressStart).toHaveBeenCalledTimes(0);
195
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
202
+ ref.current.dispatchEvent(
203
+ createPointerEvent('pointerup', {
204
+ pageX: 55,
205
+ pageY: 55,
206
+ }),
207
+ );
208
expect(onPressStart).toHaveBeenCalledTimes(1);
209
jest.runAllTimers();
210
expect(onPressStart).toHaveBeenCalledTimes(1);
@@ -420,9 +432,21 @@ describe('Event responder: Press', () => {
432
);
433
ReactDOM.render(element, container);
434
435
+ ref.current.getBoundingClientRect = () => ({
436
+ top: 50,
437
+ left: 50,
438
+ bottom: 500,
439
+ right: 500,
440
+ });
441
+
442
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
443
jest.advanceTimersByTime(100);
425
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
444
+ ref.current.dispatchEvent(
445
+ createPointerEvent('pointerup', {
446
+ pageX: 55,
447
+ pageY: 55,
448
+ }),
449
+ );
450
jest.advanceTimersByTime(10);
451
expect(onPressChange).toHaveBeenCalledWith(true);
452
expect(onPressChange).toHaveBeenCalledWith(false);
@@ -479,13 +503,21 @@ describe('Event responder: Press', () => {
503
</Press>
504
);
505
ReactDOM.render(element, container);
506
+ ref.current.getBoundingClientRect = () => ({
507
+ top: 0,
508
+ left: 0,
509
+ bottom: 100,
510
+ right: 100,
511
+ });
512
});
513
514
it('is called after "pointerup" event', () => {
515
ref.current.dispatchEvent(
516
createPointerEvent('pointerdown', {pointerType: 'pen'}),
517
);
488
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
518
+ ref.current.dispatchEvent(
519
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
520
+ );
521
expect(onPress).toHaveBeenCalledTimes(1);
522
expect(onPress).toHaveBeenCalledWith(
523
expect.objectContaining({pointerType: 'pen', type: 'press'}),
@@ -510,7 +542,9 @@ describe('Event responder: Press', () => {
542
ReactDOM.render(element, container);
543
544
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
513
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
545
+ ref.current.dispatchEvent(
546
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
547
+ );
548
expect(onPress).toHaveBeenCalledTimes(1);
549
});
550
@@ -709,10 +743,10 @@ describe('Event responder: Press', () => {
743
ReactDOM.render(element, container);
744
745
ref.current.getBoundingClientRect = () => ({
712
- top: 50,
713
- left: 50,
714
- bottom: 500,
715
- right: 500,
746
+ top: 0,
747
+ left: 0,
748
+ bottom: 100,
749
+ right: 100,
750
});
751
ref.current.dispatchEvent(
752
createPointerEvent('pointerdown', {pointerType: 'touch'}),
@@ -720,8 +754,8 @@ describe('Event responder: Press', () => {
754
ref.current.dispatchEvent(
755
createPointerEvent('pointermove', {
756
pointerType: 'touch',
723
- pageX: 55,
724
- pageY: 55,
757
+ pageX: 10,
758
+ pageY: 10,
759
}),
760
);
761
expect(onPressMove).toHaveBeenCalledTimes(1);
@@ -741,17 +775,17 @@ describe('Event responder: Press', () => {
775
ReactDOM.render(element, container);
776
777
ref.current.getBoundingClientRect = () => ({
744
- top: 50,
745
- left: 50,
746
- bottom: 500,
747
- right: 500,
778
+ top: 0,
779
+ left: 0,
780
+ bottom: 100,
781
+ right: 100,
782
});
783
ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'}));
784
ref.current.dispatchEvent(
785
createPointerEvent('pointermove', {
786
pointerType: 'mouse',
753
- pageX: 55,
754
- pageY: 55,
787
+ pageX: 10,
788
+ pageY: 10,
789
}),
790
);
791
expect(onPressMove).not.toBeCalled();
@@ -768,10 +802,10 @@ describe('Event responder: Press', () => {
802
ReactDOM.render(element, container);
803
804
ref.current.getBoundingClientRect = () => ({
771
- top: 50,
772
- left: 50,
773
- bottom: 500,
774
- right: 500,
805
+ top: 0,
806
+ left: 0,
807
+ bottom: 100,
808
+ right: 100,
809
});
810
ref.current.dispatchEvent(
811
createPointerEvent('pointerdown', {pointerType: 'touch'}),
@@ -780,8 +814,8 @@ describe('Event responder: Press', () => {
814
ref.current.dispatchEvent(
815
createPointerEvent('pointermove', {
816
pointerType: 'touch',
783
- pageX: 55,
784
- pageY: 55,
817
+ pageX: 10,
818
+ pageY: 10,
819
}),
820
);
821
ref.current.dispatchEvent(createPointerEvent('touchmove'));
@@ -843,7 +877,9 @@ describe('Event responder: Press', () => {
877
ref.current.dispatchEvent(
878
createPointerEvent('pointermove', coordinatesInside),
879
);
846
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
880
+ ref.current.dispatchEvent(
881
+ createPointerEvent('pointerup', coordinatesInside),
882
+ );
883
jest.runAllTimers();
884
885
expect(events).toEqual([
@@ -890,7 +926,9 @@ describe('Event responder: Press', () => {
926
expect(events).toEqual(['onPressStart', 'onPressChange']);
927
events = [];
928
893
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
929
+ ref.current.dispatchEvent(
930
+ createPointerEvent('pointerup', coordinatesInside),
931
+ );
932
expect(events).toEqual(['onPressEnd', 'onPressChange', 'onPress']);
933
});
934
@@ -923,7 +961,9 @@ describe('Event responder: Press', () => {
961
pageY: rectMock.top - pressRetentionOffset.top,
962
}),
963
);
926
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
964
+ ref.current.dispatchEvent(
965
+ createPointerEvent('pointerup', coordinatesInside),
966
+ );
967
expect(events).toEqual([
968
'onPressStart',
969
'onPressChange',
@@ -933,6 +973,86 @@ describe('Event responder: Press', () => {
973
'onPress',
974
]);
975
});
976
+
977
+ it('responder region accounts for decrease in element dimensions', () => {
978
+ let events = [];
979
+ const ref = React.createRef();
980
+ const createEventHandler = msg => () => {
981
+ events.push(msg);
982
+ };
983
+
984
+ const element = (
985
+ <Press
986
+ onPress={createEventHandler('onPress')}
987
+ onPressStart={createEventHandler('onPressStart')}
988
+ onPressEnd={createEventHandler('onPressEnd')}>
989
+ <div ref={ref} />
990
+ </Press>
991
+ );
992
+
993
+ ReactDOM.render(element, container);
994
+ ref.current.getBoundingClientRect = getBoundingClientRectMock;
995
+ ref.current.dispatchEvent(createPointerEvent('pointerdown'));
996
+ // emulate smaller dimensions change on activation
997
+ ref.current.getBoundingClientRect = () => ({
998
+ width: 80,
999
+ height: 80,
1000
+ top: 60,
1001
+ left: 60,
1002
+ right: 490,
1003
+ bottom: 490,
1004
+ });
1005
+ const coordinates = {
1006
+ pageX: rectMock.left,
1007
+ pageY: rectMock.top,
1008
+ };
1009
+ // move to an area within the pre-activation region
1010
+ ref.current.dispatchEvent(
1011
+ createPointerEvent('pointermove', coordinates),
1012
+ );
1013
+ ref.current.dispatchEvent(createPointerEvent('pointerup', coordinates));
1014
+ expect(events).toEqual(['onPressStart', 'onPressEnd', 'onPress']);
1015
+ });
1016
+
1017
+ it('responder region accounts for increase in element dimensions', () => {
1018
+ let events = [];
1019
+ const ref = React.createRef();
1020
+ const createEventHandler = msg => () => {
1021
+ events.push(msg);
1022
+ };
1023
+
1024
+ const element = (
1025
+ <Press
1026
+ onPress={createEventHandler('onPress')}
1027
+ onPressStart={createEventHandler('onPressStart')}
1028
+ onPressEnd={createEventHandler('onPressEnd')}>
1029
+ <div ref={ref} />
1030
+ </Press>
1031
+ );
1032
+
1033
+ ReactDOM.render(element, container);
1034
+ ref.current.getBoundingClientRect = getBoundingClientRectMock;
1035
+ ref.current.dispatchEvent(createPointerEvent('pointerdown'));
1036
+ // emulate larger dimensions change on activation
1037
+ ref.current.getBoundingClientRect = () => ({
1038
+ width: 200,
1039
+ height: 200,
1040
+ top: 0,
1041
+ left: 0,
1042
+ right: 550,
1043
+ bottom: 550,
1044
+ });
1045
+ const coordinates = {
1046
+ pageX: rectMock.left - 50,
1047
+ pageY: rectMock.top - 50,
1048
+ };
1049
+ // move to an area within the post-activation region
1050
+ ref.current.dispatchEvent(
1051
+ createPointerEvent('pointermove', coordinates),
1052
+ );
1053
+ ref.current.dispatchEvent(createPointerEvent('pointerup', coordinates));
1054
+ expect(events).toEqual(['onPressStart', 'onPressEnd', 'onPress']);
1055
+ });
1056
});
1057
1058
describe('beyond bounds of hit rect', () => {
@@ -973,7 +1093,9 @@ describe('Event responder: Press', () => {
1093
ref.current.dispatchEvent(
1094
createPointerEvent('pointermove', coordinatesOutside),
1095
);
976
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1096
+ ref.current.dispatchEvent(
1097
+ createPointerEvent('pointerup', coordinatesOutside),
1098
+ );
1099
jest.runAllTimers();
1100
1101
expect(events).toEqual([
@@ -1019,7 +1141,9 @@ describe('Event responder: Press', () => {
1141
jest.runAllTimers();
1142
expect(events).toEqual(['onPressMove']);
1143
events = [];
1022
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1144
+ ref.current.dispatchEvent(
1145
+ createPointerEvent('pointerup', coordinatesOutside),
1146
+ );
1147
jest.runAllTimers();
1148
expect(events).toEqual([]);
1149
});
@@ -1049,13 +1173,23 @@ describe('Event responder: Press', () => {
1173
);
1174
1175
ReactDOM.render(element, container);
1176
+ ref.current.getBoundingClientRect = () => ({
1177
+ top: 0,
1178
+ left: 0,
1179
+ bottom: 0,
1180
+ right: 0,
1181
+ });
1182
1183
// 1
1184
events = [];
1185
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
1056
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1186
+ ref.current.dispatchEvent(
1187
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
1188
+ );
1189
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
1058
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1190
+ ref.current.dispatchEvent(
1191
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
1192
+ );
1193
jest.runAllTimers();
1194
1195
expect(events).toEqual([
@@ -1073,7 +1207,9 @@ describe('Event responder: Press', () => {
1207
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
1208
jest.advanceTimersByTime(250);
1209
jest.advanceTimersByTime(500);
1076
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1210
+ ref.current.dispatchEvent(
1211
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
1212
+ );
1213
jest.runAllTimers();
1214
1215
expect(events).toEqual([
@@ -1121,9 +1257,17 @@ describe('Event responder: Press', () => {
1257
);
1258
1259
ReactDOM.render(element, container);
1260
+ ref.current.getBoundingClientRect = () => ({
1261
+ top: 0,
1262
+ left: 0,
1263
+ bottom: 0,
1264
+ right: 0,
1265
+ });
1266
1267
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
1126
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1268
+ ref.current.dispatchEvent(
1269
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
1270
+ );
1271
expect(events).toEqual([
1272
'pointerdown',
1273
'inner: onPressStart',
@@ -1147,9 +1291,17 @@ describe('Event responder: Press', () => {
1291
</Press>
1292
);
1293
ReactDOM.render(element, container);
1294
+ ref.current.getBoundingClientRect = () => ({
1295
+ top: 0,
1296
+ left: 0,
1297
+ bottom: 0,
1298
+ right: 0,
1299
+ });
1300
1301
ref.current.dispatchEvent(createPointerEvent('pointerdown'));
1152
- ref.current.dispatchEvent(createPointerEvent('pointerup'));
1302
+ ref.current.dispatchEvent(
1303
+ createPointerEvent('pointerup', {pageX: 10, pageY: 10}),
1304
+ );
1305
expect(fn).toHaveBeenCalledTimes(1);
1306
});
1307
packages/react-events/src/utils.js
+1
-1
@@ -15,7 +15,7 @@ import type {
15
export function getEventCurrentTarget(
16
event: ReactResponderEvent,
17
context: ReactResponderContext,
18
-) {
18
+): Element {
19
const target: any = event.target;
20
let currentTarget = target;
21
while (