Use Ink colours for flags and comments.

Review Request #14195 — Created Oct. 10, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

This change adds new --rb-p-flag-draft-* and --rb-p-flag-published-*
definitions to our standard colour palette. These should be used for flags
that indicate whether an item is a draft or published. The colours either
exactly match the old ones that we used or are very close. We update the
following items to use these new definitions:

  • Comment flags
  • Comment tooltips
  • Image region comments
  • Datagrid draft labels
  • File attachment draft labels

This change also fixes a syntax mistake in the definition for
--rb-theme-rich-text-code-fg, and pulls out a couple variable
definitions for --rb-theme-rich-text-code-bg to make code blocks
in comment tooltips look nicer.

Looked at the comments, datagrid labels and file attachment labels on
darkmode and lightmode.

Summary ID
Use Ink colours for flags and comments.
This change adds new `--rb-p-flag-draft-*` and `--rb-p-flag-published-*` definitions to our standard colour palette. These should be used for flags that indicate whether an item is a draft or published. We update the following items to use these new definitions: * Comment flags * Comment tooltips * Image region comments * Datagrid draft labels * File attachment draft labels This change also fixes a syntax mistake in the definition for `--rb-theme-rich-text-code-fg`, and pulls out a couple variable definitions for `--rb-theme-rich-text-code-bg` to fix make code blocks in comment tooltips look nicer.
7011123c35140dda874c1a3e73410ffa9bf58305

Description From Last Updated

We can't redefine these variables to use CSS variables, because it's possible some third party might try to use LESS …

daviddavid

We can use var(--ink-u-border-radius-s) here.

daviddavid

We can use var(--ink-u-border-thin) here.

daviddavid

Instead of 1px can we use var(--ink-u-border-thin)?

daviddavid

Can you add a blank line here and before the next section, to preserve the two blankes per groupings of …

chipx86chipx86

We might want to make a variable that defines the border so call sites don't have to worry about the …

chipx86chipx86
david
  1. 
      
  2. Show all issues

    We can't redefine these variables to use CSS variables, because it's possible some third party might try to use LESS operators like lighten() on them. Instead, places where we use #rb-ns-ui.field-state-label should be changed to use the new CSS vars directly.

  3. 
      
maubin
david
  1. 
      
  2. Show all issues

    We can use var(--ink-u-border-radius-s) here.

  3. Show all issues

    We can use var(--ink-u-border-thin) here.

  4. Show all issues

    Instead of 1px can we use var(--ink-u-border-thin)?

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

    Can you add a blank line here and before the next section, to preserve the two blankes per groupings of variables?

  3. Show all issues

    We might want to make a variable that defines the border so call sites don't have to worry about the components of the border.

  4. 
      
maubin
chipx86
  1. Ship It!
  2. 
      
maubin
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (5dd1fb9)