From f63f3d17785a2c6b9dd0b36a6d7752594cc13e24 Mon Sep 17 00:00:00 2001 From: "Aeneas Rekkas (arekkas)" Date: Sun, 17 Jul 2016 18:45:03 +0200 Subject: [PATCH 1/2] Resolves regression issue by introducing `offsetParent` to Allows nodes to use an arbitrary node as origin instead of the parent. This is useful, when the parent's position is changing. When used, resolves #170 This issue was introduced by a398097ebcc2cbb4df5582a8a3f42f51d21745a0 --- README.md | 1 + lib/DraggableCore.es6 | 11 +++++++++++ lib/utils/domFns.es6 | 7 ++++--- lib/utils/positionFns.es6 | 2 +- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 34a156bf..cf0f9a2d 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,7 @@ on itself and thus must have callbacks attached to be useful. cancel: string, disabled: boolean, enableUserSelectHack: boolean, + offsetParent: HTMLElement, grid: [number, number], handle: string, onStart: DraggableEventHandler, diff --git a/lib/DraggableCore.es6 b/lib/DraggableCore.es6 index 832c95f8..64e412fa 100644 --- a/lib/DraggableCore.es6 +++ b/lib/DraggableCore.es6 @@ -66,6 +66,16 @@ export default class DraggableCore extends React.Component { */ enableUserSelectHack: PropTypes.bool, + /** + * `offsetParent`, if set, uses the passed DOM node to compute drag offsets + * instead of using the parent node. + */ + offsetParent: function(props, propName) { + if (process.browser && props[propName] && props[propName].nodeType !== 1) { + throw new Error('Draggable\'s offsetParent must be a DOM Node.'); + } + }, + /** * `grid` specifies the x and y that dragging should snap to. */ @@ -152,6 +162,7 @@ export default class DraggableCore extends React.Component { cancel: null, disabled: false, enableUserSelectHack: true, + offsetParent: null, handle: null, grid: null, transform: null, diff --git a/lib/utils/domFns.es6 b/lib/utils/domFns.es6 index 9afa67a5..b06b6be7 100644 --- a/lib/utils/domFns.es6 +++ b/lib/utils/domFns.es6 @@ -2,6 +2,7 @@ import {findInArray, isFunction, int} from './shims'; import browserPrefix, {getPrefix, browserPrefixToStyle, browserPrefixToKey} from './getPrefix'; +import type DraggableCore from '../DraggableCore'; import type {ControlPosition} from './types'; let matchesSelectorFunc = ''; @@ -95,9 +96,9 @@ export function innerWidth(node: HTMLElement): number { } // Get from offsetParent -export function offsetXYFromParentOf(evt: {clientX: number, clientY: number}, node: HTMLElement & {offsetParent: HTMLElement}): ControlPosition { - const offsetParent = node.offsetParent || document.body; - const offsetParentRect = node.offsetParent === document.body ? {left: 0, top: 0} : offsetParent.getBoundingClientRect(); +export function offsetXYFromParentOf(evt: {clientX: number, clientY: number}, node: HTMLElement & {offsetParent: HTMLElement}, draggableCore: DraggableCore): ControlPosition { + const offsetParent = draggableCore.props.offsetParent || node.offsetParent || document.body; + const offsetParentRect = offsetParent ? {left: 0, top: 0} : offsetParent.getBoundingClientRect(); const x = evt.clientX + offsetParent.scrollLeft - offsetParentRect.left; const y = evt.clientY + offsetParent.scrollTop - offsetParentRect.top; diff --git a/lib/utils/positionFns.es6 b/lib/utils/positionFns.es6 index b2df07b4..3b88b7c4 100644 --- a/lib/utils/positionFns.es6 +++ b/lib/utils/positionFns.es6 @@ -66,7 +66,7 @@ export function canDragY(draggable: Draggable): boolean { export function getControlPosition(e: MouseEvent, touchIdentifier: ?number, draggableCore: DraggableCore): ?ControlPosition { const touchObj = typeof touchIdentifier === 'number' ? getTouch(e, touchIdentifier) : null; if (typeof touchIdentifier === 'number' && !touchObj) return null; // not the right touch - return offsetXYFromParentOf(touchObj || e, ReactDOM.findDOMNode(draggableCore)); + return offsetXYFromParentOf(touchObj || e, ReactDOM.findDOMNode(draggableCore), draggableCore); } // Create an data object exposed by 's events From 6855c3828ad0bfe29db9d7c0ce3c4ea2b0b30a4a Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Tue, 19 Jul 2016 14:31:41 -0500 Subject: [PATCH 2/2] Refactor `offsetParent` prop & add test --- README.md | 5 +++ lib/utils/domFns.es6 | 8 ++--- lib/utils/positionFns.es6 | 8 +++-- specs/draggable.spec.jsx | 76 +++++++++++++++++++++++++++++++++------ 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index cf0f9a2d..3d9acedd 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,11 @@ grid: [number, number], // Example: '.handle' handle: string, +// If desired, you can provide your own offsetParent for drag calculations. +// By default, we use the Draggable's offsetParent. This can be useful for elements +// with odd display types or floats. +offsetParent: HTMLElement, + // Called whenever the user mouses down. Called regardless of handle or // disabled status. onMouseDown: (e: MouseEvent) => void, diff --git a/lib/utils/domFns.es6 b/lib/utils/domFns.es6 index b06b6be7..7ae4ad5a 100644 --- a/lib/utils/domFns.es6 +++ b/lib/utils/domFns.es6 @@ -2,7 +2,6 @@ import {findInArray, isFunction, int} from './shims'; import browserPrefix, {getPrefix, browserPrefixToStyle, browserPrefixToKey} from './getPrefix'; -import type DraggableCore from '../DraggableCore'; import type {ControlPosition} from './types'; let matchesSelectorFunc = ''; @@ -96,9 +95,10 @@ export function innerWidth(node: HTMLElement): number { } // Get from offsetParent -export function offsetXYFromParentOf(evt: {clientX: number, clientY: number}, node: HTMLElement & {offsetParent: HTMLElement}, draggableCore: DraggableCore): ControlPosition { - const offsetParent = draggableCore.props.offsetParent || node.offsetParent || document.body; - const offsetParentRect = offsetParent ? {left: 0, top: 0} : offsetParent.getBoundingClientRect(); +export function offsetXYFromParent(evt: {clientX: number, clientY: number}, offsetParent: ?HTMLElement): ControlPosition { + if (!offsetParent) offsetParent = document.body; + const isBody = offsetParent === offsetParent.ownerDocument.body; + const offsetParentRect = isBody ? {left: 0, top: 0} : offsetParent.getBoundingClientRect(); const x = evt.clientX + offsetParent.scrollLeft - offsetParentRect.left; const y = evt.clientY + offsetParent.scrollTop - offsetParentRect.top; diff --git a/lib/utils/positionFns.es6 b/lib/utils/positionFns.es6 index 3b88b7c4..e1591bdc 100644 --- a/lib/utils/positionFns.es6 +++ b/lib/utils/positionFns.es6 @@ -1,7 +1,7 @@ // @flow import {isNum, int} from './shims'; import ReactDOM from 'react-dom'; -import {getTouch, innerWidth, innerHeight, offsetXYFromParentOf, outerWidth, outerHeight} from './domFns'; +import {getTouch, innerWidth, innerHeight, offsetXYFromParent, outerWidth, outerHeight} from './domFns'; import type Draggable from '../Draggable'; import type {Bounds, ControlPosition, DraggableData} from './types'; @@ -66,7 +66,11 @@ export function canDragY(draggable: Draggable): boolean { export function getControlPosition(e: MouseEvent, touchIdentifier: ?number, draggableCore: DraggableCore): ?ControlPosition { const touchObj = typeof touchIdentifier === 'number' ? getTouch(e, touchIdentifier) : null; if (typeof touchIdentifier === 'number' && !touchObj) return null; // not the right touch - return offsetXYFromParentOf(touchObj || e, ReactDOM.findDOMNode(draggableCore), draggableCore); + // User can provide an offsetParent if desired. + const offsetParent = draggableCore.props.offsetParent || + ReactDOM.findDOMNode(draggableCore).offsetParent || + document.body; + return offsetXYFromParent(touchObj || e, offsetParent); } // Create an data object exposed by 's events diff --git a/specs/draggable.spec.jsx b/specs/draggable.spec.jsx index a0b68a7b..a8f53e5a 100644 --- a/specs/draggable.spec.jsx +++ b/specs/draggable.spec.jsx @@ -456,32 +456,74 @@ describe('react-draggable', function () { }); it('should modulate position on scroll', function (done) { - // This test fails in karma under Chrome & Firefox, positioning quirks - // FIXME: Why? Chrome reports 2x scrollTo, Phantom reports 0x, Firefox reports 1x as it should - var is_ff = navigator.userAgent.toLowerCase().indexOf('Firefox') > -1; - if (!is_ff) return done(); - - var dragCalled = false; + let dragCalled = false; function onDrag(e, coreEvent) { assert(coreEvent.deltaY === 500); dragCalled = true; } drag = TestUtils.renderIntoDocument(
); - var node = ReactDOM.findDOMNode(drag); + const node = ReactDOM.findDOMNode(drag); + + // Create a container we can scroll. I'm doing it this way so we can still access . + // Enzyme (airbnb project) would make this a lot easier. + const fragment = fragmentFromString(` +
+
+
+
+ `); + transplantNodeInto(node, fragment, (f) => f.children[0]); TestUtils.Simulate.mouseDown(node, {clientX: 0, clientY: 0}); assert(drag.state.dragging === true); - document.body.style.height = '10000px'; - window.scrollTo(0, 500); - TestUtils.Simulate.mouseUp(node, {clientX: 0, clientY: 0}); + // Scroll the inner container & trigger a scroll + fragment.scrollTop = 500; + mouseMove(0, 0); + TestUtils.Simulate.mouseUp(node); setTimeout(function() { + assert(drag.state.dragging === false); assert(dragCalled === true); - assert(drag.state.clientY === 500); + assert(drag.state.y === 500); + // Cleanup + document.body.removeChild(fragment); done(); }, 50); + }); + + it('should respect offsetParent on nested div scroll', function (done) { + let dragCalled = false; + function onDrag(e, coreEvent) { + dragCalled = true; + // Because the offsetParent is the body, we technically haven't moved at all relative to it + assert(coreEvent.deltaY === 0); + } + drag = TestUtils.renderIntoDocument(
); + const node = ReactDOM.findDOMNode(drag); + + // Create a container we can scroll. I'm doing it this way so we can still access . + // Enzyme (airbnb project) would make this a lot easier. + const fragment = fragmentFromString(` +
+
+
+
+ `); + transplantNodeInto(node, fragment, (f) => f.children[0]); + + TestUtils.Simulate.mouseDown(node, {clientX: 0, clientY: 0}); + fragment.scrollTop = 500; + + mouseMove(0, 0); + TestUtils.Simulate.mouseUp(node); + setTimeout(function() { + assert(dragCalled); + // Cleanup + document.body.removeChild(fragment); + done(); + }, 50); }); describe('draggable callbacks', function () { @@ -597,3 +639,15 @@ function simulateMovementFromTo(drag, fromX, fromY, toX, toY) { mouseMove(toX, toY); TestUtils.Simulate.mouseUp(node); } + +function fragmentFromString(strHTML) { + var temp = document.createElement('div'); + temp.innerHTML = strHTML; + return temp.children[0]; +} + +function transplantNodeInto(node, into, selector) { + node.parentNode.removeChild(node); + selector(into).appendChild(node); + document.body.appendChild(into); +}