Draggable: No cloning in connectToSortable and ensure correct position

Draggables now forcefully recalculate their position when dragged out
of a sortable. Sortables now override draggable position when a
draggable is dragged into it. Lastly, no longer remove sortable helper
when dragging a draggable out, which allows us to not use a clone.

Fixes #7734
Fixes #8809
Closes gh-1322

diff --git a/tests/unit/draggable/draggable.html b/tests/unit/draggable/draggable.html
index 8a19fc0463115cca86173b58968fc52e725cc256..3b4db9921e54b2aae68b7888d044ec0e12de25e4 100644 (file)
--- a/tests/unit/draggable/draggable.html
+++ b/tests/unit/draggable/draggable.html
@@ -46,6 +46,31 @@
                #draggable3, #draggable4 {
                        z-index: 100;
                }
+               #sortable {
+                       position: relative;
+                       top: 8000px;
+                       left: 10px;
+               }
+               #sortable2 {
+                       position: relative;
+                       top: 9000px;
+                       left: 10px;
+               }
+               .sortable {
+                       width: 300px;
+                       height: 100px;
+                       padding: 0;
+                       margin: 0;
+                       border: 0;
+               }
+               .sortable li {
+                       height: 100px;
+                       padding: 0;
+                       margin: 0;
+                       border: 0;
+                       list-style: none;
+                       display: inline-block;
+               }
        </style>
 
        <script src="../../../external/qunit/qunit.js"></script>
@@ -60,7 +85,8 @@
                        "ui/mouse.js",
                        "ui/resizable.js",
                        "ui/draggable.js",
-                       "ui/droppable.js"
+                       "ui/droppable.js",
+                       "ui/sortable.js"
                ]
        });
        </script>
@@ -90,6 +116,18 @@
        </div>
        <div style="width: 1px; height: 1000px;"></div>
        <div style="position: absolute; width: 1px; height: 2000px;"></div>
+       <ul id="sortable" class="sortable">
+                   <li>Item 0</li>
+                   <li id="draggableSortable">Item 1</li>
+                   <li>Item 2</li>
+                   <li>Item 3</li>
+                 </ul>
+                 <ul id="sortable2" class="sortable">
+                   <li id="draggableSortableClone">Item 0</li>
+                   <li>Item 1</li>
+                   <li>Item 2</li>
+                   <li>Item 3</li>
+                 </ul>
 </div>
 
 </body>

diff --git a/tests/unit/draggable/draggable_options.js b/tests/unit/draggable/draggable_options.js
index abce62a462fdcb18d1ee406ac9ad390c54e171f9..f724decbcfb7ffc793ce7dd277f258c629176e0b 100644 (file)
--- a/tests/unit/draggable/draggable_options.js
+++ b/tests/unit/draggable/draggable_options.js
@@ -252,6 +252,72 @@ test( "{ connectToSortable: selector }, default", function() {
 });
 */
 
+test( "connectToSortable, dragging out of a sortable", function() {
+       expect( 3 );
+
+       var sortItem, dragHelper,
+               element = $( "#draggableSortable" ).draggable({
+                       scroll: false,
+                       connectToSortable: "#sortable"
+               }),
+               sortable = $( "#sortable" ).sortable(),
+               dx = 50,
+               dy = 50,
+               offsetBefore = element.offset(),
+               offsetExpected = {
+                       left: offsetBefore.left + dx,
+                       top: offsetBefore.top + dy
+               };
+
+       $( sortable ).one( "sortstart", function( event, ui ) {
+               sortItem = ui.item;
+       });
+
+       $( element ).one( "drag", function( event, ui ) {
+               dragHelper = ui.helper;
+       });
+
+       $( element ).one( "dragstop", function( event, ui ) {
+               // http://bugs.jqueryui.com/ticket/8809
+               // Position issue when connected to sortable
+               deepEqual( ui.helper.offset(), offsetExpected, "draggable offset is correct" );
+
+               // http://bugs.jqueryui.com/ticket/7734
+               // HTML IDs are removed when dragging to a Sortable
+               equal( sortItem[ 0 ], dragHelper[ 0 ], "both have the same helper" );
+               equal( sortItem.attr( "id" ), dragHelper.attr( "id" ), "both have the same id" );
+       });
+
+       element.simulate( "drag", {
+               dx: dx,
+               dy: dy
+       });
+});
+
+test( "connectToSortable, dragging clone into sortable", function() {
+       expect( 1 );
+
+       var element = $( "#draggableSortableClone" ).draggable({
+                       scroll: false,
+                       connectToSortable: "#sortable",
+                       helper: "clone"
+               }),
+               sortable = $( "#sortable" ).sortable(),
+               offsetSortable = sortable.offset();
+
+       $( sortable ).one( "sortbeforestop", function( event, ui ) {
+               // http://bugs.jqueryui.com/ticket/8809
+               // Position issue when connected to sortable
+               deepEqual( ui.helper.offset(), offsetSortable, "sortable offset is correct" );
+       });
+
+       element.simulate( "drag", {
+               x: offsetSortable.left + 1,
+               y: offsetSortable.top + 1,
+               moves: 1
+       });
+});
+
 test( "{ containment: Element }", function() {
        expect( 1 );
 

diff --git a/ui/draggable.js b/ui/draggable.js
index 48832b34d3bdcbe2241362b4466e4bed89e5874b..8cbcfe1ab59fa195d9a0f9868619fa7e84d8c1eb 100644 (file)
--- a/ui/draggable.js
+++ b/ui/draggable.js
@@ -177,23 +177,8 @@ $.widget("ui.draggable", $.ui.mouse, {
                this.offsetParentCssPosition = this.offsetParent.css( "position" );
 
                //The element's absolute position on the page minus margins
-               this.offset = this.positionAbs = this.element.offset();
-               this.offset = {
-                       top: this.offset.top - this.margins.top,
-                       left: this.offset.left - this.margins.left
-               };
-
-               //Reset scroll cache
-               this.offset.scroll = false;
-
-               $.extend(this.offset, {
-                       click: { //Where the click happened, relative to the element
-                               left: event.pageX - this.offset.left,
-                               top: event.pageY - this.offset.top
-                       },
-                       parent: this._getParentOffset(),
-                       relative: this._getRelativeOffset() //This is a relative to absolute position minus the actual position calculation - only used for relative positioned helper
-               });
+               this.positionAbs = this.element.offset();
+               this._refreshOffsets( event );
 
                //Generate the original position
                this.originalPosition = this.position = this._generatePosition( event, false );
@@ -234,6 +219,21 @@ $.widget("ui.draggable", $.ui.mouse, {
                return true;
        },
 
+       _refreshOffsets: function( event ) {
+               this.offset = {
+                       top: this.positionAbs.top - this.margins.top,
+                       left: this.positionAbs.left - this.margins.left,
+                       scroll: false,
+                       parent: this._getParentOffset(),
+                       relative: this._getRelativeOffset()
+               };
+
+               this.offset.click = {
+                       left: event.pageX - this.offset.left,
+                       top: event.pageY - this.offset.top
+               };
+       },
+
        _mouseDrag: function(event, noPropagation) {
                // reset any necessary cached properties (see #5009)
                if ( this.offsetParentCssPosition === "fixed" ) {
@@ -736,11 +736,13 @@ $.ui.plugin.add("draggable", "connectToSortable", {
 
                                this.instance.options.helper = this.instance.options._helper;
 
-                               //If the helper has been the original item, restore properties in the sortable
-                               if (inst.options.helper === "original") {
-                                       this.instance.currentItem.css({ top: "auto", left: "auto" });
-                               }
-
+                               // restore properties in the sortable, since the draggable may have
+                               // modified them in unexpected ways (#8809)
+                               this.instance.currentItem.css({
+                                       position: this.instance.placeholder.css( "position" ),
+                                       top: "",
+                                       left: ""
+                               });
                        } else {
                                this.instance.cancelHelperRemoval = false; //Remove the helper in the sortable instance
                                this.instance._trigger("deactivate", event, uiSortable);
@@ -750,8 +752,6 @@ $.ui.plugin.add("draggable", "connectToSortable", {
 
        },
        drag: function( event, ui, draggable ) {
-               var dragElement = this;
-
                $.each( draggable.sortables, function() {
                        var innermostIntersecting = false,
                                thisSortable = this,
@@ -787,9 +787,7 @@ $.ui.plugin.add("draggable", "connectToSortable", {
 
                                        sortable.isOver = 1;
 
-                                       sortable.currentItem = $( dragElement )
-                                               .clone()
-                                               .removeAttr( "id" )
+                                       sortable.currentItem = ui.helper
                                                .appendTo( sortable.element )
                                                .data( "ui-sortable-item", true );
 
@@ -827,6 +825,10 @@ $.ui.plugin.add("draggable", "connectToSortable", {
 
                                if ( sortable.currentItem ) {
                                        sortable._mouseDrag( event );
+                                       // Copy the sortable's position because the draggable's can potentially reflect
+                                       // a relative position, while sortable is always absolute, which the dragged
+                                       // element has now become. (#8809)
+                                       ui.position = sortable.position;
                                }
 
                        } else {
@@ -846,11 +848,15 @@ $.ui.plugin.add("draggable", "connectToSortable", {
                                        sortable._mouseStop( event, true );
                                        sortable.options.helper = sortable.options._helper;
 
-                                       sortable.currentItem.remove();
                                        if ( sortable.placeholder ) {
                                                sortable.placeholder.remove();
                                        }
 
+                                       // Recalculate the draggable's offset considering the sortable
+                                       // may have modified them in unexpected ways (#8809)
+                                       draggable._refreshOffsets( event );
+                                       ui.position = draggable._generatePosition( event, true );
+
                                        draggable._trigger( "fromSortable", event );
 
                                        // draggable revert needs that