Fix sortable for floating items#916
Conversation
… floating. Fixes #8792 - Issue with floated items in connected lists.
…ermost container.
…ing / sorting with jQuery UI 1.9.1 and above currentItem might be floating which demands horizontal display hence distance should be calculated based on that. This is useful especially when the target container is initially empty, which makes its floating property false even after some floating items are dragged into it.
…. Fixes comment 5 of #9041: the over event fires on every pixel movement
|
Hi @zhizhangchen, thanks for contributing! These changes look good. The only thing I would say here is that we need to add tests to the test suite to prevent future regressions. These few lines of code have went back and forth a few times in the past. Are you comfortable doing so? The jsfiddles you provided should serve as a good basis. Also, have you signed our CLA: http://contribute.jquery.org/CLA/ ? Please do so if you haven't. |
|
OK, I'll try to write some tests. For the CLA, I checked with our legal 3 months ago and got a reply that he's working with Joel Kinney who was preparing an institutional agreement for companies. But after that, I haven't got any update. I'll check again. |
|
Thanks a lot! Really solid work you've done here. |
There was a problem hiding this comment.
Shouldn't this conditional come before the change triggers?
There was a problem hiding this comment.
In fact, this gaurd should move above line 873, right below the other gaurd clause.
|
I wrote the tests for you :-) |
|
Sory I'm terribly busy these days. I'll learn the tests after I make myself through this period. Thank you very much for all the coments and the tests! |
|
Sorry I'm terribly busy these days. I'll learn the tests after I make myself through this period. Thank you very much for all the comments and the tests! |
|
No problem! Thank you for the fixes here! |
|
I read the tests. It's quite interesting. I think next time I can try to write some tests along with code changes... Thank you! |
Fixed #8792:
http://jsfiddle.net/zhizhangchen/2HbZr/
http://jsfiddle.net/3JSd4/
Fixed #9041:
http://jsfiddle.net/A6QEn/
Fixed comment 5 of #9041
http://jsfiddle.net/bUFuK/1/
Regression tests:
http://jsfiddle.net/Y9Ned/1/
http://jsfiddle.net/YUqVL/2/
http://jsfiddle.net/3JdaT/1/