-
Notifications
You must be signed in to change notification settings - Fork 112
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: add theme color support #121
Conversation
34a40ca
to
aa78a30
Compare
@iamahuman do you mind opening a new PR or trying to solve these conflicts? |
Hi @iamahuman and @atinux . |
aa78a30
to
59579cf
Compare
I've rebased and slightly tweaked implementation. I dropped dab64d8 as I couldn't see it was needed; what was the issue that caused you to add it @iamahuman? |
@ram-you @danielroe I see the following problems with this PR:
I'm currently using this: const lightThemeColor = '#ffffff'
const darkThemeColor = '#111827'
// Set theme-color based on color mode.
const colorMode = useColorMode()
useHead(() => ({
meta: [
{
name: 'theme-color',
key: 'system prefers light mode',
media: '(prefers-color-scheme: light)',
content: colorMode.preference === 'dark' ? darkThemeColor : lightThemeColor,
},
{
name: 'theme-color',
key: 'system prefers dark mode',
media: '(prefers-color-scheme: dark)',
content: colorMode.preference === 'light' ? lightThemeColor : darkThemeColor,
},
{
name: 'theme-color',
key: 'fallback for browsers without media support for theme-color',
content: colorMode.preference === 'dark' ? darkThemeColor : lightThemeColor,
},
],
})) This avoids flickering, at least when the preference is system and the media query on theme-color is supported, but also has flickering, when the system is in light mode and dark is preferred (and vice-versa). To solve the underlying issue, the server needs to know the preference of the user in advance. Therefore a cookie is needed instead of relying on localStore. However, cookie support has been removed in version 2. |
I've resolved the conflicts, made small changes and merged everything with the latest commits of color-mode → https://github.com/Arecsu/color-mode/commits/master In the playground, it works great. No flash of theme color visible, and user switching or system-wide color scheme changes work as well. Didn't test it in a SSR scenario though. Just the Here is the line in the code:
And translated in the
Which, in the runtime, results in:
Spent a lot of time on it. Tried a crazy number of things, but couldn't manage to solve it. Backticks, JSON.stringify, etc. For some reason, it gets the double quotes wrong in my blog (latest dependencies, just updated every package yesterday), but can't reproduce it in the playground. Didn't try to update to the latest dependencies in the playground though. |
Closing in favour of #206 |
Allow specifying theme-color according to the current dark/light mode configuration.
Implementing this outside of
color-mode-module
has the following caveats:<meta name="theme-color" content="...">
tags in the raw HTML. If we always used the system color mode, we could use themedia="..."
attribute, but this does not apply to our case.theme-color
meta tag.storageKey
, the developer also have to make sure the custom script always uses the same storage key.Making the theme-color support built-in solves all of the problems above.
Note: dynamic
theme-color
is out of scope, since there would be no elegant way to specify this. The user can still implement their owntheme-color
setting mechanism despite the caveats.