Added CheckboxColumn for datagrids
Review Request #3796 — Created Jan. 28, 2013 and discarded
| Information | |
|---|---|
| gcoupar | |
| Djblets | |
| master | |
| Reviewers | |
| djblets | |
Added CheckboxColumn for datagrids This commit adds a new CheckboxColumn class inheriting from Column. When added to a DataGrid this column will contain checkboxes, with a checkbox in the column header that can check/uncheck all the checkboxes. The is_selectable & is_selected functions can be overridden to control whether a checkbox is displayed in a row and whether that checkbox is initially checked. The checkboxes have a data-object-id attribute that should allow javascript code to determine which rows have been checked and use that accordingly. This should allow mass submit/discard buttons to be easily added to the dashboard in reviewboard.
- Integrated into revivewboard (code in this review: http://reviews.reviewboard.org/r/3799/ ) - Confirmed that header checkbox selects/deselects properly - Confirmed that header behaves as expected (movable, displays correctly etc.) - Confirmed that it's possible to write javascript to check which rows are selected and act on it.
| Description | From | Last Updated |
|---|---|---|
|
The other columns don't really follow this yet, but this should be one line, like: """A column that renders a … |
 chipx86
|
|
|
No need for this pass. |
chipx86
|
|
|
You don't need the + when concatenating strings. Removing it will let you remove one layer of parens here. |
chipx86
|
|
|
Should probably escape the contents of the label, in case it includes something like: '/>.. |
chipx86
|
|
|
Should follow this convention: """One-line summary Description. """ |
chipx86
|
|
|
Same here. |
chipx86
|
|
|
Missing a trailing period. |
chipx86
|
|
|
Not sure this should be here? |
chipx86
|
|
|
Should be able to fit the function on the first line. |
chipx86
|
|
|
Must use var statements. Should be like: var checked = ..., col = ...; |
chipx86
|
|
|
This will be faster if you make the selector more fine-grained. Should include the datagrid itself, if possible, and limit … |
chipx86
|
-
I'm excited about this change and the Review Board one :) Some things to fix though.
-
djblets/datagrid/grids.py (Diff revision 1) The other columns don't really follow this yet, but this should be one line, like: """A column that renders a checkbox.""" Ideally, it should also go into some detail on how it's used. Basically, what you have in your review request description.
-
-
djblets/datagrid/grids.py (Diff revision 1) You don't need the + when concatenating strings. Removing it will let you remove one layer of parens here.
-
djblets/datagrid/grids.py (Diff revision 1) Should probably escape the contents of the label, in case it includes something like: '/><script>..</script>
-
djblets/datagrid/grids.py (Diff revision 1) Should follow this convention: """One-line summary Description. """
-
-
-
-
djblets/media/js/datagrid.js (Diff revision 1) Should be able to fit the function on the first line.
-
djblets/media/js/datagrid.js (Diff revision 1) Must use var statements. Should be like: var checked = ..., col = ...; -
djblets/media/js/datagrid.js (Diff revision 1) This will be faster if you make the selector more fine-grained. Should include the datagrid itself, if possible, and limit it to input elements (input[data-checkbox-col....])
GC
Change Summary:
Fixed all the issues pointed out (except possibly a selector). Added a new parameter checkbox_column_name to be used for the data-checkbox-col HTML attribute, as the label will be localised and isn't really suitable for this.
Diff: |
Revision 2 (+65) |
|---|
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py Ignored Files: djblets/datagrid/templates/datagrid/column_header.html djblets/media/js/datagrid.js
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py Ignored Files: djblets/datagrid/templates/datagrid/column_header.html djblets/media/js/datagrid.js
GC
Change Summary:
Updated click handler for checkbox column header to find children of parent datagrid.
Diff: |
Revision 4 (+66) |
|---|
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py Ignored Files: djblets/datagrid/templates/datagrid/column_header.html djblets/media/js/datagrid.js
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/datagrid/grids.py Ignored Files: djblets/datagrid/templates/datagrid/column_header.html djblets/media/js/datagrid.js