Add read-only banner under navbar

Review Request #8812 — Created March 11, 2017 and submitted

Information

khp
Review Board
master

Reviewers

Read-only mode is a setting an admin can enable to prevent writes to
the database. This can be used when the site is under maintenence or
being upgraded. This commit adds a banner under the nav bar when the
site is in read-only mode.

Check that the banner appears in read-only mode
Check that the banner does not appear when not in read-only mode


Description From Last Updated

Can you wrap your description at 72 characters?

brenniebrennie

Please attach a screenshot.

brenniebrennie

Just one newline here?

szhangszhang

It looks like these lines should be indented by an addition one space?

szhangszhang

Can you change these to read-only-... instead? It's nicer than readonly.

chipx86chipx86

Can you make these colours into constants in defs.less?

brenniebrennie

Can you make these constants in defs.less?

chipx86chipx86

Where does the 20px come from? It'd be nice to have this value in defs.less.

chipx86chipx86

Can you change this to read_only_banner?

chipx86chipx86

This would be much nicer as read_only_banner.html. Although, is there really any reason to split that off into another file? …

chipx86chipx86

Should have the same name as the block. Helps to catch problems as code shuffles around, as Django will do …

chipx86chipx86

Can you rename this to readonly_banner.html ?

brenniebrennie

If this file is included, then this check was already done. No need to duplicate it here.

chipx86chipx86

Instead of "Site" how about "Review Board"

brenniebrennie
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/css/pages/base.less
        reviewboard/templates/base/readonlybanner.html
        reviewboard/templates/base.html
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/css/pages/base.less
        reviewboard/templates/base/readonlybanner.html
        reviewboard/templates/base.html
    
    
  2. 
      
szhang
  1. 
      
  2. reviewboard/static/rb/css/pages/base.less (Diff revision 1)
     
     
     
    Show all issues

    Just one newline here?

  3. reviewboard/templates/base.html (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    It looks like these lines should be indented by an addition one space?

    1. Nice catch, will fix.

  4. 
      
KH
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/css/pages/base.less
        reviewboard/templates/base/readonlybanner.html
        reviewboard/templates/base.html
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/css/pages/base.less
        reviewboard/templates/base/readonlybanner.html
        reviewboard/templates/base.html
    
    
  2. 
      
brennie
  1. 
      
  2. Show all issues

    Can you wrap your description at 72 characters?

  3. Show all issues

    Please attach a screenshot.

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

    Can you make these colours into constants in defs.less?

  5. Show all issues

    Can you rename this to readonly_banner.html ?

  6. 
      
chipx86
  1. 
      
  2. Show all issues

    Can you change these to read-only-... instead? It's nicer than readonly.

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

    Can you make these constants in defs.less?

  4. Show all issues

    Where does the 20px come from? It'd be nice to have this value in defs.less.

  5. reviewboard/templates/base.html (Diff revision 2)
     
     
    Show all issues

    Can you change this to read_only_banner?

  6. reviewboard/templates/base.html (Diff revision 2)
     
     
    Show all issues

    This would be much nicer as read_only_banner.html.

    Although, is there really any reason to split that off into another file? It seems we could just put it inline here. It's faster than doing an include, and is only 3 lines.

  7. reviewboard/templates/base.html (Diff revision 2)
     
     
    Show all issues

    Should have the same name as the block. Helps to catch problems as code shuffles around, as Django will do a name check and make sure blocks line up.

  8. Show all issues

    If this file is included, then this check was already done. No need to duplicate it here.

  9. 
      
KH
KH
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/css/pages/base.less
        reviewboard/static/rb/css/defs.less
        reviewboard/templates/base.html
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/css/pages/base.less
        reviewboard/static/rb/css/defs.less
        reviewboard/templates/base.html
    
    
  2. 
      
brennie
  1. 
      
  2. reviewboard/templates/base.html (Diff revision 3)
     
     
    Show all issues

    Instead of "Site" how about "Review Board"

  3. 
      
KH
KH
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (4725dfa)
Loading...