Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS Variables For Dark Mode #4241

Closed
JamieB-gu opened this issue Mar 14, 2022 · 6 comments
Closed

CSS Variables For Dark Mode #4241

JamieB-gu opened this issue Mar 14, 2022 · 6 comments
Labels

Comments

@JamieB-gu
Copy link
Contributor

JamieB-gu commented Mar 14, 2022

A rough proposal for using CSS variables to power dark mode. There's lots of prior art on this topic here: guardian/source#1060 - many/most of the ideas in here come from @oliverlloyd @SiAdcock @mxdvl and @iainjchambers-guardian.

Overview

Editorial Palette

The editorial palette already provides a framework for selecting semantic light and dark mode colours for a given piece of UI. Here's a simplified example of what the text API looks like:

const text = {
  headline,
  headlineDark,
  standfirst,
  standfirstDark,
  ...
}

CSS Variables

What if we were to leverage this to provide a group of CSS variables for each colour mode?

const lightPalette = css`
  --text-headline: ${text.headline(format)};
  --text-standfirst: ${text.standfirst(format)};
  --background-headline: ${background.headline(format)};
`

const darkPalette = css`
  --text-headline: ${text.headlineDark(format)};
  --text-standfirst: ${text.standfirstDark(format)};
  --background-headline: ${background.headlineDark(format)};
`

Note: This is for demo purposes, we may be able to reduce the boilerplate.

Components

Then in our components:

const styles = css`
  color: var(--text-headline);
  background-color: var(--background-headline);
`

const Headline = ({ copy }) =>
  <h1 css={styles}>{copy}</h1>

Note: Again this is for demo purposes, we may want to import these variable strings, instead of entering them manually in each place, for type safety. Or maybe that's overkill and this is fine.

Switching Modes

Now we simply activate either lightPalette or darkPalette at the top level of our app to have everything switch over automatically. We could do this via a media query (most likely option in apps):

const topLevelStyles = css`
  ${lightPalette}

  @media (prefers-color-scheme: dark) {
    ${darkPalette}
  }
`

Or via other mechanisms, like JS or a <link> tag (more likely in dotcom):

<link
  rel="stylesheet"
  href="light.css"
  media="(prefers-color-scheme: light)"
/>

<link
  rel="stylesheet"
  href="dark.css"
  media="(prefers-color-scheme: dark)"
/>

Storybook

There's currently a gap in our visual regression testing in Storybook: we're not testing dark mode. This is largely because our current dark mode implementation depends on the dark mode media query. The preference that powers this query can't be changed using the browser APIs, so we're stuck in light mode in storybook/Chromatic.

Switching to CSS variables would allow us to trigger light and dark mode simply by loading one of the two palettes of variables. We could configure one story to load the light palette and another to load the dark palette, thus getting around our existing problem.

Shared Components

Currently we have a need for shared components to only support dark mode some of the time, i.e. in some scenarios we can't have them provide dark mode styles even when the user is in dark mode. For common-rendering this will be the case for as long as dotcom doesn't support dark mode. For Source this may always be the case, as there may be some products that never support dark mode.

At the moment we're relying on a supportsDarkMode prop to tell components in common-rendering whether to include their dark mode styles. We've discussed also using this pattern in Source.

With the CSS variables approach this may no longer be necessary, as the components themselves may not need to worry about explicitly setting dark mode styles themselves. For common-rendering we can use the mechanism demonstrated in the Components section above; the nice thing about this is that it automatically works while dotcom doesn't support dark mode, because on that platform the variable will never be set to its dark variant. For Source we could do something like this (Source's Container component):

<Container
  element="div"
  backgroundColor="var(--background-headline)"
  borderColor="var(--border-headline)"
>
  <h1>A headline</h1>
</Container>

Advantages & Disadvantages

Advantages

  • Less CSS shipped to the client (one, top-level dark mode media query, versus many scattered throughout)
  • Simpler shared components in common-rendering and Source (no need for the supportsDarkMode prop, or built-in dark mode styles)
  • Provides a mechanism for getting dark mode into storybook (swapping between the different CSS variable groups)
  • Provides a mechanism for getting dark mode on dotcom
  • Handles the differing requirements of dotcom and the apps
    • Apps can swap CSS variable groups based on media query, dotcom can swap based on user toggle
    • Apps can ship both sets of variables in one bundle, dotcom can selectively load them

Disadvantages

  • More globals (components will now depend on globally defined CSS variables rather than their own props)
  • Browser support for CSS variables
@oliverlloyd
Copy link
Contributor

More globals (components will now depend on globally defined CSS variables rather than their own props)

Do you think this proposal could be made to work with editorialPalette / decidePalette? Would that then remove the need for globals? That might also be nice as it removes the need to refactor each file needing colour, you could just add a layer of abstraction inside the palette file

@JamieB-gu
Copy link
Contributor Author

JamieB-gu commented Mar 14, 2022

Do you think this proposal could be made to work with editorialPalette / decidePalette?

The mechanism itself would work with either yes. I'm hoping they'll eventually be the same thing, but it's definitely possible to add a bunch of dark mode stuff to decidePalette instead.

Would that then remove the need for globals?

I think the existence of globals is inherent to how this works? You'll be swapping the value that a CSS var points to at a global level, depending on the user's preference.

That might also be nice as it removes the need to refactor each file needing colour, you could just add a layer of abstraction inside the palette file

I think this may be what I was referring to when I mentioned importing the variable strings instead of inlining the var names? So you could have an extra level where you have an object something like:

const text = {
  headline: '--var(text-headline)',
  standfirst: '--var(text-standfirst)',
}

And import/use that at the component level. That would guarantee that you always use a colour that's defined.

@oliverlloyd
Copy link
Contributor

I think the existence of globals is inherent to how this works? You'll be swapping the value that a CSS var points to at a global level, depending on the user's preference.

I think we're saying the same thing 😄 So yes, technically a css variable is global but I was thinking if we keep their use limited to only the 'palette' file(s) then it's a sort of controlled global. We could perhaps enforce this with linting.

So, as you say, instead of

const text = {
  headline: ${neutral[0]},
  standfirst: ${neutral[46]},
}

have

const text = {
  headline: '--var(text-headline)',
  standfirst: '--var(text-standfirst)',
}

At this point it's a question of tokenisation. You could put the abstraction at different places (we could map the Source api to replace each value with a css var) or define more descriptive colours locally. Eventually, I can imagine this sort of thing should be lifted into Source.

I think the key is to not require any changes to the component code. This already uses palette and I guess doesn't care about dark mode. For full support we'd probably end up adding more colour values to the palette object but that's an extension of an existing pattern.

@JamieB-gu
Copy link
Contributor Author

I think we're saying the same thing 😄

👍

At this point it's a question of tokenisation. You could put the abstraction at different places (we could map the Source api to replace each value with a css var) or define more descriptive colours locally.

I think the difficulty I see with using Source colours (e.g. var(--opinion-500)) is that they don't have dark mode variants that share the same name. The core of the above solution is that the headline text colour is always var(--text-headline), and the value of --text-headline is swapped by your dark mode mechanism (e.g. media queries). This means the components don't have to worry so much about dark mode any more, amongst other things.

If we put Source colours into CSS variables I'm less clear about how you'd swap to the dark mode variants 🤔.

@mxdvl
Copy link
Contributor

mxdvl commented Jul 21, 2022

I would be for using tokens from decidePalette / editorialPalette over Source colours because they don’t have a concept of “dark” opinion[500], as highlighted by @JamieB-gu.

One issue that is not addressed here is what would be the format passed to the palette function? We can easily run it for the format of the current article, but in Dotcom we also have Cards for other articles, and then we would need to have tokens generated for each and every possible format.

@mxdvl
Copy link
Contributor

mxdvl commented Jan 11, 2024

This is the approach we’ve taken in #9181

@mxdvl mxdvl closed this as completed Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Todo (next priority)
Development

No branches or pull requests

3 participants