Skip to content

Sortable: Allow sortover to trigger even if current container not change... #1038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
matt-hoskins wants to merge 3 commits into from
Closed

Sortable: Allow sortover to trigger even if current container not change... #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
885b90e
Sortable: Allow sortover to trigger even if current container not cha…
matt-hoskins Jul 31, 2013
4e17aad
Sortable Tests: Cover out event and out/over sequences involving conn…
matt-hoskins Aug 27, 2013
0365017
Sortable Tests: Fix lint errors in previous commit for additional tes…
matt-hoskins Jan 28, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions tests/unit/sortable/sortable.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"ui/jquery.ui.core.js",
"ui/jquery.ui.widget.js",
"ui/jquery.ui.mouse.js",
"ui/jquery.ui.draggable.js",
"ui/jquery.ui.sortable.js"
]
});
Expand Down Expand Up @@ -47,6 +48,25 @@
#sortable-table {
width: 100%;
}
ul.sortableoutover, ul.sortableoutoverdraggable {
position:relative;
top:0;
left:0;
}
ul.sortableoutover, ul.sortableoutover li, ul.sortableoutoverdraggable, ul.sortableoutoverdraggable li {
margin: 0;
padding: 0;
border-width: 0;
}
ul.sortableoutover li, ul.sortableoutoverdraggable li {
height: 20px;
}
#sortableoutoverspace {
height: 100px;
}
#sortableoutoverdraggablelist {
height: 20px;
}
</style>
</head>
<body>
Expand Down Expand Up @@ -94,6 +114,30 @@ <h2 id="qunit-userAgent"></h2>
<img src="../images/jqueryui_32x32.png" alt="">
</div>

<ul id="sortableoutover" class="sortableoutover">
<li>Item 1</li>
<li>Item 2</li>
<li>Item 3</li>
<li>Item 4</li>
<li>Item 5</li>
</ul>

<div id="sortableoutoverspace" class="sortableoutoverspace">
Space
</div>

<ul id="sortableoutoverdraggablelist" class="sortableoutoverdraggable">
<li id="sortableoutoverdraggable">Item 1</li>
</ul>

<ul id="sortableoutover2" class="sortableoutover">
<li>Item 1</li>
<li>Item 2</li>
<li>Item 3</li>
<li>Item 4</li>
<li>Item 5</li>
</ul>

</div>
</body>
</html>
176 changes: 171 additions & 5 deletions tests/unit/sortable/sortable_events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,21 +248,187 @@ test( "over", function() {
dy: 20
});

ok( hash, "stop event triggered" );
ok( hash, "over event triggered" );
ok( hash.helper, "UI includes: helper" );
ok( hash.placeholder, "UI hash includes: placeholder" );
ok( hash.position && ( "top" in hash.position && "left" in hash.position ), "UI hash includes: position" );
ok( hash.offset && ( hash.offset.top && hash.offset.left ), "UI hash includes: offset" );
ok( hash.item, "UI hash includes: item" );
ok( hash.sender, "UI hash includes: sender" );
equal( overCount, 1, "over fires only once" );
});

test( "out", function() {
expect( 8 );

var hash,
outCount = 0;

$("#sortableoutover").sortable({
out: function( e, ui ) {
hash = ui;
outCount++;
}
}).find( "li:eq(0)" ).simulate( "drag", {
dy: 150
});

ok( hash, "out event triggered" );
ok( hash.helper, "UI should not include: helper" );
ok( hash.placeholder, "UI hash includes: placeholder" );
ok( hash.position && ( "top" in hash.position && "left" in hash.position ), "UI hash includes: position" );
ok( hash.offset && ( hash.offset.top && hash.offset.left ), "UI hash includes: offset" );
ok( hash.item, "UI hash includes: item" );
ok( hash.sender, "UI hash does not include: sender" );
equal( overCount, 1, "over fires only once" );
equal( outCount, 1, "out fires only once" );
});

/*
test("out", function() {
ok(false, "missing test - untested code is broken code.");
test("#9335: over and out firing, including with connectWith and a draggable", function() {
expect(24);

/*
* The fixture structure is:
* - a UL with 5 * LI of 20px height each (total height 100px) to be made sortable
* - a DIV of height 100px
* - a UL with 1 * LI of 20px height (total height fixed at 20px) with the LI to be made draggable
* - a UL with 5 * LI of 20px height each (total height 100px) to be made sortable
*/

var fired = {},
firedCount = function (id,type) { var key=id + "-" + type; return (key in fired) ? fired[key] : 0; },
fire = function (sortable,type) { var key=$(sortable).attr("id") + "-" + type; fired[key] = ((key in fired) ? fired[key] : 0) + 1; },
sortable_ul=$("#sortableoutover"),
drag_el = sortable_ul.find( "li:eq(0)" ),
draggable_container=$("#sortableoutoverdraggablelist"),
draggable=draggable_container.find("li:eq(0)");

// Initialise with hooks to count how often out and over events fire
$("#qunit-fixture ul.sortableoutover").sortable({
connectWith: "#qunit-fixture ul.sortableoutover",
out: function () {
fire(this,"out");
},
over: function () {
fire(this,"over");
}
});
draggable.draggable({
connectToSortable: "#qunit-fixture ul.sortableoutover",
revert:"invalid"
});

// Test that after dragging out (but keeping the mouse down)
// registers an initial over and then an out against the main sortable
fired = {};
TestHelpers.sortable.dragBegin(drag_el, {
dy: 150
});
equal( firedCount( "sortableoutover", "over" ), 1, "Drag outside sortable fires over once initially" );
equal( firedCount( "sortableoutover", "out" ), 1, "Drag outside sortable fires out once" );

// Release the mouse button while not over the sortable triggers no 'out' event
fired = {};
TestHelpers.sortable.dragEnd(drag_el, { });
equal( firedCount( "sortableoutover", "over" ), 0, "Completion of drag outside sortable fires no over" );
equal( firedCount( "sortableoutover", "out" ), 0, "Completion of drag outside sortable fires no out" );

// NB: because the above drag may well have resulted in the drag element being repositioned in the
// list we get the current first element again
drag_el = sortable_ul.find( "li:eq(0)" );

// Test that dragging out and then back over fires an initial over, the out, and then another over
fired = {};
TestHelpers.sortable.dragBegin(drag_el, {
dy: 150
});
TestHelpers.sortable.dragContinue(drag_el, {
dy: -150
});
equal( firedCount( "sortableoutover", "over" ), 2, "Drag outside and then back over sortable fires over once initially and then a second on return" );
equal( firedCount( "sortableoutover", "out" ), 1, "Drag outside and then back over sortable fires out once" );

// Releasing the mouse button while 'over' triggers an 'out'
fired = {};
TestHelpers.sortable.dragEnd(drag_el, { });
equal( firedCount( "sortableoutover", "over" ), 0, "Releasing the mouse button after a drag out and then back over triggers no further over" );
equal( firedCount( "sortableoutover", "out" ), 1, "Releasing the mouse button after a drag out and then back over triggers a final out" );

// Test that dragging out and then over second sortable fires initial over and out on the first
// and then an over on the second
fired = {};
TestHelpers.sortable.dragBegin(drag_el, {
dy: 150
});
TestHelpers.sortable.dragContinue(drag_el, {
dy: 100
});
equal( firedCount( "sortableoutover", "over" ), 1, "Drag outside first and then over second sortable fires over on first sortable" );
equal( firedCount( "sortableoutover", "out" ), 1, "Drag outside first and then over second sortable fires out on first sortable" );
equal( firedCount( "sortableoutover2", "over" ), 1, "Drag outside first and then over second sortable fires over on second sortable" );
equal( firedCount( "sortableoutover2", "out" ), 0, "Drag outside first and then over second sortable fires no out on second sortable" );

// Releasing the mouse button while 'over' triggers an 'out' - this time it should be on the second sortable, not the first
fired = {};
TestHelpers.sortable.dragEnd(drag_el, { });
equal( firedCount( "sortableoutover", "out" ), 0, "Releasing the mouse button after a drag out and over the second sortable shouldn't trigger an out on the first" );
equal( firedCount( "sortableoutover2", "out" ), 1, "Releasing the mouse button after a drag out and over the second sortable should trigger an out on the second" );

// Dragging draggable over one sorted list and then out and then over another sorted list and then out
// should trigger one over and one out on each
fired = {};
TestHelpers.sortable.dragBegin(draggable, {
dy: 70 // Over second sortable
});
TestHelpers.sortable.dragContinue(draggable, {
dy: -120 // Over space div
});
TestHelpers.sortable.dragContinue(draggable, {
dy: -100 // Over first sortable
});
TestHelpers.sortable.dragContinue(draggable, {
dy: 100 // Over space div
});
equal( firedCount( "sortableoutover", "over" ), 1, "Dragging over both sortables and then back out should trigger over on first" );
equal( firedCount( "sortableoutover", "out" ), 1, "Dragging over both sortables and then back out should trigger out on first" );
equal( firedCount( "sortableoutover2", "over" ), 1, "Dragging over both sortables and then back out should trigger over on second" );
equal( firedCount( "sortableoutover2", "out" ), 1, "Dragging over both sortables and then back out should trigger out on second" );

// Release the mouse button while outside both sortables shouldn't trigger any further out events
fired = {};
TestHelpers.sortable.dragEnd(draggable, { });
equal( firedCount( "sortableoutover", "out" ), 0, "Dragging over both sortables and then back out shouldn't trigger any further out event on first on mouseup" );
equal( firedCount( "sortableoutover2", "out" ), 0, "Dragging over both sortables and then back out shouldn't trigger any further out event on second on mouseup" );

// At the time of writing this test if you drop a draggable outside of a sortable but it was
// over a sortable at some point during the drag then it ends up in the sortable (i.e. it doesn't revert), so reset
// the draggable back to being in its container before the next test
draggable
.draggable("destroy")
.appendTo(draggable_container)
.draggable({
connectToSortable: "#qunit-fixture ul.sortableoutover",
revert:"invalid"
});

// Test dragging draggable over connected sortable
// registers an initial over and then an out against the main sortable
fired = {};
TestHelpers.sortable.dragBegin(draggable, {
dy: 50
});
equal( firedCount( "sortableoutover2", "over" ), 1, "Dragging draggable over sortable should trigger over" );
equal( firedCount( "sortableoutover2", "out" ), 0, "Dragging draggable over sortable shouldn't trigger out until mouseup" );

// Release the mouse button while over the sortable should trigger out
// NB: this will have dropped drag_el on to this second list
fired = {};
TestHelpers.sortable.dragEnd(draggable, { });
equal( firedCount( "sortableoutover2", "over" ), 0, "Completion of drag of draggable over sortable shouldn't trigger over" );
equal( firedCount( "sortableoutover2", "out" ), 1, "Completion of drag of draggable over sortable should trigger out" );

});

/*
test("activate", function() {
ok(false, "missing test - untested code is broken code.");
});
Expand Down
61 changes: 61 additions & 0 deletions tests/unit/sortable/sortable_test_helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,66 @@ TestHelpers.sortable = {
dy: dy
});
equal( $( handle ).parent().children().index( handle ), index, msg );
},
findCenter: function ( elem ) {
var offset,
document = $( elem[0].ownerDocument );
offset = elem.offset();

return {
x: offset.left + elem.outerWidth() / 2 - document.scrollLeft(),
y: offset.top + elem.outerHeight() / 2 - document.scrollTop()
};
},
drag: function( target, options ) {
// Adapted from the jquery simulate plugin - a version that can avoid doing mouseup or mouse down is useful
// - don't currently need all the functionality from there, but it seemed pointless to pare it down
// in case it does become useful later.

target = $( target );
var i = 0,
center = TestHelpers.sortable.findCenter( target ),
x = Math.floor( center.x ),
y = Math.floor( center.y ),
coord = { clientX: x, clientY: y },
dx = options.dx || ( options.x !== undefined ? options.x - x : 0 ),
dy = options.dy || ( options.y !== undefined ? options.y - y : 0 ),
moves = options.moves || 3,
nomousedown = options.nomousedown || false,
nomouseup = options.nomouseup || false;

if ( !nomousedown ) {
target.simulate( "mousedown", coord );
}

for ( ; i < moves ; i++ ) {
x += dx / moves;
y += dy / moves;

coord = {
clientX: Math.round( x ),
clientY: Math.round( y )
};

$( target[0].ownerDocument ).simulate( "mousemove", coord );
}

if ( !nomouseup ) {
if ( $.contains( document, target[0] ) ) {
target.simulate( "mouseup", coord );
target.simulate( "click", coord );
} else {
$( document ).simulate( "mouseup", coord );
}
}
},
dragBegin: function ( target, options ) {
TestHelpers.sortable.drag( target, $.extend( {} , options, { "nomouseup":true } ) );
},
dragContinue: function ( target, options ) {
TestHelpers.sortable.drag( target, $.extend( {} , options, { "nomouseup":true, "nomousedown":true } ) );
},
dragEnd: function ( target, options ) {
TestHelpers.sortable.drag( target, $.extend( {} , options, { "nomousedown":true } ) );
}
};
25 changes: 13 additions & 12 deletions ui/jquery.ui.sortable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,20 +878,21 @@ $.widget("ui.sortable", $.ui.mouse, {
return;
}

if(this.currentContainer === this.containers[innermostIndex]) {
return;
}

itemWithLeastDistance ? this._rearrange(event, itemWithLeastDistance, null, true) : this._rearrange(event, null, this.containers[innermostIndex].element, true);
this._trigger("change", event, this._uiHash());
this.containers[innermostIndex]._trigger("change", event, this._uiHash(this));
this.currentContainer = this.containers[innermostIndex];
if(this.currentContainer !== this.containers[innermostIndex]) {
itemWithLeastDistance ? this._rearrange(event, itemWithLeastDistance, null, true) : this._rearrange(event, null, this.containers[innermostIndex].element, true);
this._trigger("change", event, this._uiHash());
this.containers[innermostIndex]._trigger("change", event, this._uiHash(this));
this.currentContainer = this.containers[innermostIndex];

//Update the placeholder
this.options.placeholder.update(this.currentContainer, this.placeholder);
//Update the placeholder
this.options.placeholder.update(this.currentContainer, this.placeholder);
}


this.containers[innermostIndex]._trigger("over", event, this._uiHash(this));
this.containers[innermostIndex].containerCache.over = 1;
if (!this.containers[innermostIndex].containerCache.over) {
this.containers[innermostIndex]._trigger("over", event, this._uiHash(this));
this.containers[innermostIndex].containerCache.over = 1;
}
}


Expand Down