Start fixing up the Review Board color scheme definitions.

 






 

 


  Review Request #10442
  —
  Created March 11, 2019 and submitted 

 




















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  release-4.0.x
 









 
 

  
 









 
 

  
 









 
 

  10466
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This review request covers several commits, starting to clean up and

fix the Review Board color scheme. We have a lot of hard-coded colors

throughout the stylesheets, many of which are unintentional variations

on the same set of basic colors.


To start, this adds new definitions for the different shades of grey,

blue, brown, green, red, and yellow. We also had a number of defined

colors which weren't used very much (or at all), which have now been

properly commented as deprecated.


The actual changes to the styles cover:


    

  • The background color assigned to input elements in the upload-diff
    
  mixin evaluated to white, so I've just removed the rule.
    • 

  • The border color for the drag-hover in the upload-diff mixin was so
    
  close to the normal border color as to be indistinguishable.
    • 

  • The issue summary table had a yellowish border on mobile only, which
    
  looked kind of weird (the tabs still used a grey border). I've removed
    
  the special rule and now everything uses the same grey border.
    • 

  • Uses of @grey and @light-blue have been changed to use the new
    
  color definitions.
    • 

  • Started converting the main page colors over to the new color
    
  definitions, primarily in the .page-colors mixin, which controls the
    
  color scheme of the header bar.
    • 

  • Changed the definition of the link color to blue-20, which is a dark
    
  teal-like color (though there are a lot of links throughout the UI
    
  which do not properly use the link-color definition yet).
    • 



While I was in here, I noticed that the scrollable portion of the "New

Review Request" page would overflow the rounded corners on Chrome.

There's a weird hack that adding a null transform fixes this, which I've

added in.

 








  


 
 

  
Verified the different page styles (base, reviewable, admin) and

checked that the affected styles looked correct.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Add new color scheme definitions.
      
This change adds the new definitions for the Review Board color scheme.
This includes varying shades of grey, blue, brown, green, red, and
yellow. This doesn't completely cover all the colors used in the
product, but it's the vast majority of them. We have a bunch of
hard-coded colors throughout the CSS, many of which are slightly varying
shades of the same thing. These new definitions will allow us to keep
more consistency throughout.

     


    		
    16139a2fbff584a72f3da8fa4c2606f7a19ce3d7

   


   


    

     

      Move old color definitions into a deprecated section.
      
We had a few named color definitions in our `defs.less` file, several of
which were completely unused. The ones which were used have been cleaned
up in various ways:

- The background color assigned to input elements in the upload-diff
  mixin evaluated to white, so I've just removed the rule.
- The border color for the drag-hover in the upload-diff mixin was so
  close to the normal border color as to be indistinguishable.
- The issue summary table had a yellowish border on mobile only, which
  looked kind of weird (the tabs still used a grey border). I've removed
  the special rule and now everything uses the same grey border.
- Uses of `@grey` and `@light-blue` have been changed to use the new
  color definitions.

While I was in here, I noticed that the scrollable portion of the "New
Review Request" page would overflow the rounded corners on Chrome.
There's a weird hack that adding a null transform fixes this, which I've
added in.

Testing Done:
Verified visual appearance of changed styles.

     


    		
    e78e4219065f95bf519ac8c5c5eed3e99ea95997

   


   


    

     

      Move button definitions into buttons.less.
      
We had a bunch of definitions that were specific only to buttons, and it
felt a bit silly to include them in `defs.less` when they were only used
once. I've moved them to `buttons.less`.

     


    		
    7a1f16d0f89f7f3d3a87519d9360939a1033054a

   


   


    

     

      Start converting main page colors over to new definitions.
      
This change starts to migrate the main page colors over to the new color
definitions. The bulk of this change relates to the `.page-colors`
mixin, which controls the color scheme of the header bar. It also
changes the `link-color` definition to use `blue-20`, which is a dark
teal-like color (though there are a number of links throughout which do
not yet use that style definition).

Testing Done:
Looked at the different page styles (base, reviewable, admin) and
verified that the topbar looked correct.

     


    		
    8838d95728080e306bab75e48e51f97e03411db0

   



  
 




 













 

  
  


   

    

     

     

     

    

   


   

  

 







 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Right now, defs.less includes colors.less. Do we want to make that official, or do we want to effectively work toward …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Generally, an overflow: hidden will do this just fine. Does it not in this case? Kind of feel like this …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Can we define these in a namespace?

      		
      
       chipx86chipx86
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/css/mixins/theme.less
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Right now, defs.less includes colors.less. Do we want to make that official, or do we want to effectively work toward deprecating defs.less (at least in this way) and instead encourage consumers to include colors.less themselves?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Hmm. So I haven't yet entirely decided how we should handle making use of the basic color definitions. You started making a "color schemes" thing, but I'm not really sure how that might generalize further. For now I was intending to have the basic hex codes all defined in colors.less, and then do things like define @draft-color in defs.less. I could see having all of that in colors.less, though. Thoughts?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      I'm not entirely sure either. I think it'd be nice to move away from defs.less and move to definitions matching up with components, and have standardized colors in colors.less, but it could turn into a mess easily... I imagine we'll heavily iterate on whichever way we start with.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      I think I'm going to proceed with putting new things in defs.less now now, and it's easy to move once we have a fuller picture.
      
 
      

      4. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/css/ui/buttons.less
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Can we define these in a namespace?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I don't want to get too far into the weeds with that right now.
      
 
      

      2. 


 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/css/pages/newReviewRequest.less
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Generally, an overflow: hidden will do this just fine. Does it not in this case?
    

    Kind of feel like this would be better in its own small change.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Yeah, this is required due to a weird bug in chrome that they don't seem interested in fixing.
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to release-4.0.x (ec6d586), release-4.0.x (a20e672), release-4.0.x (d20bd44), release-4.0.x (ae389d6)