Added CheckboxColumn for datagrids

Review Request #3796 — Created Jan. 28, 2013 and discarded

Information

Djblets
master

Reviewers

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 …

chipx86chipx86

No need for this pass.

chipx86chipx86

You don't need the + when concatenating strings. Removing it will let you remove one layer of parens here.

chipx86chipx86

Should probably escape the contents of the label, in case it includes something like: '/>..

chipx86chipx86

Should follow this convention: """One-line summary Description. """

chipx86chipx86

Same here.

chipx86chipx86

Missing a trailing period.

chipx86chipx86

Not sure this should be here?

chipx86chipx86

Should be able to fit the function on the first line.

chipx86chipx86

Must use var statements. Should be like: var checked = ..., col = ...;

chipx86chipx86

This will be faster if you make the selector more fine-grained. Should include the datagrid itself, if possible, and limit …

chipx86chipx86
reviewbot
  1. 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
    
    
  2. 
      
chipx86
  1. I'm excited about this change and the Review Board one :) Some things to fix though.
  2. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
    Show all issues
    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.
  3. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues
    No need for this pass.
  4. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
    Show all issues
    You don't need the + when concatenating strings. Removing it will let you remove one layer of parens here.
  5. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues
    Should probably escape the contents of the label, in case it includes something like: '/><script>..</script>
  6. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
     
    Show all issues
    Should follow this convention:
    
    """One-line summary
    
    Description.
    """
  7. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
     
    Show all issues
    Same here.
  8. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues
    Missing a trailing period.
  9. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues
    Not sure this should be here?
  10. djblets/media/js/datagrid.js (Diff revision 1)
     
     
     
    Show all issues
    Should be able to fit the function on the first line.
  11. djblets/media/js/datagrid.js (Diff revision 1)
     
     
     
    Show all issues
    Must use var statements. Should be like:
    
    var checked = ...,
        col = ...;
  12. djblets/media/js/datagrid.js (Diff revision 1)
     
     
    Show all issues
    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....])
    1. I've added .datagrid as a selector, is that be good enough?  Wasn't sure if there was a reasonable way to get the specific datagrid that this checkbox is contained in.   Suppose I could find the parent of the checkbox that was clicked and then find children of that if you'd rather?
    2. I think grabbing the parent and then finding children of that is a great solution. Review Board of course has no pages with multiple datagrids on it, but other users of djblets might.
  13. 
      
GC
reviewbot
  1. 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
    
    
  2. 
      
GC
reviewbot
  1. 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
    
    
  2. 
      
GC
reviewbot
  1. 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
    
    
  2. 
      
chipx86
  1. I know it's been a long time coming, but I'm working on getting these checkbox datagrid changes ported over to the new codebase and in for RB 2.0. I'll be discarding these review requests and pointing to the new changes. Thanks!

  2. 
      
GC
Review request changed
Status:
Discarded
Change Summary:
Modified a bit and put up again at /r/5141/