Skip to content

Commit

Permalink
fix(grid): incorrect x and y passed to onStop
Browse files Browse the repository at this point in the history
This was caused by `onStop` erroneously recalculating coordinates,
which is not necessary / should not happen in the real world, as `mouseup`
and `touchend` should not pass `clientX` and `clientY` values meaningfully
different than the last corresponding move event. In fact, they are already
ignored by `<Draggable>`.

For this reason, we can simply trust the `lastX` and `lastY` in `onDragStop`,
and pass those back.

Fixes #413, bokuweb/react-rnd#453
  • Loading branch information
STRML committed Jul 29, 2020
1 parent 9c5e8c3 commit 8d95806
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 6 deletions.
7 changes: 2 additions & 5 deletions lib/DraggableCore.js
Expand Up @@ -381,11 +381,8 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D

handleDragStop: EventHandler<MouseTouchEvent> = (e) => {
if (!this.state.dragging) return;

const position = getControlPosition(e, this.state.touchIdentifier, this);
if (position == null) return;
const {x, y} = position;
const coreEvent = createCoreData(this, x, y);
// No dragging is possible on stop, so we just echo back lastX & lastY.
const coreEvent = createCoreData(this, this.state.lastX, this.state.lastY);

// Call event handler
const shouldContinue = this.props.onStop(e, coreEvent);
Expand Down
50 changes: 49 additions & 1 deletion specs/draggable.spec.jsx
Expand Up @@ -153,6 +153,54 @@ describe('react-draggable', function () {
simulateMovementFromTo(drag, 0, 0, 100, 100);
});

// https://github.com/STRML/react-draggable/issues/413
it('should adjust draggable data output when `grid` prop supplied', function () {
let dragCount = 0;
function onDrag(event, data) {
if (dragCount === 0) {
// Should be 150, not 160
assert(data.x === 150);
assert(data.y === 150);
assert(data.lastX === 0);
assert(data.lastY === 0);
} else {
// Should be 200, not 180 (snaps)
assert(data.x === 200);
assert(data.y === 150);
assert(data.lastX === 150);
assert(data.lastY === 150);
}
}
function onDragStop(event, data) {
if (dragCount === 0) {
assert(data.x === 150);
assert(data.y === 150);
// As this is another step: drag, then dragStop
// dragStops lastX always === x
assert(data.lastX === 150);
assert(data.lastY === 150);
} else {
assert(data.x === 200);
assert(data.y === 150);
assert(data.lastX === 200);
assert(data.lastY === 150);
}
}
drag = TestUtils.renderIntoDocument(
<Draggable
grid={[50, 50]}
onDrag={onDrag}
onStop={onDragStop}>
<div />
</Draggable>
);

simulateMovementFromTo(drag, 0, 0, 160, 160);
// then, move again, should snap `x` to 200
dragCount++;
simulateMovementFromTo(drag, 150, 150, 180, 160);
});

it('should throw when setting className', function () {
drag = (<Draggable className="foo"><span /></Draggable>);

Expand Down Expand Up @@ -959,7 +1007,7 @@ function simulateMovementFromTo(drag, fromX, fromY, toX, toY) {

TestUtils.Simulate.mouseDown(node, {clientX: fromX, clientY: fromX});
mouseMove(toX, toY, node);
TestUtils.Simulate.mouseUp(node);
TestUtils.Simulate.mouseUp(node, {clientX: toX, clientY: toY});
}

function fragmentFromString(strHTML) {
Expand Down

0 comments on commit 8d95806

Please sign in to comment.