Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Sortable: Copy the cell structure when sorting a table row. Fixes #91…
…85 - Sortable: Placeholder breaks table-layout: fixed.

(cherry picked from commit 09b3533)
  • Loading branch information
scottgonzalez committed Apr 17, 2013
1 parent 83e0b4c commit 9711c54
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 18 deletions.
3 changes: 3 additions & 0 deletions tests/unit/sortable/sortable.html
Expand Up @@ -44,6 +44,9 @@
border-width: 0;
height:19px;
}
#sortable-table {
width: 100%;
}
</style>
</head>
<body>
Expand Down
38 changes: 25 additions & 13 deletions tests/unit/sortable/sortable_options.js
Expand Up @@ -359,19 +359,31 @@ test( "{ placeholder: String }", function() {
});

test( "{ placholder: String } tr", function() {
expect( 3 );

var element = $( "#sortable-table tbody" ).sortable({
placeholder: "test",
start: function( event, ui ) {
ok( ui.placeholder.hasClass( "test" ), "placeholder has class" );
equal( ui.placeholder.children().length, 1, "placeholder tr contains a td" );
equal( ui.placeholder.children().html(), $( "<span>&#160;</span>" ).html(),
"placeholder td has content for forced dimensions" );
}
});

element.find( "tr" ).eq( 0 ).simulate( "drag", {
expect( 4 );

var originalWidths,
element = $( "#sortable-table tbody" ).sortable({
placeholder: "test",
start: function( event, ui ) {
var currentWidths = otherRow.children().map(function() {
return $( this ).width();
}).get();
ok( ui.placeholder.hasClass( "test" ), "placeholder has class" );
deepEqual( currentWidths, originalWidths, "table cells maintian size" );
equal( ui.placeholder.children().length, dragRow.children().length,
"placeholder has correct number of cells" );
equal( ui.placeholder.children().html(), $( "<span>&#160;</span>" ).html(),
"placeholder td has content for forced dimensions" );
}
}),
rows = element.children( "tr" ),
dragRow = rows.eq( 0 ),
otherRow = rows.eq( 1 );

originalWidths = otherRow.children().map(function() {
return $( this ).width();
}).get();
dragRow.simulate( "drag", {
dy: 1
});
});
Expand Down
11 changes: 6 additions & 5 deletions ui/jquery.ui.sortable.js
Expand Up @@ -763,15 +763,16 @@ $.widget("ui.sortable", $.ui.mouse, {
element: function() {

var nodeName = that.currentItem[0].nodeName.toLowerCase(),
element = $( that.document[0].createElement( nodeName ) )
element = $( "<" + nodeName + ">", that.document[0] )
.addClass(className || that.currentItem[0].className+" ui-sortable-placeholder")
.removeClass("ui-sortable-helper");

if ( nodeName === "tr" ) {
// Use a high colspan to force the td to expand the full
// width of the table (browsers are smart enough to
// handle this properly)
element.append( "<td colspan='99'>&#160;</td>" );
that.currentItem.children().each(function() {

This comment has been minimized.

Copy link
@timvasil

timvasil Apr 22, 2014

This fails when there are non-visible cells. Suggestion: change .children() to .children().filter(':visible')

$( "<td>&#160;</td>", that.document[0] )
.attr( "colspan", $( this ).attr( "colspan" ) || 1 )
.appendTo( element );
});
} else if ( nodeName === "img" ) {
element.attr( "src", that.currentItem.attr( "src" ) );
}
Expand Down

0 comments on commit 9711c54

Please sign in to comment.