From 75fd2948111fd83f67162c2f6f00f8c0f0fa7d44 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 22 May 2019 18:34:58 +0100 Subject: [PATCH 1/2] [Flare] Account for fixed elements in getAbsoluteBoundingClientRect --- packages/react-events/src/Press.js | 30 ++++- .../src/__tests__/Press-test.internal.js | 124 +++++++++++++++++- 2 files changed, 147 insertions(+), 7 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index f059327f3e21..41270cafe92a 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -436,10 +436,32 @@ function getAbsoluteBoundingClientRect( let offsetY = 0; // Traverse through all offset nodes - while (node != null && node.nodeType !== Node.DOCUMENT_NODE) { - offsetX += (node: any).scrollLeft; - offsetY += (node: any).scrollTop; - node = node.parentNode; + while (node != null) { + const parent = node.parentNode; + const scrollTop = node.scrollTop; + const scrollLeft = node.scrollLeft; + const isParentDocumentNode = + parent !== null && parent.nodeType === Node.DOCUMENT_NODE; + + // Check if the current node is fixed position, by using + // offsetParent node for a fast-path. Then we need to + // check if it's a scrollable container by checking if + // either scrollLeft or scrollTop are not 0. If it is + // a scrollable container and the parent node is not + // the document, then we can stop traversing the tree. + if ( + !isParentDocumentNode && + node.offsetParent === null && + (scrollLeft !== 0 || scrollTop !== 0) + ) { + break; + } + offsetX += node.scrollLeft; + offsetY += node.scrollTop; + if (isParentDocumentNode) { + break; + } + node = parent; } return { left: left + offsetX, diff --git a/packages/react-events/src/__tests__/Press-test.internal.js b/packages/react-events/src/__tests__/Press-test.internal.js index c3d73bf78ff6..32bae5e13af6 100644 --- a/packages/react-events/src/__tests__/Press-test.internal.js +++ b/packages/react-events/src/__tests__/Press-test.internal.js @@ -1109,8 +1109,8 @@ describe('Event responder: Press', () => { ReactDOM.render(element, container); ref.current.getBoundingClientRect = getBoundingClientRectMock; - // Emulate the element being offset - document.body.scrollTop = 1000; + // Emulate the element being offset with scroll + document.firstElementChild.scrollTop = 1000; const updatedCoordinatesInside = { pageX: coordinatesInside.pageX, pageY: coordinatesInside.pageY + 1000, @@ -1125,7 +1125,125 @@ describe('Event responder: Press', () => { createEvent('pointerup', updatedCoordinatesInside), ); jest.runAllTimers(); - document.body.scrollTop = 0; + document.firstElementChild.scrollTop = 0; + + expect(events).toEqual([ + 'onPressStart', + 'onPressChange', + 'onPressMove', + 'onPressEnd', + 'onPressChange', + 'onPress', + ]); + }); + + it('"onPress" is called on release inside a fixed container', () => { + let events = []; + const ref = React.createRef(); + const fixedContainerRef = React.createRef(); + const createEventHandler = msg => () => { + events.push(msg); + }; + + const element = ( +
+ +
+ +
+ ); + + ReactDOM.render(element, container); + + const fixedDiv = fixedContainerRef.current; + Object.defineProperty(fixedDiv, 'offsetParent', { + value: null, + }); + + // The fixed container is not scrolled + fixedDiv.scrollTop = 0; + ref.current.getBoundingClientRect = getBoundingClientRectMock; + // Emulate the element being offset with scroll + document.firstElementChild.scrollTop = 1000; + const updatedCoordinatesInside = { + pageX: coordinatesInside.pageX, + pageY: coordinatesInside.pageY + 1000, + }; + ref.current.dispatchEvent( + createEvent('pointerdown', updatedCoordinatesInside), + ); + container.dispatchEvent( + createEvent('pointermove', updatedCoordinatesInside), + ); + container.dispatchEvent( + createEvent('pointerup', updatedCoordinatesInside), + ); + jest.runAllTimers(); + document.firstElementChild.scrollTop = 0; + + expect(events).toEqual([ + 'onPressStart', + 'onPressChange', + 'onPressMove', + 'onPressEnd', + 'onPressChange', + 'onPress', + ]); + }); + + it('"onPress" is called on release inside a fixed scrolled container', () => { + let events = []; + const ref = React.createRef(); + const fixedContainerRef = React.createRef(); + const createEventHandler = msg => () => { + events.push(msg); + }; + + const element = ( +
+ +
+ +
+ ); + + ReactDOM.render(element, container); + + const fixedDiv = fixedContainerRef.current; + Object.defineProperty(fixedDiv, 'offsetParent', { + value: null, + }); + + // The fixed container is scrolled + fixedDiv.scrollTop = 100; + ref.current.getBoundingClientRect = getBoundingClientRectMock; + // Emulate the element being offset with scroll + document.firstElementChild.scrollTop = 1000; + const updatedCoordinatesInside = { + pageX: coordinatesInside.pageX, + pageY: coordinatesInside.pageY + 100, + }; + ref.current.dispatchEvent( + createEvent('pointerdown', updatedCoordinatesInside), + ); + container.dispatchEvent( + createEvent('pointermove', updatedCoordinatesInside), + ); + container.dispatchEvent( + createEvent('pointerup', updatedCoordinatesInside), + ); + jest.runAllTimers(); + document.firstElementChild.scrollTop = 0; expect(events).toEqual([ 'onPressStart', From 4f8cfc4094e82e020f7db2061b014e7db1630ac3 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 22 May 2019 18:58:19 +0100 Subject: [PATCH 2/2] inline scrollLeft and scrollTop --- packages/react-events/src/Press.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 41270cafe92a..8a2d3cc930e7 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -456,8 +456,8 @@ function getAbsoluteBoundingClientRect( ) { break; } - offsetX += node.scrollLeft; - offsetY += node.scrollTop; + offsetX += scrollLeft; + offsetY += scrollTop; if (isParentDocumentNode) { break; }