-
Notifications
You must be signed in to change notification settings - Fork 309
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
Comments
I've been thinking about this for a while. Within theming APIs specifically, we could convert 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 |
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 |
Polyfilling the CSS Update: Some interesting news regarding However, it also works differently than But also, you can manually set So overall, |
RFC - The approach for polyfilling
|
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. |
I'll get some hard numbers as I start implementing the polyfill, but based on usage patterns. Regarding CSS bundle size:
Therefore, a conservative estimate is that using 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. |
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 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
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. |
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 |
@fredrivett Use the |
I've been searching for this 😉 Will sub on this to keep up |
I plan on writing a |
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
andcreateTheme
The text was updated successfully, but these errors were encountered: