search

kornelski / slip

Slip.js — UI library for manipulating lists via swipe and drag gestures
BSD 2-Clause "Simplified" License
2.44k stars 213 forks source link

Refactor scrolling, fixing several issues #72

Closed carter-thaxton closed 8 years ago

carter-thaxton commented 8 years ago

In previous versions, when scrolling as the user drags to the top or bottom of the scrollContainer (often the entire window), any changes to scrollContainer.scrollTop would be applied to target.startPosition.

The big idea here is to store the scrollContainer.scrollTop value at the beginning of the drag as target.origScrollTop, and then rather than apply multiple differences over and over to target.startPosition, we simply include the difference between the current scrollContainer.scrollTop and the origScrollTop in the calculation of getTotalMovement().

This sounds like it should make no difference, but in many cases the scrollContainer.scrollTop value can be changed by something other than this scrolling code, e.g. when a mouse wheel is turned while dragging, using a third finger to scroll on iOS, etc. Most notably, this fixes iOS Safari where the "stretching" effect is applied as the user attempts to scroll past the limits. Safari would animate this into the current value of scrollTop, and the element being dragged would hopelessly move away from the user's finger.

The scrolling code has been refactored into updateScrolling(), which is now only called from the reorder state, and not from all states, e.g. from swiping or the undecided state. This makes the behavior respond much more intuitively and avoids glitches when simply touching elements near the edges of the screen without intending to actually drag it up or down.

There's still a small little bug on iOS Safari, where changes to scrollContainer.scrollTop that should be out of bounds are not truncated, as they are on all other browsers that I've tested this on. In order to account for this, I've added some simple truncation in updateScrolling(), (see the calculation of offset using Math.min and Math.max), so we at least don't attempt to scroll beyond the triggerOffset. This is at least much better than the behavior I was seeing before this pull request, which was to drag the element off the screen by thousands of pixels.

I've tried this on a variety of browsers and mobile platforms. As far as I can tell, it behaves quite well.

kornelski commented 8 years ago

Thank you very much. This is quite solid work!