-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add light-dark() support #26
Conversation
README.md
Outdated
@@ -53,6 +53,38 @@ By default (without classes on `html`), website will use browser dark/light | |||
theme. If user want to use dark theme, you set `html.is-dark` class. | |||
If user want to force light theme, you use `html.is-light`. | |||
|
|||
This plugin also supports the [light-dark()](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/light-dark) color function: |
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.
Too complex. Just add lines to the main example.
index.js
Outdated
@@ -9,6 +10,25 @@ function replaceAll(string, find, replace) { | |||
return string.replace(new RegExp(escapeRegExp(find), 'g'), replace) | |||
} | |||
|
|||
function addColorSchemeMedia(isDark, color, postcss, declaration) { |
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.
Let’s move postcss
at the end, since it is complex arg
index.js
Outdated
function addColorSchemeMedia(isDark, color, postcss, declaration) { | ||
let mediaQuery = postcss.atRule({ | ||
name: 'media', | ||
params: `(prefers-color-scheme: ${isDark ? 'dark' : 'light'})` |
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.
Space will not work if you apply it after minification.
Do we have it originally? If not, let’s just remove space.
index.js
Outdated
@@ -109,6 +129,21 @@ module.exports = (opts = {}) => { | |||
} | |||
} | |||
}, | |||
DeclarationExit: (declaration, { postcss }) => { | |||
if (!declaration.value.startsWith('light-dark(')) return |
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 am not sure that all decl should start from ligh-dark()
.
What about: border-color: black light-dark(black, white)
index.js
Outdated
DeclarationExit: (declaration, { postcss }) => { | ||
if (!declaration.value.startsWith('light-dark(')) return | ||
|
||
let matches = [...declaration.value.matchAll(LIGHT_DARK)][0] |
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.
Can we have multiple light-dark()
in the value?
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.
indeed, we might have something like radial-gradient()
which can include multiple light-dark()
we can also have light-dark()
inside strings, in which case we should not touch it
Co-authored-by: Andrey Sitnik <[email protected]>
Co-authored-by: Andrey Sitnik <[email protected]>
Thanks! Do you have Twitter and Mastodon to mention you? |
yes, I have Twitter: https://twitter.com/VladBrok99 |
Closes #25
If we try to convert code from this:
To this:
Then it will not respect the user's preferred color scheme specified via system settings.
So we also need to use
prefers-color-scheme
media query.This plugin already can transform
prefers-color-scheme
, so if we convert this:To this:
then the current plugin code will run and we will get the following: