Sortable: Setting table row placeholder height to be same as sorted row

Fixes #13662
Closes gh-1578

diff --git a/tests/unit/sortable/options.js b/tests/unit/sortable/options.js
index fa0c487b720c06c978dce1eac7fea7aaf57c1d51..30929a501dca3c8efbd8056935a8e2f376373981 100644 (file)
--- a/tests/unit/sortable/options.js
+++ b/tests/unit/sortable/options.js
@@ -255,15 +255,55 @@ 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.");
-});
+QUnit.test( "{ forcePlaceholderSize: false } table rows", function( assert ) {
+       assert.expect( 1 );
 
-test("{ forcePlaceholderSize: true }", function() {
-       ok(false, "missing test - untested code is broken code.");
-});
+       var element = $( "#sortable-table2 tbody" );
+
+       element.sortable( {
+               placeholder: "test",
+               forcePlaceholderSize: false,
+               start: function( event, ui ) {
+                       assert.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
+       } );
+} );
 
+QUnit.test( "{ forcePlaceholderSize: true } table rows", function( assert ) {
+       assert.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 ) {
+                       assert.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 8bfbd337088b25d037e4977db2c3059a054fbc25..4ea5b0fc62703e1fa37d0ae4573938fe5f2a3928 100644 (file)
--- 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%;
        }
        </style>
@@ -105,6 +106,24 @@
        </tbody>
 </table>
 
+<!-- This table has rows of varying height -->
+<table id="sortable-table2">
+       <tbody>
+               <tr>
+                       <td>1<br>1</td>
+                       <td>1</td>
+               </tr>
+               <tr>
+                       <td>2</td>
+                       <td>2</td>
+               </tr>
+               <tr>
+                       <td>3</td>
+                       <td>3</td>
+               </tr>
+       </tbody>
+</table>
+
 <div id="sortable-images">
        <img src="../../images/jqueryui_32x32.png" alt="">
        <img src="../../images/jqueryui_32x32.png" alt="">

diff --git a/ui/widgets/sortable.js b/ui/widgets/sortable.js
index f7298ebe53f504e27d9c79183913b5db90f65d1b..a953501512c8cfb095305d322088a35e18e05ce7 100644 (file)
--- a/ui/widgets/sortable.js
+++ b/ui/widgets/sortable.js
@@ -875,20 +875,20 @@ return $.widget( "ui.sortable", $.ui.mouse, {
 
        _createPlaceholder: function( that ) {
                that = that || this;
-               var className,
+               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 )
-                                                       ._removeClass( element, "ui-sortable-helper" );
+                                       that._addClass( element, "ui-sortable-placeholder",
+                                                       className || that.currentItem[ 0 ].className )
+                                               ._removeClass( element, "ui-sortable-helper" );
 
                                        if ( nodeName === "tbody" ) {
                                                that._createTrPlaceholder(
@@ -917,9 +917,15 @@ 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() ) {
+                                       // 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 <td>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 ) -