Not All Scroll Events are Heard by $document

[HandyAndyShortStack]

I was having an issue yesterday when using this library for sorting a group of lists inside a scrollable div. If I began to drag a list item and scrolled the div during the drag, I found that the placeholder would no longer follow my mouse position correctly. It seemed to be tracking a point directly above or below my pointer by the same distance that I had scrolled. Here is an example of what I experienced: http://bl.ocks.org/handyandyshortstack/raw/9005218/.

Upon further investigation, I found this was because the ContainerGroup was not recalculating container and offset metrics in response to these scroll events. It seems that scroll events inside scrollable elements never reach the document, so ContainerGroup.prototype.scroll is not called when they are fired.

I patched up my copy of jquery-sortable by placing the following line at the end of ContainerGroup.prototype.toggleListeners (my copy is based on v0.9.11):

$('#my-scrollable-element')[method]('scroll', that.scrollProxy)

I am already passing in a namespaced jQuery object at the end of the file so one more minor modification is no big deal, but I really think this issue should be dealt with more properly. I am not sure how best to go about it though. Perhaps an additional group default could be exposed where users could pass in additional elements or selectors on which to attach event listeners while dragging. It also might be feasable to look explicitly for css indicating scrollability recursively on the parents of sortable elements and attach listeners to those if they are found.

[johnny]

I get an application error on the link you provided.

I am on the fence with this one. Making this plugin work with every kind of scrolling scenario is impossible and i want to keep this out as much as possible. Therefore autoscroll will never be part of this plugin.

I had once pondered the idea of passing a scrollable element as an option, but scraped it again. It was an option too specialized.

I do like the idea of looking at the parents though. I will investigate this

[HandyAndyShortStack]

Hmm it looks like bl.ocks.org is down. Here is the source https://gist.github.com/HandyAndyShortStack/9005218
and here is an alternate hosted version
http://jquery-sortable_scrolling_problem_demo.meteor.com/

[HandyAndyShortStack]

I was about to formulate a pull request when I realized that my solution in commit fce46da would not work for some situations involving two or more grouped sortable lists. If you begin dragging an item from a sortable list and then scroll a parent of an item in a separate list that is not a parent of the first list, you will encounter the same behavior as I described in the initial issue description. In order to remedy this, I need to look at the parents of every item of every container in the group. I can make the process somewhat more efficient by stopping the search up the parent chain at the containter element and searching up from the container element only once per container. If items are nested I do not see a way of avoiding checking the parent chain between the nest parent and the container element more than once, but this should not cause significant performance issues.

I have amended commit fce46da to remove the portion of the commit description indicating that it resolves this issue and am working on a solution that solves the cases described in the preceding paragraph. You can see my fix for parents of the dragged element working here:
http://bl.ocks.org/HandyAndyShortStack/9266959

[HandyAndyShortStack]

Okay, @aatrostle and I have put together a demo of the problem with my first fix attempt:
http://bl.ocks.org/HandyAndyShortStack/raw/9275414/
It acts exactly as I expected it would. Working on a solution that fixes this case as well.

[johnny]

Great, thanks for taking this in your hands and for providing so many
examples!

The problems you described seem vaguely familiar. I am very interested in
the solution you come up with.

On Fri, Feb 28, 2014 at 6:30 PM, Andy Short [email protected]:

Okay, @aatrostle https://github.com/aatrostle and I have put together a
demo of the problem with my first fix attempt:
http://bl.ocks.org/HandyAndyShortStack/raw/9275414/
It acts exactly as I expected it would. Working on a solution that fixes
this case as well.

Reply to this email directly or view it on GitHubhttps://github.com//issues/67#issuecomment-36374361
.

[HandyAndyShortStack]

I have implemented a possible solution to the issue in this commit:
HandyAndyShortStack@5a89cc9
It finds any parent element of any item within the group that has css indicating possible scrollability and makes sure to toggle scroll handlers on these elements properly. In my browser this process added about 5 milliseconds to the dragInit function. For my purposes I am happy to sacrifice a couple of milliseconds at the beginning of the drag process to fix the scrolling issues, but I felt that this was too much of a performance hit to make it a default option. I therefore added a group option called scrollParent that enables this feature when is set to true, but defaults to false. I have not formulated a pull request because I would like a little feedback from @johnny before making changes to the plugin's public API. You can see my solution in action here:
http://bl.ocks.org/HandyAndyShortStack/raw/9280345/

[johnny]

Thank you again for working this and please excuse my long silence. I struggle to find the time to work on oss projects atm.

I looked at your code and I have several suggestions.

1. I'd like for scrollParent to accept a jquery object. This might solve many use cases and would not need to require search.
2. The scrollStyles should be global.
3. The search should only happen, if the container is enabled
4. Instead of iterating over all elements and their parrents, you should be able to use parents(). This would relieve you of the need to keep track of visited elements.
5. css() accepts an array. You could use that to look up the attributes

And finally, I have a question:
Why do you need to iterate over all the items? Is it that there might be a scroll element between the items and the container?

Please open a pull request, even if you have not addressed any of the above points. A pull request is easier to discuss and directly shows the complete diff.

[HandyAndyShortStack]

Thanks for the feedback. I will open a pull request incorporating the suggested changes as soon as I have time. In regard to your question, It does seem that there can be scrollable elements between items and their containers in nested situations. I may be misreading the library though. I will endeavor to produce an example of such a situation.

[jgerigmeyer]

Any progress on this? It'd be great to get this fix merged into a release!