From a0f7729456ba9ba9b3d7cbd886c5a8c926ca0f1b Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Thu, 15 Aug 2013 15:59:39 +0100 Subject: [PATCH 1/5] 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/jquery.ui.sortable.js | 93 +++++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 35 deletions(-) diff --git a/ui/jquery.ui.sortable.js b/ui/jquery.ui.sortable.js index 9c7bf446cda..6142365b35b 100644 --- a/ui/jquery.ui.sortable.js +++ b/ui/jquery.ui.sortable.js @@ -284,7 +284,8 @@ $.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); @@ -342,47 +343,69 @@ $.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 form 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 - // beetween 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 form 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 + // beetween 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 6f24000c8a59d04673b24d548098f9756d45272a Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Mon, 21 Oct 2013 19:18:37 +0100 Subject: [PATCH 2/5] 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/jquery.ui.sortable.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ui/jquery.ui.sortable.js b/ui/jquery.ui.sortable.js index d80496de2d4..3385dfb1a83 100644 --- a/ui/jquery.ui.sortable.js +++ b/ui/jquery.ui.sortable.js @@ -345,12 +345,13 @@ $.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")) { touchingEdge = 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")) { touchingEdge = this.items.length - 1; this.direction = "up"; } From bd539d334d1abec472c4064c9ac26b06a5066678 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Mon, 21 Oct 2013 19:20:49 +0100 Subject: [PATCH 3/5] 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 8e0bac501f4..891918a7614 100644 --- a/tests/unit/sortable/sortable.html +++ b/tests/unit/sortable/sortable.html @@ -44,6 +44,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%; } @@ -66,6 +89,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 cad2c87a1fe005e5757254f89e99745b08062e9e Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Mon, 21 Oct 2013 19:33:04 +0100 Subject: [PATCH 4/5] 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 cfe95dbc12eade884354b3f7f13cdb5f78b24658 Mon Sep 17 00:00:00 2001 From: Michael Orchard Date: Tue, 22 Oct 2013 14:26:28 +0100 Subject: [PATCH 5/5] Sortable: changed same item at edge check from helper to placeholder. Fixed #5772 Sortable: containment parent restricting replacement of first and last elements --- ui/jquery.ui.sortable.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ui/jquery.ui.sortable.js b/ui/jquery.ui.sortable.js index 3385dfb1a83..0643eb19c3c 100644 --- a/ui/jquery.ui.sortable.js +++ b/ui/jquery.ui.sortable.js @@ -357,9 +357,8 @@ $.widget("ui.sortable", $.ui.mouse, { } } - if(touchingEdge !== undefined && this.helper[0] !== this.items[touchingEdge].item[0]) { - // Rearrange, if the helper is touching the edge of the containment and not - // already the item at the edge. + if(touchingEdge !== undefined && this.placeholder[0] !== this.items[touchingEdge].item[0]) { + // Rearrange if the helper is at the edge of the containment and the placeholder is not at that edge. this._rearrange(event, this.items[touchingEdge], false); this._trigger("change", event, this._uiHash()); } else {