WIP: Review Dropdown Dialog

Review Request #8184 — Created May 24, 2016 and discarded

Information

Review Board

Reviewers

Attached is an experimental prototype for how the Review Dialog could look instead of the current modal dialog.

This is not posted for code review (needs a lot of cleanup); this is posted for general feedback and input from the RB team as to whether they have interest in this being upstreamed.

Visual usage and ensuring it maintains all current functionality.


Description From Last Updated

Can I get fresh eyes on this? If these changes aren't desired upstream, please let me know.

DA davidwalsh
DA
chipx86
  1. Hi David,

    A few things jump out at me, to start.

    1) The green is really distracting. I think we should keep much of the current aesthetic to help focus this purely on the interaction differences. Try something where the box is a panel coming down below the banner, instead of as part of the banner.
    2) Not sure we need such a large amount of spacing on the left/right. It doesn't do much other than reduce the amount of space available within the dialog, particularly with smaller screen sizes.
    3) The height of the dialog looks very small, though this might be because of the spacing just added around all sides. The dialog's focus is to help you scan over your review and edit it. Having sufficient height (and width, really) is important there.
    4) There's too much room for error, having the "Finish Edit" button right next to publish/discard. I just know someone's going to accidentally publish when they meant to close. Maybe play with the button on the right-hand side of the banner somewhere, with some form of visual indicator for opening/closing. (Not 100% sure what's best here, just worth trying some things).
    5) Some feedback I've seen in the past has been that people would like to make the dialog smaller, temporarily, while looking at code. I think the spacing on here is perhaps designed to help with that, but I'd suggest instead having a resize handle to increase/decrease the height of the dialog.

    1. Per your comments:

      1. The color we use is negligble and can be easily changed. I prefer a much more toned down green, rgb(219, 240, 219), which I think fits in better with your app colors. The color can be worked on.

      2. The left and right space is set to 5%; again, this too can be easily changed. I thought the whitespace made the dropdown look more "detached" for the whole page itself.

      3. The height is allowed to grow to within 80px of the screen height; i.e. it will get bigger with the more comments that are in a review. If there are a bunch of comments, the dropdown dialog is scrollable.

      4. Will think on this.

      5. Will think on this as well.

    2. 1) It may be, but we're already distracted by the bikeshedding here, so let's keep it at the color and style we had before. (FWIW, I don't want the dialog to be that green, or any green, but certainly not that sort of green. Makes me feel seasick.)
      2) We should stay away from percentages. Let's keep it roughly what we have now. Again, I don't want to have to even think about visual differences at this stage. Let's keep it fully about the functionality differences.
      3) Same as above. Let's keep this work simple and focused for now.

      1. Reverted to old color: white. Think something like #eee would look better.
      2. Set to 20px like it was before
      3. Same
      4. Moved the buttons away from each other; let me know if you think that's enough difference
      5. Made the dropdown resizable

      Cheeers!

  2. 
      
DA
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/views/draftReviewBannerView.js
        reviewboard/static/rb/js/extensions/models/reviewDialogCommentHookModel.js
        reviewboard/static/rb/js/extensions/models/reviewDialogHookModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/views/reviewDialogView.js
        reviewboard/templates/reviews/reviewable_base.html
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/views/draftReviewBannerView.js
        reviewboard/static/rb/js/extensions/models/reviewDialogCommentHookModel.js
        reviewboard/static/rb/js/extensions/models/reviewDialogHookModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/views/reviewDialogView.js
        reviewboard/templates/reviews/reviewable_base.html
    
    
  2. 
      
DA
DA
  1. 
      
  2. Show all issues

    Can I get fresh eyes on this? If these changes aren't desired upstream, please let me know.

    1. What exactly are you looking for with review? It looks like the code is pretty preliminary (as you mention). If you're just wondering whether this is a direction we agree with, I think at a high level, it is.

    2. One thing I noticed right away is that it looks really noisy without the darkened overlay on the page. It also doesn't blend well with the top bar. I'd like to see the border restored on the top, with this appearing as something sliding down "below" it.

    3. I know putting a dark overlay on top of the page content itself probably defeats some of what you're looking for with this. Perhaps a sizable box-shadow around the drop-down?

  3. 
      
chipx86
  1. I'm busy working on a new rethink/modernization of some of our reviewing capabilities, ideally for 3.0, and the review dialog is a big part of that. For now, to prevent throw-away work, I'm going to hold off on any changes to the review dialog code or styling. Once I have something worth showing that our team generally likes, we'll start talking with you guys about it and see if there's a good way to split up this work.

    1. We can close this; I'll be implementing it on our instance of ReviewBoard.

  2. 
      
DA
Review request changed
Status:
Discarded