Skip to content

FIX: #454 #455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions src/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ test("should call onDragStart when start dragging", async t => {
.simulate("mousedown");
t.is(onDragStart.callCount, 1);
t.is(onDragStart.firstCall.args[0].type, "mousedown");
t.is(onDragStart.firstCall.args[1].x, 200);
t.is(onDragStart.firstCall.args[1].y, 200);
t.is(onDragStart.firstCall.args[1].x, 100);
t.is(onDragStart.firstCall.args[1].y, 100);
});

test("should call onDrag when dragging", async t => {
Expand All @@ -124,9 +124,9 @@ test("should call onDrag when dragging", async t => {
.simulate("mousedown", { clientX: 0, clientY: 0 });
mouseMove(200, 220);
t.is(onDrag.callCount, 1);
t.is(onDrag.firstCall.args[1].x, 600);
t.is(onDrag.firstCall.args[1].y, 620);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(400px, 420px)"), -1);
t.is(onDrag.firstCall.args[1].x, 300);
t.is(onDrag.firstCall.args[1].y, 320);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(300px, 320px)"), -1);
});

test("should call onDragStop when drag stop", async t => {
Expand All @@ -137,10 +137,10 @@ test("should call onDragStop when drag stop", async t => {
.at(0)
.simulate("mousedown", { clientX: 0, clientY: 0 });
mouseMove(200, 220);
mouseUp(100, 120);
mouseUp(200, 220);
t.is(onDragStop.callCount, 1);
t.is(onDragStop.firstCall.args[1].x, -100);
t.is(onDragStop.firstCall.args[1].y, -100);
t.is(onDragStop.firstCall.args[1].x, 300);
t.is(onDragStop.firstCall.args[1].y, 320);
});

test("should dragging disabled when axis equals none", async t => {
Expand Down Expand Up @@ -208,7 +208,7 @@ test("should snap when dragging smaller than threshold", async t => {
.at(0)
.simulate("mousedown", { clientX: 0, clientY: 0 });
mouseMove(14, 49);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(108px, 108px)"), -1);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(100px, 100px)"), -1);
});

test("should snap when dragging larger than threshold", async t => {
Expand All @@ -220,7 +220,7 @@ test("should snap when dragging larger than threshold", async t => {
.at(0)
.simulate("mousedown", { clientX: 0, clientY: 0 });
mouseMove(15, 50);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(138px, 208px)"), -1);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(130px, 200px)"), -1);
});

test("should limit position by parent bounds", async t => {
Expand All @@ -244,7 +244,7 @@ test("should limit position by parent bounds", async t => {
.childAt(0)
.getDOMNode()
.getAttribute("style") || ""
).indexOf("transform: translate(708px, 508px)"),
).indexOf("transform: translate(700px, 500px)"),
-1,
);
});
Expand Down Expand Up @@ -275,7 +275,7 @@ test("should limit position by selector bounds", async t => {
.childAt(0)
.getDOMNode()
.getAttribute("style") || ""
).indexOf("translate(908px, 708px)"),
).indexOf("translate(900px, 700px)"),
-1,
);
});
Expand Down Expand Up @@ -468,7 +468,7 @@ test("should move x when resizing left", async t => {
t.deepEqual(onResize.getCall(0).args[2].clientWidth, 150);
t.deepEqual(onResize.getCall(0).args[2].clientHeight, 100);
t.deepEqual(onResize.getCall(0).args[3], { width: 50, height: 0 });
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(58px, 108px)"), -1);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(50px, 100px)"), -1);
});

test("should move y when resizing top", async t => {
Expand Down Expand Up @@ -512,7 +512,7 @@ test("should move y when resizing top", async t => {
t.deepEqual(onResize.getCall(0).args[2].clientWidth, 100);
t.deepEqual(onResize.getCall(0).args[2].clientHeight, 150);
t.deepEqual(onResize.getCall(0).args[3], { width: 0, height: 50 });
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(108px, 58px)"), -1);
t.not((rnd.getDOMNode().getAttribute("style") || "").indexOf("transform: translate(100px, 50px)"), -1);
});

test("should snapped by original grid when x axis resizing smaller then threshold", async t => {
Expand Down Expand Up @@ -742,8 +742,8 @@ test("should clamped by parent size", async t => {
.at(0)
.simulate("mousedown", { clientX: 0, clientY: 0 });
mouseMove(1200, 1200);
t.is((rnd.childAt(0).getDOMNode() as HTMLElement).style.width, "800px");
t.is((rnd.childAt(0).getDOMNode() as HTMLElement).style.height, "600px");
t.is((rnd.childAt(0).getDOMNode() as HTMLElement).style.width, "808px");
t.is((rnd.childAt(0).getDOMNode() as HTMLElement).style.height, "608px");
});

test("should clamped by selector size", async t => {
Expand Down Expand Up @@ -788,14 +788,14 @@ test("should clamped by selector size", async t => {
.childAt(0)
.childAt(0)
.getDOMNode() as HTMLElement).style.width,
"1000px",
"1008px",
);
t.is(
(rnd
.childAt(0)
.childAt(0)
.getDOMNode() as HTMLElement).style.height,
"800px",
"808px",
);
});

Expand Down
54 changes: 51 additions & 3 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,15 @@ export class Rnd extends React.Component<Props, State> {
}

onDragStop(e: Event, data: DraggableData) {
const { left, top } = this.getOffsetFromParent();

let draggablePosition = {
x: data.x + left,
y: data.y + top,
};

if (this.props.onDragStop) {
const { left, top } = this.getOffsetFromParent();
this.props.onDragStop(e, { ...data, x: data.x + left, y: data.y + top });
this.props.onDragStop(e, { ...data, ...draggablePosition });
}
}

Expand Down Expand Up @@ -439,7 +445,9 @@ export class Rnd extends React.Component<Props, State> {
}

getOffsetFromParent(): { top: number; left: number } {
const { dragAxis } = this.props;
const parent = this.getParent();

if (!parent) {
return {
top: 0,
Expand All @@ -451,10 +459,45 @@ export class Rnd extends React.Component<Props, State> {
const parentTop = parentRect.top;
const selfRect = this.getSelfElement().getBoundingClientRect();
const position = this.getDraggablePosition();
return {
const rawPosition = {
left: selfRect.left - parentLeft - position.x,
top: selfRect.top - parentTop - position.y,
};
let adjustedPosition = rawPosition;

switch (dragAxis) {
case "x":
adjustedPosition = {
...rawPosition,
top: 0,
};
break;
case "y":
adjustedPosition = {
...rawPosition,
left: 0,
};
break;
case "both":
adjustedPosition = {
...rawPosition,
};
break;
case "none":
adjustedPosition = {
...rawPosition,
left: 0,
top: 0,
};
break;
default:
adjustedPosition = {
...rawPosition,
};
break;
}

return adjustedPosition;
}

render() {
Expand All @@ -473,9 +516,11 @@ export class Rnd extends React.Component<Props, State> {
onResizeStart,
onResize,
onResizeStop,
snapOnResizeStop,
onDragStart,
onDrag,
onDragStop,
snapOnDragStop,
resizeHandleStyles,
resizeHandleClasses,
enableResizing,
Expand All @@ -494,14 +539,17 @@ export class Rnd extends React.Component<Props, State> {
...cursorStyle,
...style,
};

const { left, top } = this.getOffsetFromParent();
let draggablePosition;

if (position) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chathuraa Sorry, could you please separate these block to another function or method like createNewPosition 🙏

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bokuweb ,

I had to update some tests, I couldnt figure out the reason for '8px' discrepancy. I managed to get pretty much everything working. apart from the uncontrolled bounds.

Thanks,
Chat.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh..Is it due to default margin? If you set html, body { padding: 0, margin: 0 }; to ./fixture.url, will the results be as you expected?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bokuweb,

oh okay, that makes sense. I've updated the getOffsetFromParent and the tests so the padding and margins become irrelevant.

The update on the getOffsetFromParent, seems to cause an issue with uncontrolled bounds. I cant seems to find where the issue is. Any suggestions?

Thanks,
Chat.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand what is issue. Why not try applying changes with you only when using position props ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that. however the issue was there on almost every where the getOffsetFromParent been used.
ie:
componentDidMount,
dragStart,
onDrag,
onDragStop,
etc

So decided to update the function to return left/top based on axis restrictions, rather than setting the logic on each function.

Re: the bounds, it only affecting the uncontrolled bounds. all the other scenarios including controlled bounds are working.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm, I'll try to fix it 🙏 So, this issue is complex....

draggablePosition = {
x: position.x - left,
y: position.y - top,
};
}

return (
<Draggable
ref={c => {
Expand Down
3 changes: 3 additions & 0 deletions stories/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import SandboxLockAspectRatioWithBounds from "./sandbox/lock-aspect-ratio-with-b

import LockAspectRatioBasic from "./lock-aspect-ratio/basic";

import RestrictDrag from "./restrict-axis/restrict-drag";

storiesOf("bare", module).add("bare", () => <Bare />);

storiesOf("basic", module)
Expand Down Expand Up @@ -58,3 +60,4 @@ storiesOf("sandbox", module)
.add("lock aspect ratio with bounds", () => <SandboxLockAspectRatioWithBounds />);

storiesOf("ratio", module).add("lock aspect ratio", () => <LockAspectRatioBasic />);
storiesOf("restrict axis", module).add("restrict drag axis", () => <RestrictDrag />);
102 changes: 102 additions & 0 deletions stories/restrict-axis/restrict-drag.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import React from "react";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👍

import { Rnd } from "../../src";

type State = {
x: number;
y: number;
width: number;
height: number;
restrict: "x" | "y" | "both" | "none";
};

const style = {
display: "flex",
alignItems: "center",
justifyContent: "center",
border: "solid 1px #ddd",
background: "#f0f0f0",
};
export default class RestrictDrag extends React.Component<{}, State> {
constructor(props) {
super(props);
this.state = {
width: 200,
height: 200,
x: 0,
y: 80,
restrict: "x",
};
}
handleDragStop = (e, d) => {
const restrictedAxis = this.state.restrict;
let pos = {};
switch (restrictedAxis) {
case "x":
pos = {
x: d.x,
};
break;
case "y":
pos = {
y: d.y,
};
break;
case "both":
pos = {
x: d.x,
y: d.y,
};
break;
case "none":
pos = {};
break;
default:
pos = {};
break;
}
this.setState({ ...pos });
};

render() {
console.log("STATE", this.state);
return (
<div>
<label>
Restrict Axis:{" "}
<select
onChange={e => {
this.setState({ restrict: e.target.value as "x" | "y" | "both" | "none" });
}}
>
<option value="x">X</option>
<option value="y">Y</option>
<option value="both">Both</option>
<option value="none">None</option>
</select>
</label>
<Rnd
style={style}
size={{
width: this.state.width,
height: this.state.height,
}}
position={{
x: this.state.x,
y: this.state.y,
}}
onDragStop={this.handleDragStop}
dragAxis={this.state.restrict}
onResize={(e, direction, ref, delta, position) => {
this.setState({
width: ref.offsetWidth,
height: ref.offsetHeight,
...position,
});
}}
>
001
</Rnd>
</div>
);
}
}
2 changes: 1 addition & 1 deletion stories/size/size-percent-uncontrolled.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import Rnd from "../../src";
import { Rnd } from "../../src";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :)

import { style } from "../styles";

export default () => (
Expand Down