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.

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

wants to merge 6 commits into from

Conversation

chathuraa
Copy link

Component change the Y position when Drag stops even though dragging is restricted to X axis

Component change the Y / X position when Drag stops even though dragging is restricted to X/Y axis
@bokuweb
Copy link
Owner

bokuweb commented Oct 3, 2018

@chathuraa Thanks for your great work :) Looks good. I'd like to ask you one more favor, could you please add reproduced case to storybook.

https://codesandbox.io/s/j1vj3j90ov

@chathuraa
Copy link
Author

chathuraa commented Oct 4, 2018 via email

@@ -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....

@chathuraa
Copy link
Author

chathuraa commented Oct 4, 2018 via email

@@ -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 :)

@@ -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 👍

@chathuraa
Copy link
Author

chathuraa commented Oct 8, 2018 via email

@jakub-chatrny
Copy link

Any chance to get this PR mergered?
Experiencing same issues

@jakub-chatrny
Copy link

For enyone facing same issue, there is workaround.
Just set vertical grid strep to some ridiculous value like dragGrid={[stepWidth, 10000]}

note: when I did set grid vertical value to 0 while using controlled state. Drag start behave really wierdly.

@FullstackJack
Copy link

Hello?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants