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

Wrong top calculation for scroll ancestors with fixed position? #61

Closed
bbenzikry opened this issue Jan 16, 2021 · 14 comments
Closed

Wrong top calculation for scroll ancestors with fixed position? #61

bbenzikry opened this issue Jan 16, 2021 · 14 comments
Labels
bug Something isn't working

Comments

@bbenzikry
Copy link

Hi there, we're using the library to create a dnd experience between a scrollable fixed position div and a sortable ( scrollable ) drop zone.
This mostly works well ( we're currently using PR #54 as our basis ) but causes our DragOverlay to appear in the wrong top position after scrolling the drop zone.

Below is an example based on the sandbox shown in #43
scroll-fixed-3
https://codesandbox.io/s/react-dnd-grid-forked-gls5l?file=/src/App.js

We made a change to check whether an element ancestor is fixed in getScrollableAncestors, which causes only that ancestor to be returned ( Obviously, this is just a small demo and doesn't deal with more complex scenarios / performance etc.)

https://github.com/bbenzikry/dnd-kit/blob/a2cd7052c1cdf04e05af936127650b1dd6db7485/packages/core/src/utilities/scroll/getScrollableAncestors.ts#L22-L26

This makes the following return the expected value for our scenario

const scrollOffsets = getScrollOffsets(scrollableAncestors);
const top = offsetTop - scrollOffsets.y;

@namroodinc
Copy link

Has there been any fixes for this bug?

@joshjg
Copy link

joshjg commented May 13, 2021

Thanks @bbenzikry - I tried the changes from your fork locally and it solved the issues I was having.

@bbenzikry
Copy link
Author

Thanks @bbenzikry - I tried the changes from your fork locally and it solved the issues I was having.

Happy to have helped :)

@joshjg
Copy link

joshjg commented Aug 3, 2021

@bbenzikry @clauderic Any plans to open this fix as a PR? Any help needed?

@bbenzikry
Copy link
Author

bbenzikry commented Aug 3, 2021

I don't mind updating my fork and opening a PR as long as there is no objection
Note that my fork is currently based on #54

@cantrellnm
Copy link
Contributor

I needed this working for my own project so I opened PR #415. It's a small fix but should be more flexible than bbenzikry's solution since the fixed node can be at any level of ancestry and have scrollable children. This worked to fix my own issues but I don't know if it will fix all issues related to fixed containers.

@bbenzikry
Copy link
Author

bbenzikry commented Aug 19, 2021

@clauderic Can we merge #415? I can only speak for myself - but I'm stuck with my own fork and would really like to be as close as possible to master without worrying about updating from upstream manually

@clauderic
Copy link
Owner

Fixed by #415

@atanasov-deyan
Copy link

atanasov-deyan commented Aug 23, 2021

Hello @clauderic ,
I might be doing something wrong, but I still can reproduce the issue with the latest version both in our use case , and in the example linked in the initial issue description. I have just forked the sandbox and updated dnd-kit to latest version, but the problem persists: https://codesandbox.io/s/react-dnd-grid-forked-o0vof , so I am kinda unsure whether this issue is fully resolved by #415 . Can someone else also confirm my findings?

@clauderic
Copy link
Owner

clauderic commented Aug 23, 2021

@atanasov-deyan the versions of @dnd-kit that fix this are not published under the latest tag on npm yet, they are tagged with next. You can consume them right away by installing the @dnd-kit packages tagged with next:

npm install @dnd-kit/core@next @dnd-kit/sortable@next

I updated the CodeSandbox with the next packages:
https://codesandbox.io/s/react-dnd-grid-forked-1h9nw?file=/src/App.js:321-333

I think there's still some other issues with that sandbox, but I believe they're related to #43

Follow this PR to know when the next packages will be released under latest:
#342

@atanasov-deyan
Copy link

atanasov-deyan commented Aug 23, 2021

ah, I knew I must be doing something wrong! Thanks for the clarification, @clauderic ! I can confirm that using dnd-kit next solves the problem for our use case, too!

@sbaechler
Copy link

For anyone googling this error: The fix has been released in @dnd-kit/core 4.0.0.

@0xtlt
Copy link

0xtlt commented Mar 13, 2023

Hi everyone, I have the same problem with dnd kit/code : "6.0.8"

Am I the only one with the same problem at this version ?

@akodkod
Copy link

akodkod commented Apr 20, 2023

Unfortunately, I have the same problem. I'm using:

"@dnd-kit/core": "^6.0.8",
"@dnd-kit/sortable": "^7.0.2",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants