Single-column "diff" view for new code files

Review Request #311 — Created March 27, 2008 and submitted

Information

Review Board SVN (deprecated)

Reviewers

We've just started using reviewboard and I decided I'd much rather see code from new files in a single, full-page column vs. in a half-page column next to an empty column.  Also, I wanted the background to be white instead of green.  Dunno what you guys think about that but I figured I'd submit the hacks I made to do this.
Verified it works in my install.

Screenshots

chipx86
  1. Thanks for the diff.
    
    I'm not a fan of removing the green because the green does indicate new code and is important, imho. I wouldn't mind a patch that just collapses the new file down to one column, though.
    
    How I would like to see this done is to have a class on the table itself that indicated that this was a new file, rather than appending -newcode to existing classes. What this would allow is for people to keep the existing styles we have and for others to write custom CSS to override those styles for inserted lines.
    
    In Review Board 2.0 (pretty early in its development, actually) we're going to have extension support. This would allow you to write a simple extension that adds custom CSS to every page to make the color white. With that and the CSS changes I mentioned, you wouldn't need to have a patched tree. Of course, it will still be a little while before we're done with 1.0.
    1. Makes sense.  See if the new diff is more what you had in mind.
      
  2. 
      
chipx86
  1. This looks better. Thanks.
    
    A couple small things and it's ready to go. The main thing really is going to be updating this to merge properly with the recent i18n work that went in.
  2. Just to make it more clear, can you put parens around the expression?
  3. The ":" should probably be removed.
    
    We don't use the "x and y or z" syntax anywhere else in the codebase. It's perfectly valid, but I don't know how readable it is to those who aren't used to the expression. Maybe we can make this more verbose by using an if statement and setting dest_revision based on that.
    
    Also, if you could localize these (put _() around them), that'd help in our effort to improve i18n.
  4. 
      
chipx86
  1. Committed in r1303. Thanks!
  2. 
      
Loading...