1988296 - Convert panel variables into design tokens

Status: RESOLVED FIXED

(Toolkit :: Themes, task, P3)

Description

[Jules Simplicio [:jules]]

We're beginning to add more design tokens for the chrome, and panels are a good candidate. Through tokens, we can handle their mode and theming and begin to integrate more with chrome styles per our meta 1987677.

To turn some of these into tokens, we need to, per our plan:

a. Identify all locations that variable is used
b. Identify its purpose
c. Integrate it into the design tokens system
d. Add documentation on how it is intended to be used (where do we put docs?)
e. Replace all uses with the token

Panel variables we can turn into tokens for tokens-platform.css:

Panel

  --panel-background
  --panel-color
  --panel-border-color
  --panel-border-radius
  --panel-padding
  --panel-shadow-margin

  --panel-separator-padding-vertical
  --panel-separator-padding-horizontal
  --panel-separator-padding

  --panel-subview-body-padding-block
  --panel-subview-body-padding-inline
  --panel-subview-body-padding

  --panel-and-palette-icon-size

Panel banner

 --panel-banner-item-color 
 --panel-banner-item-background-color 
 --panel-banner-item-hover-bgcolor 
 --panel-banner-item-active-bgcolor 
 --panel-banner-item-update-supported-bgcolor 
 --panel-banner-item-info-icon-bgcolor 

Please note because panels are themeable and also sometimes depend on OS style, so these variables show up in: PanelUI-shared.css, global-shared.css, popup.css

Arrowpanel and themeable panel variables

Ultimately, the variables above that you see for panel background, border, and text color point to these below because they point to theme manifest variables and respond to addon themes. Since part of the goal of tokenizing the chrome is looking into integrating with theming system, I figure I'd note that. We could benefit from dropping 'arrowpanel' and preferring a different prefix as it is semantically disconnected from such feature (which we also happen to not really use anymore beyond onboarding callouts).

--arrowpanel-background // same as `popup` theme manifest variable 
--arrowpanel-color // same as `popup_text` theme manifest variable
--arrowpanel-border-color // same as `popup_border` theme manifest variable

--arrowpanel-border-radius
--arrowpanel-padding
--arrowpanel-shadow-margin
--arrowpanel-menuitem-border-radius
--arrowpanel-menuitem-margin-block
--arrowpanel-menuitem-margin-inline
--arrowpanel-menuitem-margin
--arrowpanel-menuitem-padding-block
--arrowpanel-menuitem-padding-inline
--arrowpanel-menuitem-padding

Menu item variables used in panel

  --menuitem-border-radius
  --menuitem-padding 
  --menuitem-margin 

These shows up in menu.css, global-shared.css, panelUI-shared.css

Comment 1

[Jules Simplicio [:jules]]

More details per our last discussion:

We will create a panel.css under /design-system that will house panel styling tokens and classes used across the panel files: panelUI-shared.css, popup.css, global-shared.css, menu.css. And those new tokens will be aliases of existing tokens, for example: --panel-padding-block: 4px becomes --panel-padding-block: var(--space-xsmall). We specifically decided not to go with the JSON -> CSS route as a first step and simply provide a CSS file directly to try out our new flow ahead of making Style Dictionary conform to it.

There will inevitably be features re-using panel properties for custom UI, and reassigning them to different values to fit their specific needs, for example, see UITour.css and Confirmation Hint. We don't want to move such styles to the design system panel file, but it may be worth seeing if any of the values re-assigned have already been making use of the default variable values and if they could potentially also import panel.css.

Let's also support compact mode in panel.css

Example of a class that can go in panel.css with token and styles from popup.css, e.g.:

menupopup, 
panel {
  -moz-window-input-region-margin: var(--panel-shadow-margin);

  --background-color-canvas: var(--panel-background);
  --panel-background: Menu;
  --panel-color: MenuText;
  --panel-padding: var(--panel-padding-block) 0;
  /* This should match GetMenuCornerRadius() on macOS, or be overridden below */
  --panel-border-radius: 6px;
  --panel-padding-block: 4px;
  --panel-border-color: ThreeDShadow;
  --panel-width: initial;
  --panel-shadow-margin: 4px;
  --panel-shadow: 0 0 var(--panel-shadow-margin) hsla(0, 0%, 0%, 0.2);

   margin: calc(-1 * var(--panel-shadow-margin));

  @media (-moz-platform: linux) {
    @media (prefers-contrast) {
      --panel-border-color: color-mix(in srgb, currentColor 60%, transparent);
    }

    /* On some linux WMs we need to draw square and opaque menus because alpha is not available */
    @media not (-moz-gtk-csd-transparency-available) {
      --panel-shadow-margin: 0px !important;
      --panel-border-radius: 0px !important;

       opacity: 1 !important;
    }
}

We should continue to support platform differences in panel.css so all of the different OS theming logic can hopefully also live in one place.

Now here are some needs out of panelUI-shared.css:

.toolbar-menupopup {
  &,
  menupopup {
    appearance: none;
    border: none;
    font: menu;
    background-color: transparent;
    --panel-background: var(--arrowpanel-background);
    --panel-color: var(--arrowpanel-color);
    --panel-border-color: var(--arrowpanel-border-color);
    --panel-border-radius: var(--arrowpanel-border-radius);
    --panel-padding: var(--panel-subview-body-padding);
    --panel-shadow-margin: var(--arrowpanel-shadow-margin);
    --menuitem-border-radius: var(--arrowpanel-menuitem-border-radius);
    --menuitem-padding: var(--arrowpanel-menuitem-padding);
    --menuitem-margin: var(--arrowpanel-menuitem-margin);
  }

Here are a couple more examples of what would go in panel.css, but this time from global-shared.css:

:root {
  --arrowpanel-background: Field;
  --arrowpanel-color: FieldText;
  --arrowpanel-border-color: ThreeDShadow;
  --arrowpanel-border-radius: 8px;
  --arrowpanel-dimmed: color-mix(in srgb, currentColor 17%, transparent);
  --arrowpanel-dimmed-further: color-mix(in srgb, currentColor 30%, transparent);
  --arrowpanel-padding: 16px;
  --arrowpanel-shadow-margin: 4px;
  --arrowpanel-menuitem-border-radius: 4px;
  --arrowpanel-menuitem-margin-block: 0px;
  --arrowpanel-menuitem-margin-inline: 8px;
  --arrowpanel-menuitem-margin: var(--arrowpanel-menuitem-margin-block) var(--arrowpanel-menuitem-margin-inline);
  --arrowpanel-menuitem-padding-block: 8px;
  --arrowpanel-menuitem-padding-inline: 8px;
  --arrowpanel-menuitem-padding: var(--arrowpanel-menuitem-padding-block) var(--arrowpanel-menuitem-padding-inline);

  @media (-moz-platform: macos) {
    /* On macOS we use native shadows via -moz-window-shadow */
    --arrowpanel-shadow-margin: 0px;
  }
}

and (which coincides with the needs above from PanelUI-shared.css):

:is(menupopup, panel):where([type="arrow"]) {
  --panel-background: var(--arrowpanel-background);
  --panel-color: var(--arrowpanel-color);
  --panel-border-color: var(--arrowpanel-border-color);
  --panel-border-radius: var(--arrowpanel-border-radius);
  --panel-padding: var(--arrowpanel-padding);
  --panel-shadow-margin: var(--arrowpanel-shadow-margin);
  --menuitem-border-radius: var(--arrowpanel-menuitem-border-radius);
  --menuitem-padding: var(--arrowpanel-menuitem-padding);
  --menuitem-margin: var(--arrowpanel-menuitem-margin);
}

Comment 2

[oarnesto]

Attachment: Bug 1988296 - Extract panel variables to `panels.css` r=#reusable-components-reviewers-rotation

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2d5d13a4ed16
https://hg.mozilla.org/integration/autoland/rev/ab18e0e7e76a
Extract panel variables to `panels.css` r=desktop-theme-reviewers,akulyk,emilio

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ac536334e686
https://hg.mozilla.org/integration/autoland/rev/a4338ac74716
Revert "Bug 1988296 - Extract panel variables to `panels.css` r=desktop-theme-reviewers,akulyk,emilio" for causing mochitest failures @ layout/xul/test/test_bug467442.xhtml

Comment 5

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures @ layout/xul/test/test_bug467442.xhtml

Backout link: https://hg.mozilla.org/integration/autoland/rev/a4338ac747162bc9b392719ddd2cd1332a08ff08

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | layout/xul/test/test_bug467442.xhtml

also these failures:

https://treeherder.mozilla.org/logviewer?job_id=546188533&repo=autoland&task=VrtB12D-QRqiMvkldeRaCA.0&lineNumber=10031

https://treeherder.mozilla.org/logviewer?job_id=546177587&repo=autoland&task=LqVjtH5oR4SKBUKEUEdFJQ.0&lineNumber=7876

https://treeherder.mozilla.org/logviewer?job_id=546188772&repo=autoland&task=NEGEMBiTQZCgWzFuPhY4-A.0&lineNumber=2309

Comment 6

[oarnesto]

Test failures caused by rounding errors from design tokens calculated from rem values.

Resolved in latest build: https://treeherder.mozilla.org/jobs?repo=try&revision=f797f7d68b57bf4da47f724cacc9de7ae4b26845

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/08f82ea5a422
https://hg.mozilla.org/integration/autoland/rev/b4ffdd826bfe
Extract panel variables to `panels.css` r=desktop-theme-reviewers,akulyk,emilio

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/b4ffdd826bfe

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/90905d69f24b
https://hg.mozilla.org/integration/autoland/rev/b5a8cd575b38
Revert "Bug 1988296 - Extract panel variables to `panels.css` r=desktop-theme-reviewers,akulyk,emilio" for causing mochitests failures in browser_report_preview.js.

Comment 10

[Serban Stanca [:SerbanS]]

Reverted this because it was causing mochitests failures in browser_report_preview.js.

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/reportbrokensite/test/browser/browser_report_preview.js | testPreview - Test timed out

Comment 11

[pstanciu]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/b5a8cd575b38

Comment 12

[:Gijs (he/him)]

Attachment: Screenshot with patch

Note that not only did this fail tests, on the nightlies with this patch the hamburger menu looks wrong on macOS (vertical padding/spacing is too short). Please fix this as well before relanding.

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bookmark toolbar folder size

Is it expected that the UI size of the bookmark toolbar folder popup is modified by this patch?
This makes the UI more dense for all cases, and the compact (browser.uidensity==1) case is really dense compared to the previous style.

The commit message sounds like the intent here is only to move the definition, so I suppose it shouldn't change the actual layout?

Comment 14

[oarnesto]

To avoid rounding errors and the unintended layout changes mentioned above, we will incorporate the added non-relative dimension tokens from 1988871.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/fa5f29fa9778
https://hg.mozilla.org/integration/autoland/rev/0c3c2d3174da
Extract panel variables to `panels.css` r=desktop-theme-reviewers,akulyk,emilio

Comment 16

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0c3c2d3174da