-
Notifications
You must be signed in to change notification settings - Fork 986
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 for using window as scrolling container #306
Fix for using window as scrolling container #306
Conversation
@clauderic there are a number of issues open for this. Is it possible to review this PR and release a fixed version? |
Hi, |
@yuval-vibo this has taken such a long time, I've forked the repo and published the dist folder myself. you can install it on npm like this: @clauderic I'm sure a lot of people would like to see this fixed in the main package... |
@ambewas Thanks Man! |
@aminland With your changes there's a strange behaviour in iOS: then the item remains in that position until I perform a page refresh, even navigating between sections |
@fabioimpe This should be fixed now. |
b9fe296
to
d7406e1
Compare
@clauderic Any progress on getting this merged in? Considering forking and merging because a client is requesting a fix immediately, but I will hold if if you plan on merging this soon. |
I also need this fix! |
I am maintaining the npm repo @fellow/react-sortable-hoc in light of this
one not being maintained...
|
Will this be merged in? I need it urgently because a client of mine is complaining, too! I cannot use @fellow/react-sortable-hoc because the Typescript compiler won't import it because it cannot find @types/@fellow/react-sortable-hoc So, I'm interested to get an official solution that also works with Typescript. |
Cool library--it would be great to see this fix get merged in. |
This PR fixes it all. @clauderic, please, it would help everyone to have this merged... |
* Fix for using window as scrolling container * fix issue with touches on mobile
Right now, even in the story book, this is broken on chrome, ff, & edge.
This just fixes it to not use document.body to set the scroll. There was also an issue with double computation in the animateNodes function, where if the scrollcontainer was set to the parent, and we calculate both window and parent separately, it's a problem. So the key is to just use the scrollcontainer to SET the scroll value, and assume both window and container can scroll.