fnRowDeselected node parameter acts unexpectedly

ScottG489

So I was previously selecting rows with jQuery UI's selectable plugin and decided to move over to use the one built into TableTools. The interfaces seemed about the same and it transitioned smoothly besides one unexpected behavior...

I'm using single select and fnRowDeselected's node parameter is always the first TR node in the table unless what is being deselected is the row that was previously selected. The behavior I expected (how it worked in jQuery UI) was that the node parameter would be that of the row which was deselected. Meaning, if I have row 3 selected, and I select row 2, I'd expect fnRowDeselected's node parameter to be 3.

I looked through the code and was able to determine why this was happening. It seems that after selecting a row that isn't currently selected, fnSelectNone() is called which calls _fnRowDeselect() passing it an array of all rows. After it deselects all of them it calls the users fnRowDeselected function with the first tr node.
What I think it should be calling the users fnRowDeselected function with is the row that was deselected (or in the case of multiselect, an array of the rows deselected) . Something like this:

[code]
"_fnRowDeselect": function(src)
{
var data = this._fnSelectData(src);
var firstTr = data.length === 0 ? null : data[0].nTr;
var deselectedTrs = [];

for (var i = 0, iLen = data.length; i < iLen; i++)
{
if (data[i].nTr && data[i]._DTTT_selected)
{
$(data[i].nTr).removeClass(this.classes.select.row);
deselectedTrs.push(data[i].nTr);
}

data[i]._DTTT_selected = false;
}

if (this.s.select.postDeselected !== null)
{
this.s.select.postDeselected.call(this, deselectedTrs);
}

TableTools._fnEventDispatch(this, 'select', firstTr);
}
[/code]

I don't intend for this to be a patch or anything, just a snippet to demonstrate what behavior I'd expect. I'm also not sure if this is actually a bug or how it was intended to act. To me it seems unintuitive at least.

Otherwise, what would you suggest to be the best way of getting data about the row being deselected?

allan

I'd say this was a bug. Its completely unintuitive the way it is at the moment, and I'm sure I didn't intend that behaviour!

In fact, looking at it further, that's a fairly horrible bit of API design in general by me :-(. I think that needs properly reworked long term (TableTools is due for a bit of work after DataTables 1.10 I think!).

I've just committed a change very similar to your own, although I've also changed the dispatched event (which also has another parameter to say that is was a deselect which triggered it...) and the 'select' callback / event, to also pass through an array. It is possible that might upset a little bit of backwards compatibility, but this really was a bug that needed to be fixes, and if anyone does hit a problem, they can just add `[0]` to their own code :-)

This is now in the TableTools 2.1.4.dev nightly.

Thanks for taking the time to post your findings! :-)

Allan

ScottG489

Hey allan. Thanks for the quick response and fix (even on a weekend!)

One other thing I noticed that you also might want to look into is that fnPreRowSelect doesn't fire when fnSelect is called. I'm guessing this is due to the fact that it's only run when a click event happens. I noticed this when I was trying to use it as a workaround for my last problem :P

Thanks again and I'm a big fan of the project!

allan

Good stuff - sorry you are finding so many bugs :-). But thanks for taking the time to report them!

I've just committed a change to fix that, and make the pre- and post- select functions perhaps a little more useful.

The pre-select now gets three parameters:
1. event that triggered the select (can be undefined!)
2. nodes to be selected or unselected
3. boolean flag to indicate if selection (true) or deselection (false)

The post-select now gets two parameters:
1. nodes that were selected or unselected
2. boolean flag to indicate if selection (true) or deselection (false)

This change is now available in the nightly :-)

Thanks again!
Allan

ScottG489

Thanks again allan.

I have just 1 (hopefully) more issue. It seems that fnSelectNone is a little too liberal with what rows it deselects. For example, now when I select a row my fnRowDeselected function ends up getting all of the nodes in the table. Perhaps fnSelectNone should only deselect the rows that are currently selected. Or perhaps _fnRowDeselect should only add nodes to the anDeselectedTrs array that were actually changing from the selected to deselected state. After all, if a row isn't currently selected is it really ever able to be deselected?

allan

Hi,

Sorry for the long delay on this one - I've not forgotten about it :-).

I've just committed a change that makes the select none action a whole lot less aggressive. It will only deselect rows which are currently selected.

I've also fixed an issue I introduced with the last commit (*guilty...*).

Regards,
Allan