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

moveOnStartChange breaks drag position #94

Closed
AndrewRayCode opened this issue Oct 10, 2015 · 6 comments
Closed

moveOnStartChange breaks drag position #94

AndrewRayCode opened this issue Oct 10, 2015 · 6 comments

Comments

@AndrewRayCode
Copy link
Contributor

Check out the associated PR and open example/index.html. All it does is add moveOnStartChange to one drag element.

broken

You can see when you start dragging it the element jumps about 400 pixels down the page, not at the mouse coords, which I think is related to improperly handling/reading clientX and clientY. I was trying to implement a custom drag strategy to deal with my flux flow but it seems it's currently impossible because moveOnStartChange breaks dragging behavior.

@AndrewRayCode
Copy link
Contributor Author

Here's what's going on:

When drag begins, handleDragStart is called. This sets offsetX to the correct position:

offsetX: dragPoint.clientX - this.state.clientX

However, if moveOnStartChange is set, then getInitialState is called:

componentWillReceiveProps: function(newProps) {
    this.setState(this.getInitialState(newProps));

Which conveniently overwrites offsetX to 0:

getInitialState: function (props) {
    ...
    return {
        offsetX: 0, offsetY: 0,

I'm not sure how this feature is supposed to work? You can't update the start position during a drag because it breaks the drag coordinates.

@STRML
Copy link
Collaborator

STRML commented Oct 11, 2015

It really doesn't work well, and this is the impetus for
https://github.com/mzabriskie/react-draggable/tree/coreRefactor

On 10/11/15 1:34 AM, Andrew Ray wrote:

Here's what's going on:

When drag begins, |handleDragStart| is called. This sets |offsetX| to
the correct position:

|offsetX: dragPoint.clientX - this.state.clientX |

However, if |moveOnStartChange| is set, then |getInitialState| is called:

|componentWillReceiveProps: function(newProps) {
this.setState(this.getInitialState(newProps)); |

Which conveniently overwrites |offsetX| to 0:

|getInitialState: function (props) { ... return { offsetX: 0, offsetY: 0, |

I'm not sure how this feature is supposed to work? You can't update
the start position during a drag because it breaks the drag coordinates.


Reply to this email directly or view it on GitHub
#94 (comment).

@AndrewRayCode
Copy link
Contributor Author

Fixed readme so users can avoid the several hours of debugging I went through #95

@mweststrate
Copy link

I run into the same (or similar) issue, the proper fix is (I think) to not
ignore start position changes while dragging, even when the flag is set.

As a work around, I now keep the moveOnStartChange of false, and
instead change the key of the draggable if I want a new x/y; a new key
results in a fresh draggable. This isn't feasible in every scenario
though, as the key change has to come from the parent component. The
proposed work around in the readme works as well: call resetState() on a
ref to the draggable.

On Sat, Oct 10, 2015 at 10:57 PM, Andrew Ray notifications@github.com
wrote:

Check out the associated PR
#93 and open
example/index.html. All it does is add moveOnStartChange to one drag
element. You can see when you start dragging it the element jumps about 400
pixels down the page, which I think is related to improperly
handling/reading clientX and clientY. I was trying to implement a custom
drag strategy to deal with my flux flow but it seems it's currently
impossible because moveOnStartChange breaks dragging behavior.


Reply to this email directly or view it on GitHub
#94.

@STRML
Copy link
Collaborator

STRML commented Oct 27, 2015

moveOnStartChange is gone in v1.0.0, use <DraggableCore> if you need more control.

@STRML STRML closed this as completed Oct 27, 2015
@mweststrate
Copy link

Awesome! This separation will help a lot :)

On Tue, Oct 27, 2015 at 10:52 PM, Samuel Reed notifications@github.com
wrote:

Closed #94 #94.


Reply to this email directly or view it on GitHub
#94 (comment).

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

No branches or pull requests

3 participants