• 
      

    Refactor markup and CSS for entries on review request page.

    Review Request #8373 — Created Sept. 1, 2016 and submitted

    Information

    Review Board
    release-3.0.x
    35843b0...

    Reviewers

    This change does some initial work cleaning up the markup and CSS for the
    entries on the review request page (review and change descriptions). The
    changes involved are:

    • Removal of the use of the {% box %} template tag. This tag was adding a
      bunch of additional layers of elements, which weren't really helpful. We were
      overriding almost all the styles we got from them, and the very generic class
      names that it used meant that our CSS selectors were a lot deeper than it
      needed to be.
    • Updated class names within the entry headers to be a bit more sensible
      ("reviewer" -> "summary", "posted_time" -> "timestamp").
    • Removed the margin on the sides of the entries, making them the same overall
      width as the review request box (this wasn't too obvious before, but is much
      more after adding a new top-level item for the initial status updates which
      doesn't have the callout and avatar).
    • Tweaking of the spacing within boxes, especially collapsed ones, so that
      there's equal height on either side of the collapse button.
    • Fixed the color of the callout triangle for change descriptions to match the
      background of the box (it was previously white, which was so close that we
      didn't notice it before).

    The details of the markup and class name changes have been added to
    https://www.notion.so/Review-Request-page-changes-for-3-0-8d27aac9e3ed4f81ac1e849ebb6127fe

    Created a review request with a handful of reviews and change descriptions.
    Verified the appearance and functionality in both desktop and mobile mode.


    Description From Last Updated

    The body_top is no longer aligned correctly. It's too far to the left.

    chipx86chipx86

    This goes before other selectors on other stylesheets (and rules below).

    chipx86chipx86

    Looks like this can be removed.

    chipx86chipx86

    Should go before .header

    chipx86chipx86
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/views/reviewBoxView.es6.js
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/static/rb/js/views/collapsableBoxView.es6.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/boxes/change.html
      
      
      
      Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/views/reviewBoxView.es6.js
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/static/rb/js/views/collapsableBoxView.es6.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/boxes/change.html
      
      
    2. 
        
    david
    1. 
        
    2. Not sure why the very bottom got cut off here--probably something weird between the full-page screenshot extension I used and devtools' mobile mode.

    3. 
        
    david
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/static/rb/js/views/tests/reviewBoxViewTests.js
          reviewboard/static/rb/js/views/collapsableBoxView.es6.js
          reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/static/rb/js/views/reviewBoxView.es6.js
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/static/rb/js/views/tests/reviewBoxViewTests.js
          reviewboard/static/rb/js/views/collapsableBoxView.es6.js
          reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/static/rb/js/views/reviewBoxView.es6.js
      
      
    2. 
        
    chipx86
    1. 
        
    2. Show all issues

      The body_top is no longer aligned correctly. It's too far to the left.

      1. So I realize that I neglected to set depends-on for all of these. This is fixed in /r/8391/

        How much do you want me to try to fix this in this change if it's known to be fixed in one that will land concurrently?

      2. Oh, I didn't know it was fixed there. I'm not worried about it in this change then.

    3. reviewboard/static/rb/css/pages/reviews.less (Diff revision 2)
       
       
       
       
      Show all issues

      This goes before other selectors on other stylesheets (and rules below).

    4. reviewboard/static/rb/css/pages/reviews.less (Diff revision 2)
       
       
       
       
       
       
      Show all issues

      Looks like this can be removed.

    5. Show all issues

      Should go before .header

    6. 
        
    david
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/static/rb/js/views/tests/reviewBoxViewTests.js
          reviewboard/static/rb/js/views/collapsableBoxView.es6.js
          reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/static/rb/js/views/reviewBoxView.es6.js
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/templates/reviews/boxes/review.html
          reviewboard/static/rb/js/views/tests/reviewBoxViewTests.js
          reviewboard/static/rb/js/views/collapsableBoxView.es6.js
          reviewboard/static/rb/js/pages/views/tests/reviewRequestPageViewTests.es6.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/boxes/change.html
          reviewboard/static/rb/js/views/reviewBoxView.es6.js
      
      
    2. 
        
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (054cb8d)