#9041 closed bug (fixed)
Sortable: Unstable dragging / sorting with jQuery UI 1.9.1 and above
Reported by: | marventus.wp | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 1.10.2 |
Component: | ui.sortable | Version: | 1.9.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
There seems to be a bug involving the sorting of elements in jQuery UI > 1.9.0 (affected versions: 1.9.1, 1.9.2, 1.10.0). Dragging and sorting are very jerky and unstable in source container are incredibly hard to resort after original sorting in target container.
I created a small demo tester to try to narrow down the problem:
http://elblawg.com/jqueryui-test/.
That's how I was able to determine the issue starts occurring with 1.9.1 and above. The offending line is 4310 of the full 1.10.0 version (ui-sortable section). Here is the diff I created:
--- C:/xampp/htdocs/blogs/elblawg/jqueryui-test/js/jquery-ui-1.10.0.js vie feb 1 01:55:45 2013 +++ C:/xampp/htdocs/blogs/elblawg/jqueryui-test/js/jquery-ui-1.10.0_2.js vie feb 1 01:55:31 2013 @@ -4307,7 +4307,7 @@ if(this.containers.length === 1) { this.containers[innermostIndex]._trigger("over", event, this._uiHash(this)); this.containers[innermostIndex].containerCache.over = 1; - } else { + } else if(this.currentContainer != this.containers[innermostIndex]) { //When entering a new container, we will find the item with the least distance and append our item near it dist = 10000;
LMK if you need further assistance.
Marv
Change History (13)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Status: | new → open |
---|---|
Summary: | Unstable dragging / sorting with jQuery UI 1.9.1 and above → Sortable: Unstable dragging / sorting with jQuery UI 1.9.1 and above |
I can see the issue as well, here's a fiddle you can play with the version numbers in http://jsfiddle.net/tj_vantoll/FeTr2/.
This broke with this commit that landed in 1.9.1: https://github.com/jquery/jquery-ui/commit/bae06d2b1ef6bbc946dce9fae91f68cc41abccda#ui/jquery.ui.sortable.js. That same commit is also blamed for #8792 with had a pull that attempted to re-add the same line you're trying to add, but unfortunately re-adding that line causes other regressions.
#8792 was related to floating and this one isn't although it appears the root cause is the likely the same.
comment:3 Changed 9 years ago by
I also have the same issue. I've recreated the issue here: http://jsfiddle.net/2rt9Z/ I had to downgrade to 1.9.0.
comment:4 Changed 9 years ago by
This is also the cause of another issue. As of version 1.9.1, the over event fires on every pixel movement. I've recreated the issue at http://jsfiddle.net/PZB5x/. Watch the console output.
comment:5 follow-up: 6 Changed 9 years ago by
Hello again. Ok, so it wasn't just me then, ;-P
@tj.vantoll: I understand what you mean. Could you provide more information on the other regressions you noticed? I tested the fix and it didn't seem to generate any issues, but, of course, it is impossible to contemplate every possible scenario. Thanks!
comment:6 Changed 9 years ago by
Replying to marventus.wp:
Hello again. Ok, so it wasn't just me then, ;-P
@tj.vantoll: I understand what you mean. Could you provide more information on the other regressions you noticed? I tested the fix and it didn't seem to generate any issues, but, of course, it is impossible to contemplate every possible scenario. Thanks!
The commit that caused this issue was a fix for #8572, #8573, and #8574. What we really need are tests that show that re-adding that line fixes this issue, and doesn't cause any regressions with #8572 / #8573 / #8574.
comment:7 Changed 9 years ago by
Hello again.
tl;dr: Re-adding the old line I mentioned (4310) does not seem to cause any serious trouble in #8572, #8573, or #8574
.
Detailed: I ran some tests on JsFiddle based on the tickets you provided and the demos created by @zhizhangchen and the proposed fix does not seem to revert sorting action to any of the situations described in any of them. I did notice that, in respect to #8572, the placeholder takes a while before "coming down" to a new position, but sorting works well regardless of that. IMHO, restoring line 4310 the way it was in 1.9.0 leads to a more stable sorting experience overall.
Tests:
1. http://jsfiddle.net/Marventus/ZRz5W/1/
2. http://jsfiddle.net/Marventus/ytxEG/
3. http://jsfiddle.net/Marventus/RMVpV/
comment:8 Changed 9 years ago by
I've sent a pull request to fix these problems: https://github.com/jquery/jquery-ui/pull/916
comment:9 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Sortable: Skip triggering over event if it's alreay over the continer. Fixes #9041: the over event fires on every pixel movement
Changeset: 07ce771a13504b851bb9f74c8ce8e960d207384a
comment:10 Changed 9 years ago by
Milestone: | none → 1.10.2 |
---|
comment:11 Changed 8 years ago by
I believe this bugfix introduced a new bug that causes the "out" event not to fire with connected lists.
comment:12 follow-up: 13 Changed 8 years ago by
I believe this bugfix introduced a new bug that causes the "out" event not to fire when using connected lists.
I was also able to recreate the issue in JSFiddle.