Update the Unified Draft Banner for light and dark modes.

Review Request #13730 — Created April 15, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

The Unified Draft Banner now works correctly in both light and dark
modes. When on dark mode, we use a cool grey background instead of the
green background (as there is no dark green that really looks good).
When on light mode, the UI looks as it did before.

All the menus (draft mode, publish button, and Review menu) now use
Ink.MenuView, giving us a degree of consistency. We modify
Ink.MenuView's CSS variables to style the draft mode menu
appropriately, and also reference them for styling the top-level menu
items that open them.

The Review menu controller item now has an open effect, just like the
draft mode controller item did. This was missing before, but has been
added for consistency. We also no longer attempt to shift it around
using negative margins (which can have adverse effects on layout), but
rather shift the menu itself.

We use flexbox in more parts of the UI to help align things, which is
necessary for the newer drop-down handle icons from Ink (which are a
different size and don't have a baseline to align against).

There's also a fix for the initial state of the banner when there's
no draft. An assumption was made in onRender() that rendered would
be set in Spina, but that flag was set prematurely in prior releases.
The fix in Spina had therefore regressed the banner. We now issue a
force-update of state, telling it to ignore the rendered flag
initially.

There's still work to do for the embedded diff navigation pane, and the
Review menu needs updates for mobile (we shipped it broken).

Tested on light and dark modes on mobile and desktop.

Verified that mode switching, publish options, and review menu options
all worked.

Unit tests pass.

Summary ID
Update the Unified Draft Banner for light and dark modes.
The Unified Draft Banner now works correctly in both light and dark modes. When on dark mode, we use a cool grey background instead of the green background (as there is no dark green that really looks good). When on light mode, the UI looks as it did before. All the menus (draft mode, publish button, and Review menu) now use `Ink.MenuView`, giving us a degree of consistency. We modify `Ink.MenuView`'s CSS variables to style the draft mode menu appropriately, and also reference them for styling the top-level menu items that open them. The Review menu controller item now has an open effect, just like the draft mode controller item did. This was missing before, but has been added for consistency. We also no longer attempt to shift it around using negative margins (which can have adverse effects on layout), but rather shift the menu itself. We use flexbox in more parts of the UI to help align things, which is necessary for the newer drop-down handle icons from Ink (which are a different size and don't have a baseline to align against). There's still work to do for the embedded diff navigation pane, and the Review menu needs updates for mobile (we shipped it broken).
cf68accd08e14e218b26202a5eecf2d55c9f04a4

Description From Last Updated

This active color seems very bright and highly saturated, and I find it difficult to read the text.

daviddavid

This needs docs.

daviddavid
maubin
  1. Nice.

  2. 
      
david
  1. 
      
  2. Show all issues

    This active color seems very bright and highly saturated, and I find it difficult to read the text.

    1. We can adjust it later then. The goal is to go with the Primary Accent colors on interactive controls where possible, to keep styling consistent and to allow customization in a universal way (changing the CSS variables in a parent).

      This color is controlled in Ink, and is used for all MenuViews, so I can explore it there.

      (It's also basically the macOS color for menu items.)

    2. Addressing this over at /r/13744/.

  3. Show all issues

    This needs docs.

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

Status: Closed (submitted)

Change Summary:

Pushed to release-7.x (fd384a5)
Loading...