I made a patch witch is adding ability to limit the amount of selected rows to a specified number, not just none, 1 or any amount (old behavior is not touched by default).
It adds a new option called "selectMaxRows" witch is 0 by default. The default value means that selectableRows works per default: 0 - no rows can be selected, 1 - 1 row can be selected, 2 and more - any number of rows can be selected.
Because on selectableRows option depend some other things, and changing it to work as a real limit would do BC breaks (for example inability to use "check all" checkbox by default if we have values 2 or more and a need to add some special value like "-1" to be able to select any number of rows), I decided to add this new option.
The option works in conjunction with "selectableRows" being grater than 1. If "selectableRows" is 0 or 1 - new option does not have any effect on the behavior. It only has effect if "selectableRows" >= 2 and new option is provided value grater than 0.
If selectableRows >= 2 and selectMaxRows > 0, than “check all” checkbox does not render any more (that’s logical, isn’t it?)
Also I made a small addition to the call of the callback we can set via “selectionChanged”. Now it has a second param witch is a DOM object of the <tr> we have clicked on - using that object we can have access to the row’s <td>'s to read or manipulate the cells of the row - it can really handy to be able to at least read the data.
I’d like the devs to review the patch and see if I have missed something (witch I probably did, most likely in CCheckBoxColumn class). The patch is against trunk revision 3379.
I would really appreciate if this patch would make into framework
Briefly reviewed it. Overall it looks OK and I think ability to limit selection to arbitrary # of rows is useful. Still I’m not sure what’s better: to have very confusing additional option or to break BC.
For some it’s rare, for others not. It really depends on the kind of work you do. Example: you give user the ability to do some batch work on the records, but you limit him with some reasonable limit. People will find use cases as usual
I guess the part about “selectionChanged” changes is good and has no objections (Coz it’s really weird not having the ability to know on witch row the user has clicked).
I agree that some users would liek to use this very often… and maybe someone would like something similar… to set the exact number of checkboxes to be checked… so "maybe" someone would like to set selectableRows=4 and expect that all 4 are checked or none…
What I mean with rare is that not many users will need this functionality…
In the selectionChanged function you can use "this" to get the current "clicked" object (column) and from there you can get to the row object…
…
So… lets see if other users will express interest in all this…
Well, here is the part where you are totally wrong about selectionChanged. If you see the code in jquery.yiigridview.js, the call looks like this
settings.selectionChanged(id);
Just add the breakpoint in selectionChanged callback implementation under firebug and you will see that there is no such thing as row on witch the click was initiated. So I had to patch the code and add an extra argument to pass when we click on the row.
Honestly, there should be a separate callback when clicking on the row, because selectionChanged is called also when we click on "check all" checkbox, so second argument is undefined on that event (actually, the rendering of that checkbox is disabled if we limit the amount of selected rows to a specific number, so we have some consistency here).
Besides, my patch works with checkboxes too - it limits not just clicks on rows, but handles checkbox selection as well.
