From bd47bd4ace3789d9eb302b0dce6f6e042d08a7f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Scott=20Gonz=C3=A1lez?= Date: Wed, 20 Feb 2013 20:16:29 -0500 Subject: [PATCH] Sortable: When sorting table rows, create a td to force dimensions. Fixes #4765 - Sortable: Placeholder not displayed when sorting table rows. --- tests/unit/sortable/sortable.html | 21 +++++++++++++++ tests/unit/sortable/sortable_options.js | 34 +++++++++++++++++++++++-- ui/jquery.ui.sortable.js | 20 ++++++++++----- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/tests/unit/sortable/sortable.html b/tests/unit/sortable/sortable.html index c23a15854..6e326a865 100644 --- a/tests/unit/sortable/sortable.html +++ b/tests/unit/sortable/sortable.html @@ -63,6 +63,27 @@
  • Item 5
  • + + + + + + + + + + + + + + + + + + + +
    12
    34
    56
    78
    + diff --git a/tests/unit/sortable/sortable_options.js b/tests/unit/sortable/sortable_options.js index cf35aedb1..fe50be002 100644 --- a/tests/unit/sortable/sortable_options.js +++ b/tests/unit/sortable/sortable_options.js @@ -185,11 +185,41 @@ test("{ opacity: 1 }", function() { test("{ placeholder: false }, default", function() { ok(false, "missing test - untested code is broken code."); }); +*/ +test( "{ placeholder: String }", function() { + expect( 1 ); -test("{ placeholder: String }", function() { - ok(false, "missing test - untested code is broken code."); + var element = $( "#sortable" ).sortable({ + placeholder: "test", + start: function( event, ui ) { + ok( ui.placeholder.hasClass( "test" ), "placeholder has class" ); + } + }); + + element.find( "li" ).eq( 0 ).simulate( "drag", { + dy: 1 + }); }); +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(), $( " " ).html(), + "placeholder td has content for forced dimensions" ); + } + }); + + element.find( "tr" ).eq( 0 ).simulate( "drag", { + dy: 1 + }); +}); + +/* test("{ revert: false }, default", function() { ok(false, "missing test - untested code is broken code."); }); diff --git a/ui/jquery.ui.sortable.js b/ui/jquery.ui.sortable.js index c9b503bd0..f095ce9c5 100644 --- a/ui/jquery.ui.sortable.js +++ b/ui/jquery.ui.sortable.js @@ -752,15 +752,23 @@ $.widget("ui.sortable", $.ui.mouse, { o.placeholder = { element: function() { - var el = $(document.createElement(that.currentItem[0].nodeName)) - .addClass(className || that.currentItem[0].className+" ui-sortable-placeholder") - .removeClass("ui-sortable-helper")[0]; + var nodeName = that.currentItem[0].nodeName.toLowerCase(), + element = $( that.document[0].createElement( nodeName ) ) + .addClass(className || that.currentItem[0].className+" ui-sortable-placeholder") + .removeClass("ui-sortable-helper"); - if(!className) { - el.style.visibility = "hidden"; + 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( " " ); } - return el; + if ( !className ) { + element.css( "visibility", "hidden" ); + } + + return element; }, update: function(container, p) {