Added mass submit/discard to dashboard

Review Request #3799 — Created Feb. 5, 2013 and submitted

Information

Review Board
master
579

Reviewers

Added mass submit/discard to dashboard

This commit adds a CheckboxColumn to the datagrid in the dashboard, as
well as some links in the datagrid header to submit or discard all the
selected reviews.  When clicked, the links will send API requests to
close each of the selected reviews and will then reload the page.

This code depends on the CheckboxColumn being added to djblets, which
I've submitted in another review: http://reviews.reviewboard.org/r/3796/
- Confirmed that submit/discard selected buttons closes the reviews then reloads the page
- Confirmed that check/uncheck all works

Description From Last Updated

Can you make this another paragraph? That way there dialog isn't as wide, and easier to scan.

chipx86chipx86

Col: 80 E501 line too long (83 > 79 characters)

reviewbotreviewbot

Should only have one import, and then import CheckboxColumn on a second line, using \ for line continuation. CheckboxColumn would …

chipx86chipx86

Labels are meant to be localized. This one should be. However, that means you'll have a problem with looking it …

chipx86chipx86

What does all this cover?

chipx86chipx86

Comments should be proper sentences and end in a trailing period. (Yes, it's nitpicky, but we strive for nitpicky consistency …

chipx86chipx86

Really, this is "registerDashboardActions".

chipx86chipx86

Should use one var statement, and comma-separate them (one per line). This query should be more limited in scope. It …

chipx86chipx86

Trailing whitespace.

chipx86chipx86

blank line between these.

chipx86chipx86

Same comment about vars. Also, blank line after vars.No need for the 'self' one though.

chipx86chipx86

Blank line.

chipx86chipx86

Can you fix these vars while you're at it?

chipx86chipx86

One space indentation. I think it'd be best if these IDs were more specific so they can't conflict. Prefix with …

chipx86chipx86
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/reviews/datagrids.py
      Ignored Files:
        reviewboard/static/rb/css/dashboard.less
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/datastore.js
        reviewboard/templates/reviews/dashboard_listview.html
    
    
  2. reviewboard/reviews/datagrids.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (83 > 79 characters)
    
  3. 
      
GC
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/reviews/datagrids.py
      Ignored Files:
        reviewboard/static/rb/css/dashboard.less
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/datastore.js
        reviewboard/templates/reviews/dashboard_listview.html
    
    
  2. 
      
chipx86
  1. Can you provide screenshots of this? One showing it off, one with the menus opened.
    
    One more addition I think is important is to prompt before making the action happen. It should ask an "Are you sure" question along the lines of our existing ones for, say, deletion (when you're an admin).
  2. reviewboard/reviews/datagrids.py (Diff revision 2)
     
     
     
    Show all issues
    Should only have one import, and then import CheckboxColumn on a second line, using \ for line continuation.
    
    CheckboxColumn would align with Column. ReviewBot is going to yell at you about this, but ignore it. It's one of those things we can't cleanly turn off...
  3. reviewboard/reviews/datagrids.py (Diff revision 2)
     
     
    Show all issues
    Labels are meant to be localized. This one should be. However, that means you'll have a problem with looking it up later. I think you may need to rethink how to do that.
  4. Show all issues
    What does all this cover?
    1. I think (sorry, did this bit a month or 2 ago) I mostly copied and adapted it from the actions on a review, but removed the left border because it didn't look particularly good.
      
      The img bit definitely isn't needed, so I've removed that.
      
      As for the rest of it: floats the actions to the right hand side of the header, removes list style, makes Think you should be able to see most of it from the screenshot.  
      
      I'm definitely open to suggestions for improving how it looks - it functions ok at the moment but I've had trouble getting it to look as slick as the actions on the review page.
    2. Ideally we'd share as much as possible with the existing action rules, instead of doing a big copy/paste.
  5. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
    Show all issues
    Comments should be proper sentences and end in a trailing period. (Yes, it's nitpicky, but we strive for nitpicky consistency :)
  6. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
    Show all issues
    Really, this is "registerDashboardActions".
  7. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
     
    Show all issues
    Should use one var statement, and comma-separate them (one per line).
    
    This query should be more limited in scope. It should only check within the dashboard's datagrid, and check input[data-...].
    
    Name should be "checkedBoxes"
  8. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
     
    Show all issues
    Trailing whitespace.
  9. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
     
    Show all issues
    blank line between these.
  10. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
     
     
    Show all issues
    Same comment about vars. Also, blank line after vars.No need for the 'self' one though.
  11. reviewboard/static/rb/js/common.js (Diff revision 2)
     
     
     
    Show all issues
    Blank line.
  12. reviewboard/static/rb/js/datastore.js (Diff revision 2)
     
     
     
     
    Show all issues
    Can you fix these vars while you're at it?
  13. Show all issues
    One space indentation.
    
    I think it'd be best if these IDs were more specific so they can't conflict. Prefix with dashboard- I guess.
  14. 
      
david
  1. Please add bug 579 to the "bugs" field here.
    
    Can you attach some screenshots of this?
  2. 
      
GC
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/reviews/datagrids.py
      Ignored Files:
        reviewboard/static/rb/css/dashboard.less
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/datastore.js
        reviewboard/templates/reviews/dashboard_listview.html
    
    
  2. 
      
chipx86
  1. 
      
  2. Show all issues
    Can you make this another paragraph? That way there dialog isn't as wide, and easier to scan.
  3. 
      
david
  1. I love the feature, but I'd like to address the UI a bit.
    
    The location and state of the command links (Submit Selected, Discard Selected) isn't optimal. It's confusing that they're in a position pretty far away from the check-boxes themselves (where the user's mouse is), and I don't think it looks very good to just have the text floating there in the header, especially when nothing is selected.
    
    Ideally, I'd like something to appear close to the mouse when the first box is checked (maybe slide out from the selection box in the column headers) and otherwise be hidden when there's nothing selected.
    
    Also, is it possible to put a check-box in the column selection drop-down? I think that would make it a lot more intuitive than just saying "Select".
  2. 
      
david
  1. Christian took this, updated it for the 2.0 code, and addressed some of my UI concerns. It's been submitted for some time now.

  2. 
      
GC
Review request changed
Status:
Completed