From 95546c5d045f8055b121f24d3e35468e2a570c1b Mon Sep 17 00:00:00 2001 From: Mike Sherov Date: Fri, 22 Aug 2014 15:16:41 -0400 Subject: [PATCH] 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 --- tests/unit/draggable/draggable.html | 40 +++++++++++++- tests/unit/draggable/draggable_options.js | 66 +++++++++++++++++++++++ ui/draggable.js | 62 +++++++++++---------- 3 files changed, 139 insertions(+), 29 deletions(-) diff --git a/tests/unit/draggable/draggable.html b/tests/unit/draggable/draggable.html index 8a19fc046..3b4db9921 100644 --- 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; + } @@ -60,7 +85,8 @@ "ui/mouse.js", "ui/resizable.js", "ui/draggable.js", - "ui/droppable.js" + "ui/droppable.js", + "ui/sortable.js" ] }); @@ -90,6 +116,18 @@
+ + diff --git a/tests/unit/draggable/draggable_options.js b/tests/unit/draggable/draggable_options.js index abce62a46..f724decbc 100644 --- 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 48832b34d..8cbcfe1ab 100644 --- 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