From d6b2a2bed3e27345e35f22834cab4fc178596ac8 Mon Sep 17 00:00:00 2001 From: Kevin Cupp Date: Wed, 15 Jul 2015 21:10:19 -0400 Subject: [PATCH] Sortable: Setting table row placeholder height to be same as sorted row Fixes #13662 --- tests/unit/sortable/options.js | 47 ++++++++++++++++++++++++++++--- tests/unit/sortable/sortable.html | 21 +++++++++++++- ui/sortable.js | 19 +++++++++---- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/tests/unit/sortable/options.js b/tests/unit/sortable/options.js index 930f339e677..57cec875cad 100644 --- a/tests/unit/sortable/options.js +++ b/tests/unit/sortable/options.js @@ -253,15 +253,54 @@ test("{ dropOnEmpty: true }, default", function() { test("{ dropOnEmpty: false }", function() { ok(false, "missing test - untested code is broken code."); }); +*/ -test("{ forcePlaceholderSize: false }, default", function() { - ok(false, "missing test - untested code is broken code."); +test("{ forcePlaceholderSize: false } table rows", function() { + expect( 1 ); + + var element = $( "#sortable-table2 tbody" ); + + element.sortable({ + placeholder: "test", + forcePlaceholderSize: false, + start: function( event, ui ) { + notEqual( ui.placeholder.height(), ui.item.height(), + "placeholder is same height as item" ); + } + }); + + // This row has a non-standard height + $("tr", element).eq( 0 ).simulate( "drag", { + dy: 1 + }); }); -test("{ forcePlaceholderSize: true }", function() { - ok(false, "missing test - untested code is broken code."); +test("{ forcePlaceholderSize: true } table rows", function() { + expect( 2 ); + + // Table should have the placeholder's height set the same as the row we're dragging + var element = $( "#sortable-table2 tbody" ); + + element.sortable({ + placeholder: "test", + forcePlaceholderSize: true, + start: function( event, ui ) { + equal( ui.placeholder.height(), ui.item.height(), + "placeholder is same height as item" ); + } + }); + + // First row has a non-standard height + $("tr", element).eq( 0 ).simulate( "drag", { + dy: 1 + }); + // Second row's height is normal + $("tr", element).eq( 1 ).simulate( "drag", { + dy: 1 + }); }); +/* test("{ forceHelperSize: false }, default", function() { ok(false, "missing test - untested code is broken code."); }); diff --git a/tests/unit/sortable/sortable.html b/tests/unit/sortable/sortable.html index 8bfbd337088..4ea5b0fc627 100644 --- a/tests/unit/sortable/sortable.html +++ b/tests/unit/sortable/sortable.html @@ -22,7 +22,8 @@ border-width: 0; height:19px; } - #sortable-table { + #sortable-table, + #sortable-table2 { width: 100%; } @@ -105,6 +106,24 @@ + + + + + + + + + + + + + + + + +
1
1		1
22
33
+
diff --git a/ui/sortable.js b/ui/sortable.js index bf1bf6a8925..4a1aa5e2396 100644 --- a/ui/sortable.js +++ b/ui/sortable.js @@ -785,15 +785,16 @@ return $.widget("ui.sortable", $.ui.mouse, { _createPlaceholder: function(that) { that = that || this; var className, + nodeName, o = that.options; if(!o.placeholder || o.placeholder.constructor === String) { className = o.placeholder; + nodeName = that.currentItem[0].nodeName.toLowerCase(); o.placeholder = { element: function() { - var nodeName = that.currentItem[0].nodeName.toLowerCase(), - element = $( "<" + nodeName + ">", that.document[0] ); + var element = $( "<" + nodeName + ">", that.document[0] ); that._addClass( element, "ui-sortable-placeholder", className || that.currentItem[ 0 ].className ) @@ -824,9 +825,17 @@ return $.widget("ui.sortable", $.ui.mouse, { return; } - //If the element doesn't have a actual height by itself (without styles coming from a stylesheet), it receives the inline height from the dragged item - if(!p.height()) { p.height(that.currentItem.innerHeight() - parseInt(that.currentItem.css("paddingTop")||0, 10) - parseInt(that.currentItem.css("paddingBottom")||0, 10)); } - if(!p.width()) { p.width(that.currentItem.innerWidth() - parseInt(that.currentItem.css("paddingLeft")||0, 10) - parseInt(that.currentItem.css("paddingRight")||0, 10)); } + // If the element doesn't have a actual height or width by itself (without styles coming from a stylesheet), + // it receives the inline height and width from the dragged item. Or, if it's a tbody or tr, it's going to have + // a height anyway since we're populating them with s above, but they're unlikely to be the correct height + // on their own if the row heights are dynamic, so we'll always assign the height of the dragged item given + // forcePlaceholderSize is true + if(!p.height() || (o.forcePlaceholderSize && (nodeName === "tbody" || nodeName === "tr"))) { + p.height(that.currentItem.innerHeight() - parseInt(that.currentItem.css("paddingTop")||0, 10) - parseInt(that.currentItem.css("paddingBottom")||0, 10)); + } + if(!p.width()) { + p.width(that.currentItem.innerWidth() - parseInt(that.currentItem.css("paddingLeft")||0, 10) - parseInt(that.currentItem.css("paddingRight")||0, 10)); + } } }; }