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

dragging working not correct #2009

Open
sashako54 opened this issue Jan 17, 2024 · 10 comments · May be fixed by #2017
Open

dragging working not correct #2009

sashako54 opened this issue Jan 17, 2024 · 10 comments · May be fixed by #2017

Comments

@sashako54
Copy link

sashako54 commented Jan 17, 2024

Describe the bug

dragging working not correct when you try to add prop isBounded to ResponsiveReactGridLayout

Your Example Website or App

https://codesandbox.io/p/sandbox/angry-grothendieck-lxtvsf?file=%2Fsrc%2FShowcaseLayout.js%3A88%2C10-88%2C35

Steps to Reproduce the Bug or Issue

  1. go to https://codesandbox.io/p/sandbox/angry-grothendieck-lxtvsf?file=%2Fsrc%2FShowcaseLayout.js%3A88%2C10-88%2C35
  2. try to drag some block in any direction
  3. its always dragged on up/left direction

Expected behavior

dragging follows cursor

react-grid-layout library version

1.4.4

Operating System Version

macos monterey 12.3

Browser

Chrome

Additional context

its working on 1.4.3

Screenshots or Videos

2024-01-17.12.55.22.mov
@jay0815
Copy link

jay0815 commented Jan 19, 2024

I updated to version 1.4.4 and also encountered the same issue. Through debugging, I found that it was due to the changes in the logic for calculating top and left in #1323 . The key issue was caused by the containerPadding. I locally patched the previous version of the GridItem file to fix it.

@artem-malko
Copy link

@jay0815 hey, could you describe your patch? What did you do?

@jay0815
Copy link

jay0815 commented Jan 25, 2024

@artem-malko here is my patch. [email protected]
I noticed that in the update from 1.4.3 to 1.4.4, commit #1323 added a calculation for the offset of containerPadding to the top and left properties, which affects the final position of the item. However, I found that calcXY already includes the handling of containerPadding.
Of course, this is a temporary solution, as it's just a rollback of that part of the calculation logic. Once I fully understand the entire logic of the position calculation, I might submit a PR to truly solve this issue.

@artem-malko
Copy link

@jay0815

By the way, if you set padding to [0, 0] — everything will work great)

@jay0815 jay0815 linked a pull request Feb 5, 2024 that will close this issue
14 tasks
@jay0815
Copy link

jay0815 commented Feb 5, 2024

@artem-malko hello,i have submitted a PR to fix the current bug.
In commit #1323, the author sought to correct an issue where ignoring the containerPadding attribute led to incorrect calculations of the final X/Y positions.
However, the fix was too simplistic and failed to take into account that, within the current library, the positions obtained during onDrag are already the outcome of calculations that incorporate the effects of both margin and containerPadding into the top and left values.
ContainerPadding is effectively equivalent to margin, because when containerPadding is not provided, margin is automatically used as the default for containerPadding. The reason why your provided containerPadding of [0,0] seems to work correctly is because, precisely at the [0,0] position, the calculation results from the current logic align with those from the correct calculation method.

@artem-malko
Copy link

@jay0815 thx a lot, will be waiting for review and release

@sikhaman
Copy link

sikhaman commented Feb 7, 2024

have same issue, isBounded works very weird with or without paddings

@nlevos
Copy link

nlevos commented Feb 14, 2024

Hi guys, I have the same problem. On dragging the block moves by itself to position 0,0.
I reverted to v1.4.3 and it works as expected!

@sikhaman
Copy link

@nlevos I also had thoughts to revert a version back, but want to wait an updates. I fixed issue by set up containerPadding={[0, 0]} as @jay0815 suggested. But this is just a patch, so waiting for the fixes and new version release

@MrZhouZh
Copy link

Have some issue, THX a lot.

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 a pull request may close this issue.

6 participants