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

feat: add theme color support #121

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

iamahuman
Copy link

Allow specifying theme-color according to the current dark/light mode configuration.

Implementing this outside of color-mode-module has the following caveats:

  1. The user may have set custom color mode via localStorage. Since the server or renderer cannot know this, it cannot emit appropriate <meta name="theme-color" content="..."> tags in the raw HTML. If we always used the system color mode, we could use the media="..." attribute, but this does not apply to our case.
  2. Implementing in nuxt init hook leads to title bar flicker, since we have to wait until Nuxt finishes initializing before we could add the theme-color meta tag.
  3. Injecting a custom script tag may not work reliably if the developer cannot control the order of script inclusion. Also, if the developer uses custom 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 own theme-color setting mechanism despite the caveats.

@iamahuman iamahuman changed the title Feature: add theme color support feat: add theme color support Feb 9, 2022
@Atinux Atinux requested a review from danielroe February 9, 2022 17:27
@Atinux
Copy link
Contributor

Atinux commented Mar 31, 2022

@iamahuman do you mind opening a new PR or trying to solve these conflicts?

@ram-you
Copy link

ram-you commented Jun 1, 2022

Hi @iamahuman and @Atinux .
is this PR going on ?

@danielroe
Copy link
Collaborator

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?

@P4sca1
Copy link

P4sca1 commented Oct 12, 2022

@ram-you @danielroe I see the following problems with this PR:

  1. When the user switches the color mode, or the color mode is set to system and the system changes the color mode, the changes are not reflected on the page until it is reloaded.
  2. I cannot see how this fixes the problems 1 and 2. The theme-color is only set on the client. When using SSR, the meta tag is not set on initial load, still causing a flash of the theme color.

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.

@Arecsu
Copy link

Arecsu commented Aug 17, 2023

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 pnpm run dev playground. However, there's one problem I think is critical to address. I built the module and wanted to use it in my blog. But the lodash template got the quotes wrong and resulted in errors.

Here is the line in the code:

const metaThemeColors = JSON.parse('<%= options.themeColors %>')

And translated in the script.min.js:

i=JSON.parse("<%= options.themeColors %>")

Which, in the runtime, results in:

i=JSON.parse("{"dark":"#091a28","light":"#f3f5f4"}")

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.

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

Successfully merging this pull request may close these issues.

None yet

7 participants