-
Notifications
You must be signed in to change notification settings - Fork 50
feat(plugin-css): Add support for color-scheme property
#550
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
feat(plugin-css): Add support for color-scheme property
#550
Conversation
🦋 Changeset detectedLatest commit: 9b3aaa6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
💯 This is fantastic, thank you! If you could be so kind as to make a |
cb3947d to
58b0f15
Compare
|
Sure! |
| "@media (prefers-color-scheme: dark)", | ||
| '[data-mode="dark"]', | ||
| ], | ||
| scheme: "dark", // Optional: set color-scheme for this mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: what happens if the baseScheme is set to e.g. only light and scheme dark is passed? Or in other words, if an invalid scheme is passed?
Would it be useful to write some config validation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, I would leave this to user responsibility (and the valid properties might change over time which brings maintenance burden).
I don't know the library well enough yet to catch the spirit, are config values validated elsewhere?
From a quick test in the tokens package, it doesn't look like there's any validation:
// in modeSelectors
{
mode: 'light',
selectors: ['hello world', '[data-color-mode="light"][data-color-mode="light"]'],
},Outputs:
hello world,
[data-color-mode="light"][data-color-mode="light"] {
--tz-border-1: 1px solid var(--tz-color-border-1);That's totally ok and out of this library's scope to me.
But I'll let you evaluate if it's worth validating or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have an opinion myself, and just figured I'd raise the question to see what you thought.
You have a solid reasoning informing your choice, I agree with you wrt the maintenance burden. I'm just not sure how hard it would be for users to debug a human error in setting the config without validation Vs having immediate error feedback.
Closes terrazzoapp#549 Co-authored-by: Steve Dodier-Lazaro <[email protected]>
a4ea4fe to
9b3aaa6
Compare
Changes
Provide support for the
color-schemeCSS property by respectivley adding bothbaseSchemeandschemetoCSSPluginOptionsandModeSelector.Closes #549
How to Review
These new options shouldn't bring any breaking change as they are optional.