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

Polyfill CSS light-dark() function to simplify theming #464

Open
necolas opened this issue Feb 23, 2024 · 10 comments
Open

Polyfill CSS light-dark() function to simplify theming #464

necolas opened this issue Feb 23, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@necolas
Copy link
Contributor

necolas commented Feb 23, 2024

Describe the feature request

https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/light-dark

We should look into how far we can polyfill this feature to help with simplifying how themes are defined without having to use media query wrappers in defineVars and createTheme

@necolas necolas added the enhancement New feature or request label Feb 23, 2024
@nmn
Copy link
Contributor

nmn commented Feb 24, 2024

I've been thinking about this for a while. Within theming APIs specifically, we could convert light-dark() into dark mode media queries automatically.

It should also be possible to define a function like this in user-space already:

const lightDark = (light: string, dark: string) => ({
  default: light,
  '@media (prefers-color-scheme: dark)': dark,
});

I've done some work to support such simple pure arrow functions usable while defining styles. In the immediate future, I'm looking to find and fix the issues when such functions are used and also improve the error messages to enforce the constraints on these functions.


One possibility might be to add stylex.lightDark() as a helper function that works similarly to stylex.firstThatWorks(). This would be trivial to add. Do you think this is a good idea?

@necolas
Copy link
Contributor Author

necolas commented Feb 24, 2024

I think if there is a safe "native" way to do something that fits within the StyleX constraints, we should prefer to try and polyfill that instead of adding new functions to the API.

The support for user-space functions would be cool for people who want custom abstractions, and could work as a stop-gap for native features we either a) haven't polyfilled yet, or b) don't want to polyfill until a spec has stabilized somewhat. I don't have a good sense of whether light-dark() is relatively stable (other than it being shipped in Firefox), but it certain feels more ergonomic than the MQs for the same purpose.

@nmn
Copy link
Contributor

nmn commented Feb 26, 2024

Polyfilling the CSS light-dark() should be trivial, will prioritize it.

Update: Some interesting news regarding light-dark(). LightningCSS can polyfill it natively so by adding LightningCSS we can get support for it for free.

However, it also works differently than @media (prefers-color-scheme: dark). Unless :root{ color-scheme: light dark } is set, it only uses the light-mode value. This should be OK.

But also, you can manually set color-scheme on any element and unlike the media query, it does affect which value the light-dark() function resolves to.

So overall, light-dark() seems to solve the problems of having to create separate light and dark themes. We can simply use light-dark() instead of the media queries globally and get more power for free.

@nmn
Copy link
Contributor

nmn commented Mar 4, 2024

RFC - The approach for polyfilling light-dark() correctly

I spent some time discussing this with @nitishmehrotra870 and we found a flaw with how LightningCSS polyfills light-dark().

Since the implementation would be a bit custom, I decided to share an RFC to explain the problems in the LightningCSS polyfill and how we plan to solve it.

Issue with LightningCSS Polyfill

LightningCSS relies on a "space hack" which involves setting a CSS variable value to a space or initial. This make a variable behave like a toggle. Here's an example:

This:

:root { color-scheme: light dark; }
.foo { color: light-dark(black, white); }

Would get transformed into this:

:root { --light-mode: initial; --dark-mode: ; }
@media (prefers-color-scheme: dark) {
  :root { --light-mode: ; --dark-mode: initial; }  
}
.foo { color: var(--light-mode, black) var(--dark-mode, white); }

Whichever variable is set to initial will cause the associated fallback value to be applied.
The variables set to ; are considered to be "defined", but they only add whitespace to final value of color.

This polyfill is already good enough when light-dark is used for CSS properties. However, when light-dark() is used to set the value of a CSS variable, this approach doesn't work as expected.

Consider if the original code looked like this:

:root { --light-mode: initial; --dark-mode: ; }
@media (prefers-color-scheme: dark) {
  :root { --light-mode: ; --dark-mode: initial; }  
}
.foo { --color: var(--light-mode, black) var(--dark-mode, white); }
.bar { color: var(--color); }

Consider the following transformed code:

:root { --light-mode: initial; --dark-mode: ; }
@media (prefers-color-scheme: dark) {
  :root { --light-mode: ; --dark-mode: initial; }  
}
.foo { --color: var(--light-mode, black) var(--dark-mode, white); }
.bar { color: var(--color); }

Here, the class bar picks the final color based on the color-scheme that was active for foo, not for bar. This is not how light-dark is supposed to work.

Proposed solution

The proposed solution is to detect every CSS variable that is ever set to a light-dark value and split it into two separate variables for light and dark respectively. And further, move the "space-hack" to where the variable is used instead.

Therefore the example above would be transformed to the following:

:root { --light-mode: initial; --dark-mode: ; }
@media (prefers-color-scheme: dark) {
  :root { --light-mode: ; --dark-mode: initial; }  
}
.foo { --color_light: black; --color_dark: white; }
.bar { color: var(--light-mode, var(--color_light)) var(--dark-mode, var(--color_dark)); }

This ensures that the light-dark function isn't resolved too early.

One Additional detail

In order to cover all edge-cases, it is also important to split variables that get a light-dark() function value indirectly.

e.g. if --color: light-dark() is used, and --other-var: var(--color) is used in the original CSS, then both --color and --other-var need to be split into _light and _dark variants. Therefore, detecting variables that can be set to light-dark will need to be done in a loop, over and over again, until there are no additional variables found.

Caveat

We will not be able to polyfill usages of light-dark() within dynamic styles as they're not known ahead of time.

Final Usage

Although there are a bunch of details to consider while implementing a polyfill for light-dark, the usage should be very simple.

  1. You will be able to use light-dark(val1, val2) just like any other value.
  2. It can be used within stylex.create styles and within theming APIs
  3. There will be no need to use @media (prefers-color-scheme: dark) anymore
  4. There will be no need to define separate "light", "dark" and "auto" themes
    1. Instead, you'd be able to define a single theme where the values are defined using light-dark
    2. You'd be able to set:
      • color-scheme: light for light mode
      • color-scheme: dark for dark mode
      • color-scheme: light dark for system
    3. color-scheme can be set on any element anywhere, and you can use it to mix and max light and dark mode within the same HTML page.

@necolas
Copy link
Contributor Author

necolas commented Mar 4, 2024

Might be worth getting some example numbers on how the different approaches influence bundle size of CSS and generated JS. And keep an eye out on the potential impact on build times of having to loop over variables as described.

@nmn
Copy link
Contributor

nmn commented Mar 5, 2024

I'll get some hard numbers as I start implementing the polyfill, but based on usage patterns.

Regarding CSS bundle size:

  • Without light-dark() support, we would be forced to create a light, dark and sometimes an auto theme. Assuming the best case scenario where defineVars is used to define the "auto" values as the default, with two createTheme calls for light and dark, we're looking at over 3x the cost of having to simply define variables.
  • With native support for light-dark() (in Firefox today and Chrome within a month or so), we cut that down to a third. light-dark() syntax is shorter than the media query, and the need for manual light and dark themes goes away entirely. Setting color-scheme is sufficient.
  • With the polyfill for light-dark(), the size of the variables would roughly double.

Therefore, a conservative estimate is that using light-dark() should result in approx 2/3 the size of variables in the final CSS bundle than not using it.


Regarding build times, I'm not too worried because in production the extra work only involves adding a polyfill to the generated CSS file. This file is already small enough and the process should be quite fast. Further, we can use the very fast LightningCSS if needed as well.

@fredrivett
Copy link
Contributor

Definite +1 on this, would be great to DRY up a sizeable amount of our code. At the mo we have to import the theme into every component, and duplicating styles like so:

styles = stylex.create({
  myThing: backgroundColor: "white",
});

darkStyles = stylex.create({
  myThing: backgroundColor: "black",
});

const MyComponent = () => {
  const theme = useTheme();

  return (
    <div {...stylex.props(styles.myThing, theme === "dark" && darkStyles.myThing)}>
      My thing
    </div>
  );
};

with light-dark this could be:

styles = stylex.create({
  myThing: backgroundColor: light-dark("white", "black"),
});

const MyComponent = () => (
  <div {...stylex.props(styles.myThing)}>My thing</div>
);

Naturally this gets more convoluted with the more variants, such as this in our Button component:

<button
  onClick={onClick}
  {...stylex.props(
    styles.button,
    size === "small" && styles.buttonSmall,
    variant === "outline" && styles.buttonOutline,
    variant === "subtle" && styles.buttonSubtle,
    theme === Theme.DARK &&
      variant === "outline" &&
      darkStyles.buttonOutline,
    theme === Theme.DARK && variant === "subtle" && darkStyles.buttonSubtle,
    style,
  )}
>
  {children}
</button>

Appreciate y'all can't get everything done at the drop of a hat and have other priorities too, just another small nudge as have been watching this one from the shadows and it seems to have gone a bit quieter recently.

@necolas
Copy link
Contributor Author

necolas commented Mar 29, 2024

If you read the theme API docs you should find that themes are a lot simpler than that and don't require conditions inside every component, only the root of the themed tree

@nmn
Copy link
Contributor

nmn commented Mar 31, 2024

@fredrivett Use the stylex.defineVars and stylex.createTheme APIs and use media queries instead.

@tounsoo
Copy link
Contributor

tounsoo commented Apr 3, 2024

I've been searching for this 😉 Will sub on this to keep up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants