Fine-tune our styling, polishing presentation and reducing visual noise.

Review Request #13197 — Created Aug. 9, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

Review Board's had the same general look for a long time now, and some
of it hasn't aged particularly well. There's a harshness to many of our
content panes and other parts of the UI, with borders that stand out too
much, colors that are too strong, and UI elements that aren't emphasized
well enough.

While we're not about to unveil a major redesign of Review Board, this
change does clean up much of our common presentation in the following
ways:

  • Borders are now more subtle throughout the UI, helping to blend in
    better with nearby backgrounds while still providing a visual
    separation. This includes content box borders, field borders in the
    review request, banner borders, and various separators.

  • Shadows are now placed below content, rather than toward the
    bottom-right, helping keep a more symmetrical feel to content boxes.

  • Status update icons are now larger, with both success and failure
    icons being enclosed in a filled circle to make them stand out better.
    Colors have been tweaked to fit the general aesthetic.

  • Status update section headers are also less harsh, andno longer
    rely on the diff color constants.

  • Diffs in the diff viewer now have more separation (2x the page
    padding). This provides a better visual separation between diffs.

  • Colors within diffs have been made softer, more pastel, with blue
    section headers instead of brown. This creates an almost "Easter"
    feel, which is more pleasant to look at. In particular, insert/delete
    lines are also far easier on the eyes, especially when looking at
    large blocks of code.

  • Headers in diffs showing the nearest function/class to expand to
    are now just a little bigger, helping it stand out as a proper header.

  • In replace lines, the replaced content is now more of a gold, rather
    than a mustard.

  • Diff text now has a very slightly larger font size, which is easier on
    the eyes on today's modern displays. It's still smaller than GitHub's
    text, with a good balance of readability and being able to fit plenty
    of content on the screen.

  • Comment bubbles are now pill-shaped, rather than slightly-rounded
    rectangles. This looks nice and stands out better. Colors have also
    been made less harsh.

One thing to note in this change is that there's a lot more hard-coded
colors, rather than reliance on the #rb-ns-ui.colors() constants. This
is intentional. Many of these parts of the UI never mapped to those
color constants to begin with. We'll also be introducing more general
concepts of palettes and mode-specific styless
(light/dark/high-constrast) in Review Board 7. All color usage we have
today is going to be redone as part of that, and as such, it's not worth
altering the base color palettes to try to match these changes today.

Tested the dashboard, review requests (with all possible types of content),
diffs, My Account page, and the administration pages on Chrome and Firefox,
verifying that presentation looked correct.

Summary ID
Fine-tune our styling, polishing presentation and reducing visual noise.
Review Board's had the same general look for a long time now, and some of it hasn't aged particularly well. There's a harshness to many of our content panes and other parts of the UI, with borders that stand out too much, colors that are too strong, and UI elements that aren't emphasized well enough. While we're not about to unveil a major redesign of Review Board, this change does clean up much of our common presentation in the following ways: * Borders are now more subtle throughout the UI, helping to blend in better with nearby backgrounds while still providing a visual separation. This includes content box borders, field borders in the review request, banner borders, and various separators. * Shadows are now placed below content, rather than toward the bottom-right, helping keep a more symmetrical feel to content boxes. * Status update icons are now larger, with both success and failure icons being enclosed in a filled circle to make them stand out better. Colors have been tweaked to fit the general aesthetic. * Status update section headers are also less harsh, andno longer rely on the diff color constants. * Diffs in the diff viewer now have more separation (2x the page padding). This provides a better visual separation between diffs. * Colors within diffs have been made softer, more pastel, with blue section headers instead of brown. This creates an almost "Easter" feel, which is more pleasant to look at. In particular, insert/delete lines are also far easier on the eyes, especially when looking at large blocks of code. * Headers in diffs showing the nearest function/class to expand to are now just a little bigger, helping it stand out as a proper header. * In replace lines, the replaced content is now more of a gold, rather than a mustard. * Diff text now has a very slightly larger font size, which is easier on the eyes on today's modern displays. It's still smaller than GitHub's text, with a good balance of readability and being able to fit plenty of content on the screen. * Comment bubbles are now pill-shaped, rather than slightly-rounded rectangles. This looks nice and stands out better. Colors have also been made less harsh. One thing to note in this change is that there's a lot more hard-coded colors, rather than reliance on the `#rb-ns-ui.colors()` constants. This is intentional. Many of these parts of the UI never mapped to those color constants to begin with. We'll also be introducing more general concepts of palettes and mode-specific styless (light/dark/high-constrast) in Review Board 7. All color usage we have today is going to be redone as part of that, and as such, it's not worth altering the base color palettes to try to match these changes today.
6ddf7975e08a4beb27a16bf2019d4e00df1b8c61

Description From Last Updated

I don't love the beige/yellow here, it looks too similar to the yellow that appears in diffs. I prefer the …

maubinmaubin

Since we're touching these, can we get these moved over to colors.less with named definitions? Same for the other explicit …

daviddavid

Looks like there's an extra blank line here?

daviddavid
maubin
  1. The borders look a bit thick in the screenshots, but in real life they look really nice.

    1. Wonder if that's some artifact of how Firefox takes screenshots?

  2. Show all issues

    I don't love the beige/yellow here, it looks too similar to the yellow that appears in diffs. I prefer the old grey, and I tested it out and think that it still looks nice with the new blue headers.
    Image

    1. I went back and forth on this. My main goal was to help prevent the diff headers from blending in too much with the background, which I feel they do. But you're right, it does then appear too much like replace lines. I'll play with this, maybe darken the grey just a tad.

  3. 
      
chipx86
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. 
      
  2. reviewboard/static/rb/css/defs.less (Diff revision 3)
     
     
     
    Show all issues

    Since we're touching these, can we get these moved over to colors.less with named definitions? Same for the other explicit color codes you're editing.

    1. I go over this a bit in the description, but I'm purposefully avoiding this for now because this is all going to be overhauled after 6.0. I have pending work here, but it's not ready to land, and anything I do to move this over is going to be erased later anyway. Every single color reference in the UI is going to be overhauled as part of that work, and will be moving toward intermediary definitions rather than direct #rb-ns-ui.colors() references, so I'd like to do that as part of that task instead.

    2. I'll make an attempt where it makes sense.

  3. Show all issues

    Looks like there's an extra blank line here?

  4. 
      
chipx86
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (23623a4)
Loading...