TableDnD icon indicating copy to clipboard operation
TableDnD copied to clipboard

onDrop handler not called

Open meszaros-lajos-gyorgy opened this issue 12 years ago • 11 comments

In the mouseup function on line 357 in the newest version the jQuery.tableDnD.originalOrder newOrder always have the same value, so something is not correct in jQuery.tableDnD.serialize().

If I comment out that check, the code works fine.

meszaros-lajos-gyorgy avatar Sep 20 '12 14:09 meszaros-lajos-gyorgy

There were several problems with the event handling firing multiple times as well as several inefficient processing of the data to determine state etc.

All of these have been addressed. and the examples are also all updated and we can verify the functionality working in that way. If you are available to help us test today that would be a life saver.

Let us know...

nickl- avatar Oct 21 '12 04:10 nickl-

You must define tr[id] attribute to make onDrop work. This is because onDrop only fire when row order changed. However, without specifying tr[id] attribute, tableDnD.serialize() will think there was not any re-order. (Bug for sure)

If you read the example in TableDnD website more carefully, you will notice that the author has ID attributes defined in every TR.

henrygotmojo avatar Jan 08 '13 06:01 henrygotmojo

I don't know if this is a bug, unless you just want to move rows around the table for kicks I guess. Without the id there is no why to distinguish the row from any other row, nothing for serialize to collect, nothing to send back to the server and hence no way to change the order on the back end so after page refresh we will have the previous order again.

@metaseit Unless you can think of a better way to uniquely identify each row id makes sense because it at least has the unique constraint, looking at cell 0 or a data attribute has no checks to ensure they need to be unique and we will then be unable to distinguish one row from another. The id attribute an row also helps to find the row which will be more difficult using the value of the first td for example.

Thoughts?.

nickl- avatar Jan 08 '13 16:01 nickl-

@nickl To make it more developer-friendly, I think the plugin could add an custom attribute to TR (e.g. data-tablednd-unique-id) when init. Therefore, the plugin could have something unique to distinguish the row, but without having the developer to define ID explicitly.

henrygotmojo avatar Jan 08 '13 17:01 henrygotmojo

Hey guys, I keep getting notifications (and subsequently have to unwatch the thread) since you're using @nickl instead of @nickl- (note the extra dash). If you can please use the correct mention. Thanks!

nickL avatar Jan 08 '13 18:01 nickL

@nickl and that is probably why I miss so many notices, was wondering what's up with that. Only if you can find a way to automate it as I'd hate to make it a burdon on you to ask that you please forward all these notices to me if you don't mind. Since its all public anyway what is the harm in forwarding posts meant for you, and since you're already getting posts meant for me, at least now I get the same amount of "mailbombing" you have to endure =) What do you say?

It's a fairly common mistake I'm sure and I doubt we are going to solve anything trying to educate the masses, problem you get unwanted mail, I mis out because I'm not notified when I should've been. Is there anyway we can reach a middle ground here? I'm open to suggestions...

nickl- avatar Jan 08 '13 20:01 nickl-

@metaseit as I mentioned the problem is that we loose unique constraint.

Although not a hard rule, the browser would not stop rendering the content, but most editors would clearly show that there's a problem if you had:

<tr id="two-of-the-same">
   <td>1</td>
   <td>first row</td>
</tr>
<tr id="two-of-the-same">
   <td>2</td>
   <td>second row</td>
</tr>

We could substitute the use of id by either a data tag or assume the key is in the first cell of the row but that makes both of these perfectly legal and nothing checking the constraint.

<tr data-tablednd-id="two-of-the-same">
   <td>1</td>
   <td>first row</td>
</tr>
<tr data-tablednd-id="two-of-the-same">
   <td>1</td>
   <td>another first row</td>
</tr>

There is also an increased complexity to select these rows;

Instead of simply using id

$("#some-unique-id");

We have to dig deeper to get the row based on identifier in data

$('tr[data-tablednd-id=some-unique-id]');

And even further if we wanted a row with cel containing the id

$('tr:has(td:parent:contains(some-unique-id))');

Nasty perhaps but do-able none the less.

Suggestion:

What we could perhaps do is generate an original row position and use that as a unique id, which we can use as the the id if none were present. A beneficial side effect is the ability to reset the table since we know the original order now :wink:

Say for example you supplied this

<tr>
   <td>1</td>
   <td>first row</td>
</tr>
<tr id="has-one">
   <td>2</td>
   <td>second row</td>
</tr>
<tr>
   <td>3</td>
   <td>third row</td>
</tr>

With something like this:

var row = 0;
$('tr').each(function () {
   $(this).data('order',++row);
   if (!$(this).attr('id')) $(this).attr('id', 'was-at-'+$(this).data('order'));
});

We ensure that everything has an id, solve the problem that serialize has nothing to supply you with, the data is useful as it gives you an indication where the row was at so at least you have a starting point if you wanted to do something with the data other in the end.

$('#was-at-1');
$('#has-one');
$('#was-at-2');

Is this what you're after?

nickl- avatar Jan 08 '13 21:01 nickl-

if i get future notices I'll try and forward over. For everyone else remember @nickl- NOT @nickl . Btw sweet name though Nick L. ;) Ok. Unwatching. Happy coding fellas.

nickL avatar Jan 08 '13 21:01 nickL

@nickl LIke your name too :grinning: As I say an across the board filter/ (gmail talk) that forwards all github notifications my way would be fine by me, I'd rather sift through the cruft and you would save me stacks from missing something because they chose @nickl instead of @nickl- and saves you the trouble of having to forward it everytime as it happens automatically. Anyway you rox! How else with such a :cool: name =)

nickl- avatar Jan 08 '13 22:01 nickl-

I forgot about this... this is kewl! +1

nickl- avatar Jun 28 '13 22:06 nickl-

There's a use case for using onDrop without requiring id's in the TR tags... One of the demos applied an alt class to even rows. Without onDrop being triggered, there's no way to update the table striping (unless TR ids exist). I'm working on an admin tool that appends new rows to the table and the ID requirement means I need to generate unique bogus ID's on-the-fly to satisfy the drop requirement.

onDrop should actually trigger onDrop and not have any additional requirements. If there is a requirement, the online documentation should be updated to specifically indicate "this function won't work unless IDs are added to one or more TRs" and not just hope developers contrast & compare HTML samples until they make a correlation.

If a table doesn't have IDs for TRs and it's not critical to the web app, the "jquery.unique-element-id.js" plugin can be used to generate unique IDs on-the-fly. Here's what I ended up using.

<style type="text/css">
.odd {background-color:#e0e0e0;}
</style>
<script type="text/javascript">
$(function(){
    $("#myTable tbody tr:even").addClass("odd");
    $("#myTable tbody tr").each(function(){
        if ($(this).attr("id") == undefined){
            $(this).uid();
        }
    }).promise().done(function(){
        $("#myTable").tableDnD({
            onDrop: function(table, row) {
                $(table).find("tbody tr.odd").removeClass("odd");
                $(table).find("tbody tr:even").addClass("odd");
            }
        });
    });
});
</script>

JamoCA avatar Mar 10 '16 16:03 JamoCA