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(grid): incorrect x and y passed to onStop #507

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

STRML
Copy link
Collaborator

@STRML STRML commented Jul 29, 2020

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

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
@STRML
Copy link
Collaborator Author

STRML commented Jul 29, 2020

@nicholaschiang could you verify this works for you - simply pin react-draggable to this git branch in your package.json resolutions?

"react-draggable": "git+https://git@github.com/strml/react-draggable.git#fix/grid-dragStop"

@nicholaschiang
Copy link

Just added it @STRML and it looks like the issue is still there:

jumping

You'll notice how the draggable keeps jumping a couple of pixels after dropping (there are some other unrelated issues shown in that GIF too... feel free to ignore them until I finish debugging on my end).

@nicholaschiang
Copy link

@STRML I also created a little repro of the same (or, at least, a similar) problem I was having when dynamically resizing the grid snap and RND size.

https://codesandbox.io/s/white-water-xrqp2?file=/src/index.js

You'll notice in that repro that the RND keeps jumping around and doesn't snap correctly to the resized grid (even though we're using the { x: data.lastX, y: data.lastY } workaround proposed in #413):

resizing-parent

Ideally, the X position should always be a multiple of the RND's width (displayed on the parent) and the Y position should always be a multiple of the RND's height (also displayed on the parent).

@trickpattyFH20
Copy link

Any idea on on a timeline for this? @STRML

@trickpattyFH20
Copy link

trickpattyFH20 commented Dec 29, 2021

@nicholaschiang could you verify this works for you - simply pin react-draggable to this git branch in your package.json resolutions?

"react-draggable": "git+https://git@github.com/strml/react-draggable.git#fix/grid-dragStop"

@STRML The repository does not exist at this link anymore.
Also, when trying to use the grid-dragStop branch from this repo as an npm dependency, the source code for this repo is not included in ./node-modules because the package.json in react-grid-layout/react-draggable does not include ./lib in the files field, or have./build committed to that branch.

Could you merge a fix for this issue, or include the source code on that branch so that it can be used as a dependency?

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.

Bug: onStop returns wrong positions when using grid
3 participants