Skip to content

Commit d345a0d

Browse files
mikesherovmikesherov
committed
Draggable: allow draggable to defer destroying itself upon DOM removal until after stop is fired. Fixes #6889 - Draggable: Cursor doesn't revert to pre-dragging state after revert action when original element is removed.
1 parent dd58f98 commit d345a0d

File tree

5 files changed

+89
-42
lines changed

5 files changed

+89
-42
lines changed

‎tests/jquery.simulate.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,12 @@ $.extend( $.simulate.prototype, {
316316
this.simulateEvent( document, "mousemove", coord );
317317
}
318318

319-
this.simulateEvent( target, "mouseup", coord );
320-
this.simulateEvent( target, "click", coord );
319+
if ( $.contains( document, target ) ) {
320+
this.simulateEvent( target, "mouseup", coord );
321+
this.simulateEvent( target, "click", coord );
322+
} else {
323+
this.simulateEvent( document, "mouseup", coord );
324+
}
321325
}
322326
});
323327

‎tests/unit/draggable/draggable_core.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,13 @@ test( "resizable handle with complex markup (#8756 / #8757)", function() {
7575
});
7676

7777
test( "#8269: Removing draggable element on drop", function() {
78-
expect( 1 );
78+
expect( 2 );
7979

80-
var element = $( "#draggable1" ).wrap( "<div id='wrapper' />" ).draggable(),
80+
var element = $( "#draggable1" ).wrap( "<div id='wrapper' />" ).draggable({
81+
stop: function() {
82+
ok( true, "stop still called despite element being removed from DOM on drop" );
83+
}
84+
}),
8185
dropOffset = $( "#droppable" ).offset();
8286

8387
$( "#droppable" ).droppable({

‎tests/unit/draggable/draggable_options.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,33 @@ test( "{ cursor: 'move' }", function() {
451451
equal( after, before, "after drag: cursor restored" );
452452
});
453453

454+
test( "#6889: Cursor doesn't revert to pre-dragging state after revert action when original element is removed", function() {
455+
function getCursor() {
456+
return $( "body" ).css( "cursor" );
457+
}
458+
459+
expect( 2 );
460+
461+
var element = $( "#draggable1" ).wrap( "<div id='wrapper' />" ).draggable({
462+
cursor: "move",
463+
revert: true,
464+
revertDuration: 0,
465+
start: function() {
466+
notEqual( getCursor(), expected, "start callback: cursor '" + expected + "'" );
467+
$( "#wrapper" ).remove();
468+
},
469+
stop: function() {
470+
equal( getCursor(), expected, "after drag: cursor restored" );
471+
}
472+
}),
473+
expected = getCursor();
474+
475+
element.simulate( "drag", {
476+
dx: -1,
477+
dy: -1
478+
});
479+
});
480+
454481
test( "cursor, default, switching after initialization", function() {
455482
expect( 3 );
456483

‎ui/jquery.ui.core.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,15 @@ $.ui.plugin = {
276276
proto.plugins[ i ].push( [ option, set[ i ] ] );
277277
}
278278
},
279-
call: function( instance, name, args ) {
279+
call: function( instance, name, args, allowDisconnected ) {
280280
var i,
281281
set = instance.plugins[ name ];
282-
if ( !set || !instance.element[ 0 ].parentNode || instance.element[ 0 ].parentNode.nodeType === 11 ) {
282+
283+
if ( !set ) {
284+
return;
285+
}
286+
287+
if ( !allowDisconnected && ( !instance.element[ 0 ].parentNode || instance.element[ 0 ].parentNode.nodeType === 11 ) ) {
283288
return;
284289
}
285290

‎ui/jquery.ui.draggable.js

Lines changed: 43 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ $.widget("ui.draggable", $.ui.mouse, {
6666
},
6767

6868
_destroy: function() {
69+
if ( ( this.helper || this.element ).is( ".ui-draggable-dragging" ) ) {
70+
this.destroyOnClear = true;
71+
return;
72+
}
6973
this.element.removeClass( "ui-draggable ui-draggable-dragging ui-draggable-disabled" );
7074
this._mouseDestroy();
7175
},
@@ -233,11 +237,6 @@ $.widget("ui.draggable", $.ui.mouse, {
233237
this.dropped = false;
234238
}
235239

236-
//if the original element is no longer in the DOM don't bother to continue (see #8269)
237-
if ( this.options.helper === "original" && !$.contains( this.element[ 0 ].ownerDocument, this.element[ 0 ] ) ) {
238-
return false;
239-
}
240-
241240
if((this.options.revert === "invalid" && !dropped) || (this.options.revert === "valid" && dropped) || this.options.revert === true || ($.isFunction(this.options.revert) && this.options.revert.call(this.element, dropped))) {
242241
$(this.helper).animate(this.originalPosition, parseInt(this.options.revertDuration, 10), function() {
243242
if(that._trigger("stop", event) !== false) {
@@ -554,13 +553,16 @@ $.widget("ui.draggable", $.ui.mouse, {
554553
}
555554
this.helper = null;
556555
this.cancelHelperRemoval = false;
556+
if ( this.destroyOnClear ) {
557+
this.destroy();
558+
}
557559
},
558560

559561
// From now on bulk stuff - mainly helpers
560562

561563
_trigger: function(type, event, ui) {
562564
ui = ui || this._uiHash();
563-
$.ui.plugin.call(this, type, [event, ui]);
565+
$.ui.plugin.call( this, type, [ event, ui, this ], true );
564566
//The absolute position has to be recalculated after plugins
565567
if(type === "drag") {
566568
this.positionAbs = this._convertPositionTo("absolute");
@@ -582,9 +584,9 @@ $.widget("ui.draggable", $.ui.mouse, {
582584
});
583585

584586
$.ui.plugin.add("draggable", "connectToSortable", {
585-
start: function(event, ui) {
587+
start: function( event, ui, inst ) {
586588

587-
var inst = $(this).draggable( "instance" ), o = inst.options,
589+
var o = inst.options,
588590
uiSortable = $.extend({}, ui, { item: inst.element });
589591
inst.sortables = [];
590592
$(o.connectToSortable).each(function() {
@@ -600,11 +602,12 @@ $.ui.plugin.add("draggable", "connectToSortable", {
600602
});
601603

602604
},
603-
stop: function(event, ui) {
605+
stop: function( event, ui, inst ) {
604606

605607
//If we are still over the sortable, we fake the stop event of the sortable, but also remove helper
606-
var inst = $(this).draggable( "instance" ),
607-
uiSortable = $.extend({}, ui, { item: inst.element });
608+
var uiSortable = $.extend( {}, ui, {
609+
item: inst.element
610+
});
608611

609612
$.each(inst.sortables, function() {
610613
if(this.instance.isOver) {
@@ -637,9 +640,9 @@ $.ui.plugin.add("draggable", "connectToSortable", {
637640
});
638641

639642
},
640-
drag: function(event, ui) {
643+
drag: function( event, ui, inst ) {
641644

642-
var inst = $(this).draggable( "instance" ), that = this;
645+
var that = this;
643646

644647
$.each(inst.sortables, function() {
645648

@@ -739,47 +742,50 @@ $.ui.plugin.add("draggable", "connectToSortable", {
739742
});
740743

741744
$.ui.plugin.add("draggable", "cursor", {
742-
start: function() {
743-
var t = $("body"), o = $(this).draggable( "instance" ).options;
745+
start: function( event, ui, instance ) {
746+
var t = $( "body" ),
747+
o = instance.options;
748+
744749
if (t.css("cursor")) {
745750
o._cursor = t.css("cursor");
746751
}
747752
t.css("cursor", o.cursor);
748753
},
749-
stop: function() {
750-
var o = $(this).draggable( "instance" ).options;
754+
stop: function( event, ui, instance ) {
755+
var o = instance.options;
751756
if (o._cursor) {
752757
$("body").css("cursor", o._cursor);
753758
}
754759
}
755760
});
756761

757762
$.ui.plugin.add("draggable", "opacity", {
758-
start: function(event, ui) {
759-
var t = $(ui.helper), o = $(this).draggable( "instance" ).options;
763+
start: function( event, ui, instance ) {
764+
var t = $( ui.helper ),
765+
o = instance.options;
760766
if(t.css("opacity")) {
761767
o._opacity = t.css("opacity");
762768
}
763769
t.css("opacity", o.opacity);
764770
},
765-
stop: function(event, ui) {
766-
var o = $(this).draggable( "instance" ).options;
771+
stop: function( event, ui, instance ) {
772+
var o = instance.options;
767773
if(o._opacity) {
768774
$(ui.helper).css("opacity", o._opacity);
769775
}
770776
}
771777
});
772778

773779
$.ui.plugin.add("draggable", "scroll", {
774-
start: function() {
775-
var i = $(this).draggable( "instance" );
780+
start: function( event, ui, i ) {
776781
if(i.scrollParent[0] !== document && i.scrollParent[0].tagName !== "HTML") {
777782
i.overflowOffset = i.scrollParent.offset();
778783
}
779784
},
780-
drag: function( event ) {
785+
drag: function( event, ui, i ) {
781786

782-
var i = $(this).draggable( "instance" ), o = i.options, scrolled = false;
787+
var o = i.options,
788+
scrolled = false;
783789

784790
if(i.scrollParent[0] !== document && i.scrollParent[0].tagName !== "HTML") {
785791

@@ -827,10 +833,9 @@ $.ui.plugin.add("draggable", "scroll", {
827833
});
828834

829835
$.ui.plugin.add("draggable", "snap", {
830-
start: function() {
836+
start: function( event, ui, i ) {
831837

832-
var i = $(this).draggable( "instance" ),
833-
o = i.options;
838+
var o = i.options;
834839

835840
i.snapElements = [];
836841

@@ -847,10 +852,9 @@ $.ui.plugin.add("draggable", "snap", {
847852
});
848853

849854
},
850-
drag: function(event, ui) {
855+
drag: function( event, ui, inst ) {
851856

852857
var ts, bs, ls, rs, l, r, t, b, i, first,
853-
inst = $(this).draggable( "instance" ),
854858
o = inst.options,
855859
d = o.snapTolerance,
856860
x1 = ui.offset.left, x2 = x1 + inst.helperProportions.width,
@@ -922,9 +926,9 @@ $.ui.plugin.add("draggable", "snap", {
922926
});
923927

924928
$.ui.plugin.add("draggable", "stack", {
925-
start: function() {
929+
start: function( event, ui, instance ) {
926930
var min,
927-
o = $(this).draggable( "instance" ).options,
931+
o = instance.options,
928932
group = $.makeArray($(o.stack)).sort(function(a,b) {
929933
return (parseInt($(a).css("zIndex"),10) || 0) - (parseInt($(b).css("zIndex"),10) || 0);
930934
});
@@ -940,15 +944,18 @@ $.ui.plugin.add("draggable", "stack", {
940944
});
941945

942946
$.ui.plugin.add("draggable", "zIndex", {
943-
start: function(event, ui) {
944-
var t = $(ui.helper), o = $(this).draggable( "instance" ).options;
947+
start: function( event, ui, instance ) {
948+
var t = $( ui.helper ),
949+
o = instance.options;
950+
945951
if(t.css("zIndex")) {
946952
o._zIndex = t.css("zIndex");
947953
}
948954
t.css("zIndex", o.zIndex);
949955
},
950-
stop: function(event, ui) {
951-
var o = $(this).draggable( "instance" ).options;
956+
stop: function( event, ui, instance ) {
957+
var o = instance.options;
958+
952959
if(o._zIndex) {
953960
$(ui.helper).css("zIndex", o._zIndex);
954961
}

0 commit comments

Comments
 (0)