-
Notifications
You must be signed in to change notification settings - Fork 29
Add optional auto scrolling of the container during sort #52
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
Conversation
|
Thanks a lot for taking the time to work on this. I'll try to carefully review this later this evening. |
|
Ah you are probably correct. I will remove that behaviour entirely tonight or tomorrow if I find some more time. I was considering to enable it when autoscrolling is enabled, so that the dragged item doesn't show above the scrollbar when the scrollable element is a container, but I guess that isn't too much of an issue as showing the scrollbar is more useful/important. Take your time and thanks for your quick response! |
|
Removed the disabling of the default browser scroll behaviour |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
ValentinH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the vertical list story to show the new feature.
|
Was testing a bunch of different implementations and the lack of horizontal scroll support stung me, so added that as well. Wasn't too big of a change compared to the vertical auto scroll 🔥 |
|
It looks great! I've updated the other stories. Can I go ahead and merge this? |
|
I feel it's functionally complete and it passed all the different implementation tests I did. As it's optional and off by default releasing it should be fine imo. Thanks for your swiftness as always! |
|
Tested some more and fixed a small bug where it stopped working when an axis was locked (axes check was swapped) |
|
I'll do a last review and merge this tonight 👍 |
|
Published as react-easy-sort@1.8.0 🎉 Thanks again @tumispro !! |
This wasn't easy, but I think I got it working pretty robustly 🙂
Functionality:
- Prevent default browser scrolling (when auto scrolling is disabled), inspired by @phoebejaffe's solution (Autoscroll #4 (comment))This is a work in progress as it doesn't (and might never) support horizontal scrolling and it still needs to be tested thoroughly. It works pretty nicely so far for a few implementations I did myself though.
Update: supports both vertical as well as horizontal auto scrolling now. Worked smoothly in all my tests: