#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 10 years ago by
comment:2 Changed 10 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 10 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 10 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 10 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 10 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 10 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 10 years ago by
I've sent a pull request to fix these problems: https://github.com/jquery/jquery-ui/pull/916
comment:9 Changed 10 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 10 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.