Sortable: added checks for the helper touching the edge of the containment while dragging #1424
Conversation
…nment while dragging. Fixed #5772 - Sortable: containment parent restricting replacement of first and last elements
…ile dragging. Fixed #5772 - Sortable: containment parent restricting replacement of first and last elements
…to first and last position
… Fixed #5772 Sortable: containment parent restricting replacement of first and last elements
lukeapage
force-pushed
the
sortable-patch
branch
from
e3c860a to
d4288e6
Compare
|
Thanks for PR!
Any update or PR about this one? |
|
@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. |
|
@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! |
|
Hi, @morchard did the work and has signed the CLA. 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. |
| expect(2); | ||
|
|
||
| var element = $("#sortable-horizontal").sortable({ | ||
| axis: "x", |
There was a problem hiding this comment.
Is is that this should have only 1 level of indentation?
|
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. |
|
@mikesherov any progress? Are there any outstanding style violations to fix? I've fixed the ones I found. |
|
@lukeapage, we're releasing 1.12 soon. This patch will definitely be in there. |
|
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? |
|
@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. |
|
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? |
|
Unfortunately @mikesherov never got around to merging this for 1.12. I'm going to close this due to the conflicts, but if you're still interested in getting the patches merged, please file a new PR. Sorry about the lack of feedback from the team. |
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.