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 for using window as scrolling container #306

Merged
merged 2 commits into from May 17, 2018

Conversation

aminland
Copy link
Contributor

@aminland aminland commented Nov 6, 2017

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.

@ambewas
Copy link

ambewas commented Jan 11, 2018

@clauderic there are a number of issues open for this. Is it possible to review this PR and release a fixed version?

#332
#304
#291

ambewas pushed a commit to ambewas/react-sortable-hoc that referenced this pull request Jan 15, 2018
@yuval-vibo
Copy link

Hi,
Any updates on this issue? waiting for a fix anxiously

@ambewas
Copy link

ambewas commented Feb 19, 2018

@yuval-vibo this has taken such a long time, I've forked the repo and published the dist folder myself.
Feel free to use this version where it has been fixed: https://github.com/ambewas/react-sortable-hoc

you can install it on npm like this:
npm install git+https://github.com/ambewas/react-sortable-hoc.git

@clauderic I'm sure a lot of people would like to see this fixed in the main package...

@yuval-vibo
Copy link

@ambewas Thanks Man!

@aminland
Copy link
Contributor Author

Until this repo starts accepting pull requests, you can get our company's version from npm directly: npm install --save @fellow/react-sortable-hoc

@fabioimpe
Copy link

fabioimpe commented Feb 27, 2018

@aminland With your changes there's a strange behaviour in iOS:
temp

then the item remains in that position until I perform a page refresh, even navigating between sections

@aminland
Copy link
Contributor Author

@fabioimpe This should be fixed now.

@aminland aminland force-pushed the window-as-scrolling-container branch from b9fe296 to d7406e1 Compare March 13, 2018 15:41
@brybrophy
Copy link

@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.

@rafael-sap
Copy link

I also need this fix!

@aminland
Copy link
Contributor Author

aminland commented Mar 16, 2018 via email

@mattes3
Copy link

mattes3 commented Mar 28, 2018

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.

@EricFries
Copy link

Cool library--it would be great to see this fix get merged in.

dmason30 pushed a commit to TMCApplications/react-sortable-hoc that referenced this pull request Apr 9, 2018
@eveningkid
Copy link

eveningkid commented Apr 13, 2018

This PR fixes it all. @clauderic, please, it would help everyone to have this merged...
@aminland's work saved my day. Thanks :)

thecoorum pushed a commit to thecoorum/react-sortable-hoc that referenced this pull request Feb 14, 2020
kiejo pushed a commit to Nuclino/react-sortable-hoc that referenced this pull request Jul 15, 2021
* Fix for using window as scrolling container

* fix issue with touches on mobile
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.

None yet

10 participants