• 
      

    Make the review request page mobile-friendly.

    Review Request #7288 — Created May 6, 2015 and submitted

    Information

    Review Board
    release-2.5.x
    84cc870...

    Reviewers

    The review request page is one of the most important pages on
    Review Board, and we want it to be fully usable on mobile devices. This
    change gets us a huge step of the way there by making the page
    mobile-friendly. It now fits on a typical mobile phone's screen. All
    operations, such as closing review requests, replying to comments, etc.
    are doable as well.
    
    Most of the changes are pretty straight-forward. Horizontal content has
    become more vertical (avatars are above review, timestamps no longer
    aligned to the upper-right, the issue summary table contents are
    multi-row cells, and so on).
    
    Diffed comment thumbnails are still horizontal (this will be addressed
    in a later change).
    
    The review request action bar had to be consolidated a bit. Now, all the
    actions are hidden under a "..." button. When clicked, this unfolds to
    show all available actions as a new, wrapped action bar.
    Implementation-wise, it's just a special action menu.
    
    These changes affect the diff viewer and file review UIs as well.
    However, those pages are not optimized yet for mobile, and will be
    updated in future changes.

    Tested that the main desktop-oriented layout continued to look correct on
    Chrome and Firefox.

    Tested the mobile UI on the iPhone and in responsive mode at different
    resolutions in Chrome.

    Tested review request actions, replying to comments, expanding/collapsing
    boxes, and toggling issue states.


    Description From Last Updated

    Can we move this below description and testing done? I feel like these are secondary.

    daviddavid

    I think I'd like this better if we presented it more like other drop-down menus: Close > Update > Download …

    daviddavid

    The text here needs to be run through gettext()

    daviddavid
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          reviewboard/templates/reviews/review_detail.html
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/templates/reviews/ui/base.html
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/templates/reviews/review_header.html
          reviewboard/static/rb/css/ui/banners.less
          reviewboard/static/rb/js/views/commentIssueBarView.js
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/static/rb/js/views/reviewReplyDraftBannerView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/static/rb/js/views/headerView.js
          reviewboard/static/rb/css/common.less
          reviewboard/templates/reviews/reviewable_base.html
          reviewboard/static/rb/css/defs.less
          reviewboard/templates/reviews/comment_issue.html
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/templates/reviews/review_detail.html
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/templates/reviews/ui/base.html
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/templates/reviews/review_header.html
          reviewboard/static/rb/css/ui/banners.less
          reviewboard/static/rb/js/views/commentIssueBarView.js
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/static/rb/js/views/reviewReplyDraftBannerView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/static/rb/js/views/headerView.js
          reviewboard/static/rb/css/common.less
          reviewboard/templates/reviews/reviewable_base.html
          reviewboard/static/rb/css/defs.less
          reviewboard/templates/reviews/comment_issue.html
      
      
    2. 
        
    david
    1. 
        
    2. Show all issues

      Can we move this below description and testing done? I feel like these are secondary.

      1. Not really, without starting to bring JavaScript into the picture to rearrange the DOM. It's an element order problem. We can't swap the order these elements are shown with CSS. I'm not really sure what impact that'll have for event handling and our stored element references.

      2. I wish we had flexbox. Then we could swap the order with CSS. Very well, carry on.

    3. Show all issues

      I think I'd like this better if we presented it more like other drop-down menus:

      Close >
      Update >
      Download Diff
      Review
      Ship It!

      That way things don't reflow oddly, and it's a bit strange to have drop-downs like close in the middle of a block (as judging from the account and support menus at the top).

      1. None of this is ideal, and I'd actually like to do a whole different design for this later, something more mobile-friendly. For the moment, I want to keep this as-is so we have something that can be used on a mobile device. The main reason being that it's really quite a pain to do anything too specialized here without causing issues with the desktop view, and the cascading menus were both a bit wonky and felt really out of place on mobile.

        I'm happy to revisit this later, and have some thoughts, but it requires some other work I want to do to create a more mobile-friendly actions menu. That's a much bigger project, though.

      2. Sure.

    4. Show all issues

      The text here needs to be run through gettext()

    5. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          reviewboard/templates/reviews/review_detail.html
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/templates/reviews/ui/base.html
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/templates/reviews/review_header.html
          reviewboard/static/rb/css/ui/banners.less
          reviewboard/static/rb/js/views/commentIssueBarView.js
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/static/rb/js/views/reviewReplyDraftBannerView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/static/rb/js/views/headerView.js
          reviewboard/static/rb/css/common.less
          reviewboard/templates/reviews/reviewable_base.html
          reviewboard/static/rb/css/defs.less
          reviewboard/templates/reviews/comment_issue.html
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/templates/reviews/review_detail.html
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/templates/reviews/ui/base.html
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/templates/reviews/review_header.html
          reviewboard/static/rb/css/ui/banners.less
          reviewboard/static/rb/js/views/commentIssueBarView.js
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/static/rb/js/views/reviewReplyDraftBannerView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/static/rb/js/views/headerView.js
          reviewboard/static/rb/css/common.less
          reviewboard/templates/reviews/reviewable_base.html
          reviewboard/static/rb/css/defs.less
          reviewboard/templates/reviews/comment_issue.html
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.5.x (b60a028)