[Flare] Fix isPressWithinResponderRegion logic (#15808)
Compare the viewport-relative coordinates of getBoundingClientRect with those of the event's client{X,Y} values. This fixes press within scrollable nodes.
Nicolas Gallagher committed
Jun 3, 2019 at 14:59 UTC
dd43cb5fb941cb253a9f0197a1f3957aebdcf3d8
2 files changed
+59
-288
packages/react-events/src/Press.js
+13
-79
@@ -42,8 +42,8 @@ type PointerType = '' | 'mouse' | 'keyboard' | 'pen' | 'touch';
42
43
type PressState = {
44
activationPosition: null | $ReadOnly<{|
45
- pageX: number,
46
- pageY: number,
45
+ x: number,
46
+ y: number,
47
|}>,
48
addedRootEvents: boolean,
49
isActivePressed: boolean,
@@ -247,14 +247,11 @@ function dispatchLongPressChangeEvent(
247
248
function activate(event: ReactResponderEvent, context, props, state) {
249
const nativeEvent: any = event.nativeEvent;
250
- const {x, y} = getEventPageCoords(nativeEvent);
250
+ const {x, y} = getEventViewportCoords(nativeEvent);
251
const wasActivePressed = state.isActivePressed;
252
state.isActivePressed = true;
253
if (x !== null && y !== null) {
254
- state.activationPosition = {
255
- pageX: x,
256
- pageY: y,
257
- };
254
+ state.activationPosition = {x, y};
255
}
256
257
if (props.onPressStart) {
@@ -432,66 +429,6 @@ function calculateDelayMS(delay: ?number, min = 0, fallback = 0) {
429
return Math.max(min, maybeNumber != null ? maybeNumber : fallback);
430
}
431
435
-function isNodeFixedPositioned(node: Node | null | void): boolean {
436
- return (
437
- node != null &&
438
- (node: any).offsetParent === null &&
439
- !isNodeDocumentNode(node.parentNode)
440
- );
441
-}
442
-
443
-function isNodeDocumentNode(node: Node | null | void): boolean {
444
- return node != null && node.nodeType === Node.DOCUMENT_NODE;
445
-}
446
-
447
-function getAbsoluteBoundingClientRect(
448
- target: Element,
449
-): {left: number, right: number, bottom: number, top: number} {
450
- const clientRect = target.getBoundingClientRect();
451
- let {left, right, bottom, top} = clientRect;
452
- let node = target.parentNode;
453
- let offsetX = 0;
454
- let offsetY = 0;
455
-
456
- // Traverse through all offset nodes
457
- while (node != null) {
458
- const parent = node.parentNode;
459
- const scrollTop = (node: any).scrollTop;
460
- const scrollLeft = (node: any).scrollLeft;
461
-
462
- // We first need to check if it's a scrollable container by
463
- // checking if either scrollLeft or scrollTop are not 0.
464
- // Then we check if either the current node or its parent
465
- // are fixed position, using offsetParent node for a fast-path.
466
- // We need to check both as offsetParent accounts for both
467
- // itself and the parent; so we need to align with that API.
468
- // If these all pass, we can skip traversing the relevant
469
- // node and go directly to its parent.
470
- if (scrollLeft !== 0 || scrollTop !== 0) {
471
- if (isNodeFixedPositioned(parent)) {
472
- node = ((parent: any): Node).parentNode;
473
- continue;
474
- }
475
- if (isNodeFixedPositioned(node)) {
476
- node = parent;
477
- continue;
478
- }
479
- }
480
- offsetX += scrollLeft;
481
- offsetY += scrollTop;
482
- if (isNodeDocumentNode(parent)) {
483
- break;
484
- }
485
- node = parent;
486
- }
487
- return {
488
- left: left + offsetX,
489
- right: right + offsetX,
490
- bottom: bottom + offsetY,
491
- top: top + offsetY,
492
- };
493
-}
494
-
432
// TODO: account for touch hit slop
433
function calculateResponderRegion(
434
context: ReactResponderContext,
@@ -504,8 +441,7 @@ function calculateResponderRegion(
441
props.pressRetentionOffset,
442
);
443
507
- const clientRect = getAbsoluteBoundingClientRect(target);
508
- let {left, right, bottom, top} = clientRect;
444
+ let {left, right, bottom, top} = target.getBoundingClientRect();
445
446
if (pressRetentionOffset) {
447
if (pressRetentionOffset.bottom != null) {
@@ -543,18 +479,18 @@ function getTouchFromPressEvent(nativeEvent: TouchEvent): Touch {
479
: (nativeEvent: any);
480
}
481
546
-function getEventPageCoords(
482
+function getEventViewportCoords(
483
nativeEvent: Event,
484
): {x: null | number, y: null | number} {
485
let eventObject = (nativeEvent: any);
486
if (isTouchEvent(eventObject)) {
487
eventObject = getTouchFromPressEvent(eventObject);
488
}
553
- const pageX = eventObject.pageX;
554
- const pageY = eventObject.pageY;
489
+ const x = eventObject.clientX;
490
+ const y = eventObject.clientY;
491
return {
556
- x: pageX != null ? pageX : null,
557
- y: pageY != null ? pageY : null,
492
+ x: x != null ? x : null,
493
+ y: y != null ? y : null,
494
};
495
}
496
@@ -578,7 +514,7 @@ function isPressWithinResponderRegion(
514
bottom = Math.max(bottom, responderRegionOnDeactivation.bottom);
515
}
516
}
581
- const {x, y} = getEventPageCoords(((nativeEvent: any): Event));
517
+ const {x, y} = getEventViewportCoords(((nativeEvent: any): Event));
518
519
return (
520
left != null &&
@@ -833,10 +769,8 @@ const PressResponder = {
769
state.activationPosition != null &&
770
state.longPressTimeout != null
771
) {
836
- const deltaX =
837
- state.activationPosition.pageX - nativeEvent.pageX;
838
- const deltaY =
839
- state.activationPosition.pageY - nativeEvent.pageY;
772
+ const deltaX = state.activationPosition.x - nativeEvent.clientX;
773
+ const deltaY = state.activationPosition.y - nativeEvent.clientY;
774
if (
775
Math.hypot(deltaX, deltaY) > 10 &&
776
state.longPressTimeout != null
packages/react-events/src/__tests__/Press-test.internal.js
+46
-209
@@ -206,8 +206,8 @@ describe('Event responder: Press', () => {
206
expect(onPressStart).toHaveBeenCalledTimes(0);
207
ref.current.dispatchEvent(
208
createEvent('pointerup', {
209
- pageX: 55,
210
- pageY: 55,
209
+ clientX: 55,
210
+ clientY: 55,
211
}),
212
);
213
expect(onPressStart).toHaveBeenCalledTimes(1);
@@ -471,8 +471,8 @@ describe('Event responder: Press', () => {
471
jest.advanceTimersByTime(100);
472
ref.current.dispatchEvent(
473
createEvent('pointerup', {
474
- pageX: 55,
475
- pageY: 55,
474
+ clientX: 55,
475
+ clientY: 55,
476
}),
477
);
478
jest.advanceTimersByTime(10);
@@ -544,7 +544,7 @@ describe('Event responder: Press', () => {
544
createEvent('pointerdown', {pointerType: 'pen'}),
545
);
546
ref.current.dispatchEvent(
547
- createEvent('pointerup', {pageX: 10, pageY: 10}),
547
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
548
);
549
expect(onPress).toHaveBeenCalledTimes(1);
550
expect(onPress).toHaveBeenCalledWith(
@@ -592,7 +592,7 @@ describe('Event responder: Press', () => {
592
593
ref.current.dispatchEvent(createEvent('pointerdown'));
594
ref.current.dispatchEvent(
595
- createEvent('pointerup', {pageX: 10, pageY: 10}),
595
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
596
);
597
expect(onPress).toHaveBeenCalledTimes(1);
598
});
@@ -668,10 +668,10 @@ describe('Event responder: Press', () => {
668
right: 100,
669
});
670
ref.current.dispatchEvent(
671
- createEvent('pointerdown', {pageX: 10, pageY: 10}),
671
+ createEvent('pointerdown', {clientX: 10, clientY: 10}),
672
);
673
ref.current.dispatchEvent(
674
- createEvent('pointermove', {pageX: 50, pageY: 50}),
674
+ createEvent('pointermove', {clientX: 50, clientY: 50}),
675
);
676
jest.runAllTimers();
677
expect(onLongPress).not.toBeCalled();
@@ -820,8 +820,8 @@ describe('Event responder: Press', () => {
820
ref.current.dispatchEvent(
821
createEvent('pointermove', {
822
pointerType: 'touch',
823
- pageX: 10,
824
- pageY: 10,
823
+ clientX: 10,
824
+ clientY: 10,
825
}),
826
);
827
expect(onPressMove).toHaveBeenCalledTimes(1);
@@ -850,8 +850,8 @@ describe('Event responder: Press', () => {
850
ref.current.dispatchEvent(
851
createEvent('pointermove', {
852
pointerType: 'mouse',
853
- pageX: 10,
854
- pageY: 10,
853
+ clientX: 10,
854
+ clientY: 10,
855
}),
856
);
857
expect(onPressMove).not.toBeCalled();
@@ -880,8 +880,8 @@ describe('Event responder: Press', () => {
880
ref.current.dispatchEvent(
881
createEvent('pointermove', {
882
pointerType: 'touch',
883
- pageX: 10,
884
- pageY: 10,
883
+ clientX: 10,
884
+ clientY: 10,
885
}),
886
);
887
ref.current.dispatchEvent(createEvent('touchmove'));
@@ -902,12 +902,12 @@ describe('Event responder: Press', () => {
902
const pressRectOffset = 20;
903
const getBoundingClientRectMock = () => rectMock;
904
const coordinatesInside = {
905
- pageX: rectMock.left - pressRectOffset,
906
- pageY: rectMock.top - pressRectOffset,
905
+ clientX: rectMock.left - pressRectOffset,
906
+ clientY: rectMock.top - pressRectOffset,
907
};
908
const coordinatesOutside = {
909
- pageX: rectMock.left - pressRectOffset - 1,
910
- pageY: rectMock.top - pressRectOffset - 1,
909
+ clientX: rectMock.left - pressRectOffset - 1,
910
+ clientY: rectMock.top - pressRectOffset - 1,
911
};
912
913
describe('within bounds of hit rect', () => {
@@ -1019,8 +1019,8 @@ describe('Event responder: Press', () => {
1019
ref.current.dispatchEvent(createEvent('pointerdown'));
1020
ref.current.dispatchEvent(
1021
createEvent('pointermove', {
1022
- pageX: rectMock.left - pressRetentionOffset.left,
1023
- pageY: rectMock.top - pressRetentionOffset.top,
1022
+ clientX: rectMock.left - pressRetentionOffset.left,
1023
+ clientY: rectMock.top - pressRetentionOffset.top,
1024
}),
1025
);
1026
ref.current.dispatchEvent(createEvent('pointerup', coordinatesInside));
@@ -1063,8 +1063,8 @@ describe('Event responder: Press', () => {
1063
bottom: 490,
1064
});
1065
const coordinates = {
1066
- pageX: rectMock.left,
1067
- pageY: rectMock.top,
1066
+ clientX: rectMock.left,
1067
+ clientY: rectMock.top,
1068
};
1069
// move to an area within the pre-activation region
1070
ref.current.dispatchEvent(createEvent('pointermove', coordinates));
@@ -1101,8 +1101,8 @@ describe('Event responder: Press', () => {
1101
bottom: 550,
1102
});
1103
const coordinates = {
1104
- pageX: rectMock.left - 50,
1105
- pageY: rectMock.top - 50,
1104
+ clientX: rectMock.left - 50,
1105
+ clientY: rectMock.top - 50,
1106
};
1107
// move to an area within the post-activation region
1108
ref.current.dispatchEvent(createEvent('pointermove', coordinates));
@@ -1111,176 +1111,6 @@ describe('Event responder: Press', () => {
1111
});
1112
});
1113
1114
- describe('the page offset changes', () => {
1115
- it('"onPress" is called on release', () => {
1116
- let events = [];
1117
- const ref = React.createRef();
1118
- const createEventHandler = msg => () => {
1119
- events.push(msg);
1120
- };
1121
-
1122
- const element = (
1123
- <Press
1124
- onPress={createEventHandler('onPress')}
1125
- onPressChange={createEventHandler('onPressChange')}
1126
- onPressMove={createEventHandler('onPressMove')}
1127
- onPressStart={createEventHandler('onPressStart')}
1128
- onPressEnd={createEventHandler('onPressEnd')}>
1129
- <div ref={ref} />
1130
- </Press>
1131
- );
1132
-
1133
- ReactDOM.render(element, container);
1134
-
1135
- ref.current.getBoundingClientRect = getBoundingClientRectMock;
1136
- // Emulate the <html> element being offset with scroll
1137
- document.firstElementChild.scrollTop = 1000;
1138
- const updatedCoordinatesInside = {
1139
- pageX: coordinatesInside.pageX,
1140
- pageY: coordinatesInside.pageY + 1000,
1141
- };
1142
- ref.current.dispatchEvent(
1143
- createEvent('pointerdown', updatedCoordinatesInside),
1144
- );
1145
- container.dispatchEvent(
1146
- createEvent('pointermove', updatedCoordinatesInside),
1147
- );
1148
- container.dispatchEvent(
1149
- createEvent('pointerup', updatedCoordinatesInside),
1150
- );
1151
- jest.runAllTimers();
1152
- document.firstElementChild.scrollTop = 0;
1153
-
1154
- expect(events).toEqual([
1155
- 'onPressStart',
1156
- 'onPressChange',
1157
- 'onPressMove',
1158
- 'onPressEnd',
1159
- 'onPressChange',
1160
- 'onPress',
1161
- ]);
1162
- });
1163
-
1164
- it('"onPress" is called on release inside a fixed container', () => {
1165
- let events = [];
1166
- const ref = React.createRef();
1167
- const fixedContainerRef = React.createRef();
1168
- const createEventHandler = msg => () => {
1169
- events.push(msg);
1170
- };
1171
-
1172
- const element = (
1173
- <div ref={fixedContainerRef}>
1174
- <Press
1175
- onPress={createEventHandler('onPress')}
1176
- onPressChange={createEventHandler('onPressChange')}
1177
- onPressMove={createEventHandler('onPressMove')}
1178
- onPressStart={createEventHandler('onPressStart')}
1179
- onPressEnd={createEventHandler('onPressEnd')}>
1180
- <div ref={ref} />
1181
- </Press>
1182
- </div>
1183
- );
1184
-
1185
- ReactDOM.render(element, container);
1186
-
1187
- const fixedDiv = fixedContainerRef.current;
1188
- Object.defineProperty(fixedDiv, 'offsetParent', {
1189
- value: null,
1190
- });
1191
-
1192
- // The fixed container is not scrolled
1193
- fixedDiv.scrollTop = 0;
1194
- fixedDiv.scrollLeft = 0;
1195
- ref.current.getBoundingClientRect = getBoundingClientRectMock;
1196
- // Emulate the <html> element being offset with scroll
1197
- document.firstElementChild.scrollTop = 1000;
1198
- const updatedCoordinatesInside = {
1199
- pageX: coordinatesInside.pageX,
1200
- pageY: coordinatesInside.pageY + 1000,
1201
- };
1202
- ref.current.dispatchEvent(
1203
- createEvent('pointerdown', updatedCoordinatesInside),
1204
- );
1205
- container.dispatchEvent(
1206
- createEvent('pointermove', updatedCoordinatesInside),
1207
- );
1208
- container.dispatchEvent(
1209
- createEvent('pointerup', updatedCoordinatesInside),
1210
- );
1211
- jest.runAllTimers();
1212
- document.firstElementChild.scrollTop = 0;
1213
-
1214
- expect(events).toEqual([
1215
- 'onPressStart',
1216
- 'onPressChange',
1217
- 'onPressMove',
1218
- 'onPressEnd',
1219
- 'onPressChange',
1220
- 'onPress',
1221
- ]);
1222
- });
1223
-
1224
- it('"onPress" is called on release inside a fixed scrolled container', () => {
1225
- let events = [];
1226
- const ref = React.createRef();
1227
- const fixedContainerRef = React.createRef();
1228
- const createEventHandler = msg => () => {
1229
- events.push(msg);
1230
- };
1231
-
1232
- const element = (
1233
- <div ref={fixedContainerRef}>
1234
- <Press
1235
- onPress={createEventHandler('onPress')}
1236
- onPressChange={createEventHandler('onPressChange')}
1237
- onPressMove={createEventHandler('onPressMove')}
1238
- onPressStart={createEventHandler('onPressStart')}
1239
- onPressEnd={createEventHandler('onPressEnd')}>
1240
- <div ref={ref} />
1241
- </Press>
1242
- </div>
1243
- );
1244
-
1245
- ReactDOM.render(element, container);
1246
-
1247
- const fixedDiv = fixedContainerRef.current;
1248
- Object.defineProperty(fixedDiv, 'offsetParent', {
1249
- value: null,
1250
- });
1251
-
1252
- // The fixed container is scrolled
1253
- fixedDiv.scrollTop = 100;
1254
- ref.current.getBoundingClientRect = getBoundingClientRectMock;
1255
- // Emulate the <html> element being offset with scroll
1256
- document.firstElementChild.scrollTop = 1000;
1257
- const updatedCoordinatesInside = {
1258
- pageX: coordinatesInside.pageX,
1259
- pageY: coordinatesInside.pageY + 1100,
1260
- };
1261
- ref.current.dispatchEvent(
1262
- createEvent('pointerdown', updatedCoordinatesInside),
1263
- );
1264
- container.dispatchEvent(
1265
- createEvent('pointermove', updatedCoordinatesInside),
1266
- );
1267
- container.dispatchEvent(
1268
- createEvent('pointerup', updatedCoordinatesInside),
1269
- );
1270
- jest.runAllTimers();
1271
- document.firstElementChild.scrollTop = 0;
1272
-
1273
- expect(events).toEqual([
1274
- 'onPressStart',
1275
- 'onPressChange',
1276
- 'onPressMove',
1277
- 'onPressEnd',
1278
- 'onPressChange',
1279
- 'onPress',
1280
- ]);
1281
- });
1282
- });
1283
-
1114
describe('beyond bounds of hit rect', () => {
1115
/** ┌──────────────────┐
1116
* │ ┌────────────┐ │
@@ -1569,16 +1399,16 @@ describe('Event responder: Press', () => {
1399
const coordinatesInside = {
1400
changedTouches: [
1401
{
1572
- pageX: rectMock.left - pressRectOffset,
1573
- pageY: rectMock.top - pressRectOffset,
1402
+ clientX: rectMock.left - pressRectOffset,
1403
+ clientY: rectMock.top - pressRectOffset,
1404
},
1405
],
1406
};
1407
const coordinatesOutside = {
1408
changedTouches: [
1409
{
1580
- pageX: rectMock.left - pressRectOffset - 1,
1581
- pageY: rectMock.top - pressRectOffset - 1,
1410
+ clientX: rectMock.left - pressRectOffset - 1,
1411
+ clientY: rectMock.top - pressRectOffset - 1,
1412
},
1413
],
1414
};
@@ -1688,8 +1518,8 @@ describe('Event responder: Press', () => {
1518
ref.current.dispatchEvent(createEvent('touchstart'));
1519
ref.current.dispatchEvent(
1520
createEvent('touchmove', {
1691
- pageX: rectMock.left - pressRetentionOffset.left,
1692
- pageY: rectMock.top - pressRetentionOffset.top,
1521
+ clientX: rectMock.left - pressRetentionOffset.left,
1522
+ clientY: rectMock.top - pressRetentionOffset.top,
1523
}),
1524
);
1525
ref.current.dispatchEvent(createEvent('touchend', coordinatesInside));
@@ -1732,8 +1562,8 @@ describe('Event responder: Press', () => {
1562
bottom: 490,
1563
});
1564
const coordinates = {
1735
- pageX: rectMock.left,
1736
- pageY: rectMock.top,
1565
+ clientX: rectMock.left,
1566
+ clientY: rectMock.top,
1567
};
1568
// move to an area within the pre-activation region
1569
ref.current.dispatchEvent(createEvent('touchmove', coordinates));
@@ -1770,8 +1600,8 @@ describe('Event responder: Press', () => {
1600
bottom: 550,
1601
});
1602
const coordinates = {
1773
- pageX: rectMock.left - 50,
1774
- pageY: rectMock.top - 50,
1603
+ clientX: rectMock.left - 50,
1604
+ clientY: rectMock.top - 50,
1605
};
1606
// move to an area within the post-activation region
1607
ref.current.dispatchEvent(createEvent('touchmove', coordinates));
@@ -1963,11 +1793,11 @@ describe('Event responder: Press', () => {
1793
events = [];
1794
ref.current.dispatchEvent(createEvent('pointerdown'));
1795
ref.current.dispatchEvent(
1966
- createEvent('pointerup', {pageX: 10, pageY: 10}),
1796
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
1797
);
1798
ref.current.dispatchEvent(createEvent('pointerdown'));
1799
ref.current.dispatchEvent(
1970
- createEvent('pointerup', {pageX: 10, pageY: 10}),
1800
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
1801
);
1802
jest.runAllTimers();
1803
@@ -1987,7 +1817,7 @@ describe('Event responder: Press', () => {
1817
jest.advanceTimersByTime(250);
1818
jest.advanceTimersByTime(500);
1819
ref.current.dispatchEvent(
1990
- createEvent('pointerup', {pageX: 10, pageY: 10}),
1820
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
1821
);
1822
jest.runAllTimers();
1823
@@ -2045,7 +1875,7 @@ describe('Event responder: Press', () => {
1875
1876
ref.current.dispatchEvent(createEvent('pointerdown'));
1877
ref.current.dispatchEvent(
2048
- createEvent('pointerup', {pageX: 10, pageY: 10}),
1878
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
1879
);
1880
expect(events).toEqual([
1881
'inner: onPressStart',
@@ -2079,7 +1909,7 @@ describe('Event responder: Press', () => {
1909
1910
ref.current.dispatchEvent(createEvent('pointerdown'));
1911
ref.current.dispatchEvent(
2082
- createEvent('pointerup', {pageX: 10, pageY: 10}),
1912
+ createEvent('pointerup', {clientX: 10, clientY: 10}),
1913
);
1914
expect(fn).toHaveBeenCalledTimes(1);
1915
});
@@ -2344,6 +2174,13 @@ describe('Event responder: Press', () => {
2174
);
2175
ReactDOM.render(element, container);
2176
2177
+ ref.current.getBoundingClientRect = () => ({
2178
+ top: 10,
2179
+ left: 10,
2180
+ bottom: 20,
2181
+ right: 20,
2182
+ });
2183
+
2184
ref.current.dispatchEvent(
2185
createEvent('pointerdown', {
2186
pointerType: 'mouse',