Skip to content
Permalink
Browse files
Sortable: Setting table row placeholder height to be same as sorted row
Fixes #13662
Closes gh-1578
  • Loading branch information
kevincupp authored and scottgonzalez committed Oct 4, 2016
1 parent b9d687d commit 87eab46a589031d781299937f95f22bf61b5ef27
Showing with 81 additions and 16 deletions.
  1. +46 −6 tests/unit/sortable/options.js
  2. +20 −1 tests/unit/sortable/sortable.html
  3. +15 −9 ui/widgets/sortable.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.");
});
@@ -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="">
@@ -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 ) -

0 comments on commit 87eab46

Please sign in to comment.