From 6f91ac197b6853fdbcbd581bc5329009e6125065 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Thu, 15 Aug 2013 15:59:39 +0100 Subject: [PATCH 1/6] Sortable: added checks for the helper touching the edge of the containment while dragging. Fixed #5772 - Sortable: containment parent restricting replacement of first and last elements --- ui/sortable.js | 93 +++++++++++++++++++++++++++++++------------------- 1 file changed, 58 insertions(+), 35 deletions(-) diff --git a/ui/sortable.js b/ui/sortable.js index 09f8aa10e97..aa484488db3 100644 --- a/ui/sortable.js +++ b/ui/sortable.js @@ -316,7 +316,8 @@ return $.widget("ui.sortable", $.ui.mouse, { _mouseDrag: function(event) { var i, item, itemElement, intersection, o = this.options, - scrolled = false; + scrolled = false, + touchingContainmentEdge = false; //Compute the helpers position this.position = this._generatePosition(event); @@ -374,47 +375,69 @@ return $.widget("ui.sortable", $.ui.mouse, { this.helper[0].style.top = this.position.top+"px"; } - //Rearrange - for (i = this.items.length - 1; i >= 0; i--) { - - //Cache variables and intersection, continue if no intersection - item = this.items[i]; - itemElement = item.item[0]; - intersection = this._intersectsWithPointer(item); - if (!intersection) { - continue; + // Check if the helper is touching the edges of the containment. + if(this.containment) { + if(this.positionAbs.left === this.containment[0] && + this.positionAbs.top === this.containment[1]) { + touchingContainmentEdge = 0; + this.direction = "down"; } - - // Only put the placeholder inside the current Container, skip all - // items from other containers. This works because when moving - // an item from one container to another the - // currentContainer is switched before the placeholder is moved. - // - // Without this, moving items in "sub-sortables" can cause - // the placeholder to jitter between the outer and inner container. - if (item.instance !== this.currentContainer) { - continue; + else if(this.positionAbs.left === this.containment[2] && + this.positionAbs.top === this.containment[3]) { + touchingContainmentEdge = this.items.length - 1; + this.direction = "up"; } + } - // cannot intersect with itself - // no useless actions that have been done before - // no action if the item moved is the parent of the item checked - if (itemElement !== this.currentItem[0] && - this.placeholder[intersection === 1 ? "next" : "prev"]()[0] !== itemElement && - !$.contains(this.placeholder[0], itemElement) && - (this.options.type === "semi-dynamic" ? !$.contains(this.element[0], itemElement) : true) - ) { + if(touchingContainmentEdge !== false && + this.helper[0] !== this.items[touchingContainmentEdge].item[0]) { + // Rearrange, if the helper is touching the edge of the containment and not + // already the item at the edge. + this._rearrange(event, this.items[touchingContainmentEdge]); + this._trigger("change", event, this._uiHash()); + } else { + //Rearrange + for (i = this.items.length - 1; i >= 0; i--) { + + //Cache variables and intersection, continue if no intersection + item = this.items[i]; + itemElement = item.item[0]; + intersection = this._intersectsWithPointer(item); + if (!intersection) { + continue; + } - this.direction = intersection === 1 ? "down" : "up"; + // Only put the placeholder inside the current Container, skip all + // items from other containers. This works because when moving + // an item from one container to another the + // currentContainer is switched before the placeholder is moved. + // + // Without this, moving items in "sub-sortables" can cause + // the placeholder to jitter between the outer and inner container. + if (item.instance !== this.currentContainer) { + continue; + } - if (this.options.tolerance === "pointer" || this._intersectsWithSides(item)) { - this._rearrange(event, item); - } else { + // cannot intersect with itself + // no useless actions that have been done before + // no action if the item moved is the parent of the item checked + if (itemElement !== this.currentItem[0] && + this.placeholder[intersection === 1 ? "next" : "prev"]()[0] !== itemElement && + !$.contains(this.placeholder[0], itemElement) && + (this.options.type === "semi-dynamic" ? !$.contains(this.element[0], itemElement) : true) + ) { + + this.direction = intersection === 1 ? "down" : "up"; + + if (this.options.tolerance === "pointer" || this._intersectsWithSides(item)) { + this._rearrange(event, item); + } else { + break; + } + + this._trigger("change", event, this._uiHash()); break; } - - this._trigger("change", event, this._uiHash()); - break; } } From f4159805466c98e86e8a14c6591a6cecff3cc1e9 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Mon, 21 Oct 2013 19:18:37 +0100 Subject: [PATCH 2/6] Sortable: added axis checks to helper touching edge of containment while dragging. Fixed #5772 - Sortable: containment parent restricting replacement of first and last elements --- ui/sortable.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ui/sortable.js b/ui/sortable.js index aa484488db3..fba3189794d 100644 --- a/ui/sortable.js +++ b/ui/sortable.js @@ -377,13 +377,13 @@ return $.widget("ui.sortable", $.ui.mouse, { // Check if the helper is touching the edges of the containment. if(this.containment) { - if(this.positionAbs.left === this.containment[0] && - this.positionAbs.top === this.containment[1]) { + if((this.positionAbs.left === this.containment[0] || this.options.axis === "y") && + (this.positionAbs.top === this.containment[1] || this.options.axis === "x")) { touchingContainmentEdge = 0; this.direction = "down"; } - else if(this.positionAbs.left === this.containment[2] && - this.positionAbs.top === this.containment[3]) { + else if((this.positionAbs.left === this.containment[2] || this.options.axis === "y") && + (this.positionAbs.top === this.containment[3] || this.options.axis === "x")) { touchingContainmentEdge = this.items.length - 1; this.direction = "up"; } From 4c6300d9ec0c0d9a71a952708c76273663bc6c54 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Mon, 21 Oct 2013 19:20:49 +0100 Subject: [PATCH 3/6] Sortable Tests: added tests for sorting elements into first and last position --- tests/unit/sortable/sortable.html | 51 +++++++++++++++++++++ tests/unit/sortable/sortable_options.js | 59 ++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/tests/unit/sortable/sortable.html b/tests/unit/sortable/sortable.html index 3edc999b76e..30977ee9a3e 100644 --- a/tests/unit/sortable/sortable.html +++ b/tests/unit/sortable/sortable.html @@ -45,6 +45,29 @@ border-width: 0; height:19px; } + #sortable-horizontal { + width: 142px; + height: 22px; + } + #sortable-vertical { + width: 22px; + height: 142px; + } + #sortable-grid { + width: 62px; + height: 62px; + } + .sortable-axis span { + display: inline-block; + width: 20px; + height: 20px; + } + .sortable-axis span.wide { + width: 60px; + } + .sortable-axis span.tall { + height: 60px; + } #sortable-table { width: 100%; } @@ -71,6 +94,34 @@
  • Item 5
    • +
    + Item 1 + Item 2 + Item 3 + Item 4 + Item 5 +
    + +
    + Item 1 + Item 2 + Item 3 + Item 4 + Item 5 +
    + +
    + Item 1 + Item 2 + Item 3 + Item 4 + Item 5 + Item 6 + Item 7 + Item 8 + Item 9 +
    + diff --git a/tests/unit/sortable/sortable_options.js b/tests/unit/sortable/sortable_options.js index f2beb4dbcd6..52644994c5c 100644 --- a/tests/unit/sortable/sortable_options.js +++ b/tests/unit/sortable/sortable_options.js @@ -212,9 +212,66 @@ test("{ containment: 'window' }", function() { test("{ containment: Selector }", function() { ok(false, "missing test - untested code is broken code."); +});*/ + +test("#5772: wide element sorting to first and last position on x-axis", function() { + expect(2); + + var element = $("#sortable-horizontal").sortable({ + axis: "x", + containment: "parent", + scroll: false + }), + item = element.find(".wide").eq(0); + + item.simulate("drag", { dx: -150 }); + + equal(item.index(), 0, "Item is sorted to first position"); + + item.simulate("drag", { dx: 150 }); + + equal(item.index(), 4, "Item is sorted to last position"); }); -test("{ cursor: 'auto' }, default", function() { +test("#5772: tall element sorting to first and last position on y-axis", function() { + expect(2); + + var element = $("#sortable-vertical").sortable({ + axis: "y", + containment: "parent", + scroll: false + }), + item = element.find(".tall").eq(0); + + item.simulate("drag", { dy: -150 }); + + equal(item.index(), 0, "Item is sorted to first position"); + + item.simulate("drag", { dy: 150 }); + + equal(item.index(), 4, "Item is sorted to last position"); +}); + +test("#5772: element sorting to first and last position on grid", function() { + expect(2); + + var element = $("#sortable-grid").sortable({ + containment: "parent", + scroll: false + }), + item = element.find("span").eq(5); + + item.simulate("drag", { dy: -150, dx: -150 }); + + equal(item.index(), 0, "Item is sorted to first position"); + + item.simulate("drag", { dx: 150, dy: 150 }); + + equal(item.index(), 8, "Item is sorted to last position"); +}); + + +/*test("{ cursor: 'auto' }, default", function() { ok(false, "missing test - untested code is broken code."); }); From 43be872958be2325cb7a75467e2b16c14c6498d3 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Mon, 21 Oct 2013 19:33:04 +0100 Subject: [PATCH 4/6] Sortable Tests: added pointer tolerance tests for sorting elements into first and last position --- tests/unit/sortable/sortable_options.js | 73 +++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/tests/unit/sortable/sortable_options.js b/tests/unit/sortable/sortable_options.js index 52644994c5c..cec159a14cb 100644 --- a/tests/unit/sortable/sortable_options.js +++ b/tests/unit/sortable/sortable_options.js @@ -214,13 +214,14 @@ test("{ containment: Selector }", function() { ok(false, "missing test - untested code is broken code."); });*/ -test("#5772: wide element sorting to first and last position on x-axis", function() { +test("#5772: wide element intersect sorting to first and last position on x-axis", function() { expect(2); var element = $("#sortable-horizontal").sortable({ axis: "x", containment: "parent", - scroll: false + scroll: false, + tolerance: "intersect" }), item = element.find(".wide").eq(0); @@ -233,13 +234,34 @@ test("#5772: wide element sorting to first and last position on x-axis", functio equal(item.index(), 4, "Item is sorted to last position"); }); -test("#5772: tall element sorting to first and last position on y-axis", function() { +test("#5772: wide element pointer sorting to first and last position on x-axis", function() { + expect(2); + + var element = $("#sortable-horizontal").sortable({ + axis: "x", + containment: "parent", + scroll: false, + tolerance: "pointer" + }), + item = element.find(".wide").eq(0); + + item.simulate("drag", { dx: -150 }); + + equal(item.index(), 0, "Item is sorted to first position"); + + item.simulate("drag", { dx: 150 }); + + equal(item.index(), 4, "Item is sorted to last position"); +}); + +test("#5772: tall element intersect sorting to first and last position on y-axis", function() { expect(2); var element = $("#sortable-vertical").sortable({ axis: "y", containment: "parent", - scroll: false + scroll: false, + tolerance: "intersect" }), item = element.find(".tall").eq(0); @@ -252,12 +274,33 @@ test("#5772: tall element sorting to first and last position on y-axis", functio equal(item.index(), 4, "Item is sorted to last position"); }); -test("#5772: element sorting to first and last position on grid", function() { +test("#5772: tall element pointer sorting to first and last position on y-axis", function() { + expect(2); + + var element = $("#sortable-vertical").sortable({ + axis: "y", + containment: "parent", + scroll: false, + tolerance: "pointer" + }), + item = element.find(".tall").eq(0); + + item.simulate("drag", { dy: -150 }); + + equal(item.index(), 0, "Item is sorted to first position"); + + item.simulate("drag", { dy: 150 }); + + equal(item.index(), 4, "Item is sorted to last position"); +}); + +test("#5772: element intersect sorting to first and last position on grid", function() { expect(2); var element = $("#sortable-grid").sortable({ containment: "parent", - scroll: false + scroll: false, + tolerance: "intersect" }), item = element.find("span").eq(5); @@ -270,6 +313,24 @@ test("#5772: element sorting to first and last position on grid", function() { equal(item.index(), 8, "Item is sorted to last position"); }); +test("#5772: element pointer sorting to first and last position on grid", function() { + expect(2); + + var element = $("#sortable-grid").sortable({ + containment: "parent", + scroll: false, + tolerance: "pointer" + }), + item = element.find("span").eq(5); + + item.simulate("drag", { dy: -150, dx: -150 }); + + equal(item.index(), 0, "Item is sorted to first position"); + + item.simulate("drag", { dx: 150, dy: 150 }); + + equal(item.index(), 8, "Item is sorted to last position"); +}); /*test("{ cursor: 'auto' }, default", function() { ok(false, "missing test - untested code is broken code."); From d4288e615b7a40dcaf416f5168c6e0bbf834c3a9 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Tue, 22 Oct 2013 14:26:28 +0100 Subject: [PATCH 5/6] Sortable: changed same item at edge check from helper to placeholder. Fixed #5772 Sortable: containment parent restricting replacement of first and last elements --- ui/sortable.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ui/sortable.js b/ui/sortable.js index fba3189794d..dca2d8adb2c 100644 --- a/ui/sortable.js +++ b/ui/sortable.js @@ -390,9 +390,7 @@ return $.widget("ui.sortable", $.ui.mouse, { } if(touchingContainmentEdge !== false && - this.helper[0] !== this.items[touchingContainmentEdge].item[0]) { - // Rearrange, if the helper is touching the edge of the containment and not - // already the item at the edge. + this.placeholder[0] !== this.items[touchingContainmentEdge].item[0]) { this._rearrange(event, this.items[touchingContainmentEdge]); this._trigger("change", event, this._uiHash()); } else { From 1a8de8e7cf77952751600600c141810588f651e0 Mon Sep 17 00:00:00 2001 From: Luke Page Date: Fri, 6 Feb 2015 09:10:09 +0000 Subject: [PATCH 6/6] Sortable: Fix style violations --- tests/unit/sortable/sortable_options.js | 56 ++++++++++++------------- ui/sortable.js | 9 ++-- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/tests/unit/sortable/sortable_options.js b/tests/unit/sortable/sortable_options.js index cec159a14cb..6b96bf1e5eb 100644 --- a/tests/unit/sortable/sortable_options.js +++ b/tests/unit/sortable/sortable_options.js @@ -218,11 +218,11 @@ test("#5772: wide element intersect sorting to first and last position on x-axis expect(2); var element = $("#sortable-horizontal").sortable({ - axis: "x", - containment: "parent", - scroll: false, - tolerance: "intersect" - }), + axis: "x", + containment: "parent", + scroll: false, + tolerance: "intersect" + }), item = element.find(".wide").eq(0); item.simulate("drag", { dx: -150 }); @@ -238,11 +238,11 @@ test("#5772: wide element pointer sorting to first and last position on x-axis", expect(2); var element = $("#sortable-horizontal").sortable({ - axis: "x", - containment: "parent", - scroll: false, - tolerance: "pointer" - }), + axis: "x", + containment: "parent", + scroll: false, + tolerance: "pointer" + }), item = element.find(".wide").eq(0); item.simulate("drag", { dx: -150 }); @@ -258,11 +258,11 @@ test("#5772: tall element intersect sorting to first and last position on y-axis expect(2); var element = $("#sortable-vertical").sortable({ - axis: "y", - containment: "parent", - scroll: false, - tolerance: "intersect" - }), + axis: "y", + containment: "parent", + scroll: false, + tolerance: "intersect" + }), item = element.find(".tall").eq(0); item.simulate("drag", { dy: -150 }); @@ -278,11 +278,11 @@ test("#5772: tall element pointer sorting to first and last position on y-axis", expect(2); var element = $("#sortable-vertical").sortable({ - axis: "y", - containment: "parent", - scroll: false, - tolerance: "pointer" - }), + axis: "y", + containment: "parent", + scroll: false, + tolerance: "pointer" + }), item = element.find(".tall").eq(0); item.simulate("drag", { dy: -150 }); @@ -298,10 +298,10 @@ test("#5772: element intersect sorting to first and last position on grid", func expect(2); var element = $("#sortable-grid").sortable({ - containment: "parent", - scroll: false, - tolerance: "intersect" - }), + containment: "parent", + scroll: false, + tolerance: "intersect" + }), item = element.find("span").eq(5); item.simulate("drag", { dy: -150, dx: -150 }); @@ -317,10 +317,10 @@ test("#5772: element pointer sorting to first and last position on grid", functi expect(2); var element = $("#sortable-grid").sortable({ - containment: "parent", - scroll: false, - tolerance: "pointer" - }), + containment: "parent", + scroll: false, + tolerance: "pointer" + }), item = element.find("span").eq(5); item.simulate("drag", { dy: -150, dx: -150 }); diff --git a/ui/sortable.js b/ui/sortable.js index dca2d8adb2c..fd1151c4b91 100644 --- a/ui/sortable.js +++ b/ui/sortable.js @@ -288,7 +288,6 @@ return $.widget("ui.sortable", $.ui.mouse, { this._cacheHelperProportions(); } - //Post "activate" events to possible containers if( !noActivation ) { for ( i = this.containers.length - 1; i >= 0; i-- ) { @@ -394,6 +393,7 @@ return $.widget("ui.sortable", $.ui.mouse, { this._rearrange(event, this.items[touchingContainmentEdge]); this._trigger("change", event, this._uiHash()); } else { + //Rearrange for (i = this.items.length - 1; i >= 0; i--) { @@ -420,9 +420,9 @@ return $.widget("ui.sortable", $.ui.mouse, { // no useless actions that have been done before // no action if the item moved is the parent of the item checked if (itemElement !== this.currentItem[0] && - this.placeholder[intersection === 1 ? "next" : "prev"]()[0] !== itemElement && - !$.contains(this.placeholder[0], itemElement) && - (this.options.type === "semi-dynamic" ? !$.contains(this.element[0], itemElement) : true) + this.placeholder[intersection === 1 ? "next" : "prev"]()[0] !== itemElement && + !$.contains(this.placeholder[0], itemElement) && + (this.options.type === "semi-dynamic" ? !$.contains(this.element[0], itemElement) : true) ) { this.direction = intersection === 1 ? "down" : "up"; @@ -514,6 +514,7 @@ return $.widget("ui.sortable", $.ui.mouse, { } if (this.placeholder) { + //$(this.placeholder[0]).remove(); would have been the jQuery way - unfortunately, it unbinds ALL events from the original node! if(this.placeholder[0].parentNode) { this.placeholder[0].parentNode.removeChild(this.placeholder[0]);