Sortable: added checks for the helper touching the edge of the containment while dragging

[lukeapage]

Rebased from #1060 - I'll handle any comments to get this in as @mickylad is busy at the moment

Fixed #5772 - Sortable: containment parent restricting replacement of first and last elements

This issue can be reproduced based on where the mouse is positioned when sorting - in the patched versions you can drag from anywhere in the item to trigger a rearrangement, where as in the unpatched versions you must drag such that the initial mouse down position inside the item will overlap with the target item.

There is also an existing issue where the vertical container will be pushed down when sorting into the last index, which is not fixed in this pull request.

[HugoHeneault]

Thanks for PR!

There is also an existing issue where the vertical container will be pushed down when sorting into the last index, which is not fixed in this pull request.

Any update or PR about this one?

[lukeapage]

@in4matik No.. We've been waiting a very long time to have this merged (since 16th Aug 2013!!)

There isn't much incentive to further fix issues until this is merged or even commented on.

[mikesherov]

@lukeapage sorry for the long delay, and thanks for contributing! There are a lot of style guide violations here, which I can clean up myself while landing this, but you might want to take a look at : http://contribute.jquery.org/style-guide/js/ if you're going to contribute more patches.

Also, please sign our CLA: http://contribute.jquery.org/CLA so I can land this patch!

Thanks again for contributing!

[lukeapage]

Hi,

@morchard did the work and has signed the CLA.
I rebased and have also signed the CLA (I have previously contributed some documentation and test fix PR's which have been merged)

Please can you point out the style violations? Do you not use jscs for style checking (and that has passed)? We've had a look and can't see anything obvious, but it would be good to learn what is a problem.

[lukeapage]

Is is that this should have only 1 level of indentation?

[mikesherov]

Yeah, mostly around spacing, actually. Unfortunately, we've yet to do a bulk JSCS cleanup yet and so haven't turned it on for the messier code in the codebase.

As a maintainer of JSCS myself, it pains me to say that.

[lukeapage]

@mikesherov any progress? Are there any outstanding style violations to fix? I've fixed the ones I found.

[mikesherov]

@lukeapage, we're releasing 1.12 soon. This patch will definitely be in there.

[victor-homyakov]

_mouseDrag already has 120 lines of code and three levels of nesting (for() { if() { if() {...} } }).

Adding ~20 more lines and one more nesting level makes this even more harder to read and understand (not to mention further fixes/improvements).

Maybe it's time to split it into smaller parts/extract some helper functions?

[lukeapage]

@victor-homyakov this patch was first proposed August 2013 and has been lingering around since then with no reason why it hasn't been pulled.

It's probably a fair point, but I think it shouldn't be part of this PR.

[cornem]

We would really like to see this PR included in one of the library's next releases. Why has this fix been hanging around for so long? Could someone please provide an update on this, or a possible work-around?