Skip to content
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.

Sign up for GitHub

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

FIX: #454 #455

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

FIX: #454 #455

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
72c8876
FIX: #454
chathuraa Oct 3, 2018
cd1c888
FIX:#454 Refactor
chathuraa Oct 4, 2018
5a7d107
FIX:#454 Refactor
chathuraa Oct 4, 2018
026963a
FIXED: #454
chathuraa Oct 5, 2018
364ea75
FIXED:#454 STORIES
chathuraa Oct 6, 2018
993ebe4
Removed console log
chathuraa Oct 6, 2018
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
30 changes: 26 additions & 4 deletions src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,32 @@ export class Rnd extends React.Component<Props, State> {
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,
};
switch (dragAxis) {
case "x":
draggablePosition = {
x: position.x - left,
y: position.y,
};
break;
case "y":
draggablePosition = {
x: position.x,
y: position.y - top,
};
break;
case "none":
draggablePosition = {
x: position.x,
y: position.y,
};
break;
default:
draggablePosition = {
x: position.x - left,
y: position.y - top,
};
break;
}
}
return (
<Draggable
Expand Down