feat: enable mode-watcher to manage the theme-color meta tag

[ollema]

closes #40.

progress:

• refactored how variables are passed into the FOUC prevention snippet in src/lib/mode-watcher.svelte. FOUC prevention snippet should work based on my testing - both for existing functionality and for the theme-color setting
• improved demo app: src/routes/+page.svelte by adding derived stores that displays how the html element and the new meta element is updated
• added themeColors prop in the demo app: src/routes/+layout.svelte
• added pre-existing <meta name="theme-color" content="..." /> in src/app.html for the demo app, see below

todo:

• right now, based on the existing code in Add option to set theme-color property #40, this requires an existing <meta name="theme-color" content="..." /> entry since we check for it's existing before overwriting it. is this intentional or should we always write (create a meta element for theme-color if needed) if themeColors are set?
• the big one: I could not figure out how to pass props from the component to the stores, see TODOs in src/lib/stores.ts. could you help me out here @huntabyte - I'm searching for some pattern where props passed to a component could somehow be read from stores which are initialised somewhere else... do we perhaps need to add some helper function for creating stores/components? use context="module"? I'm not sure, I have not encountered this before
• tests, when/if the two above are resolved
• changeset, when/if the all of the above are resolved

[changeset-bot]

🦋 Changeset detected

Latest commit: 1b42b67

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mode-watcher	Minor

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

[huntabyte]

Hey @ollema, currently traveling this week but I'll try to find some time to check this out!

[huntabyte]

Hey @ollema, hit me up on Discord when you see this!

[ollema]

looks great! left some comments for further improvement (for either of us to fix, I can do it if you don't have time)

[ollema]

elem and te was chosen instead of more descriptive names for bundle size reasons. but maybe this was not needed, maybe this code is also minimized. will have to double check!

[huntabyte]

Yeah man these get minified. Descriptive names > short names (unless you're building a minifier 😉)

[ollema]

I could have sworn that this kind of hacky code where you include something that's stringified as a raw html string would not be minified but I am probably mistaken

[huntabyte]

if you can find rootEl in the client-side bundle I'll send you some $ 😂

[ollema]

haha sure 🤣

does this count?

this is from running pnpm build && pnpm preview. maybe this is not a legit test though?

other functions ARE minimized here but it seems like the FOUC snippet is still untouched with rootEl and the other variable names intact

[huntabyte]

No you're fine, I should have done a more thorough review before making bold claims like that 😂 It's because we are stringifying it, and then slapping that stringified version directly into the tag, vs calling the setInitialMode.toString() in there.

[ollema]

I thought so too, so I tried:

	{@html `<\u{73}cript nonce="%sveltekit.nonce%">(${setInitialMode.toString()})("dark");</script>`}

similar to skeleton, but that did not work it seems. still did not end up being minimized!

[ollema]

maybe the function needs to be defined in a .ts file and exported, let me try that

[ollema]

tried:

<script lang="ts">
	import { setInitialMode } from './mode.js';
</script>
...

<svelte:head>
	{@html `<\u{73}cript nonce="%sveltekit.nonce%">(${setInitialMode.toString()})("dark", undefined);</script>`}
</svelte:head>

with:

// mode.ts
export function setInitialMode(defaultMode: Mode, themeColors?: ThemeColors) {
	const rootEl = document.documentElement;
	const mode = localStorage.getItem('mode-watcher-mode') || defaultMode;
	const light =
		mode === 'light' ||
		(mode === 'system' && window.matchMedia('(prefers-color-scheme: light)').matches);

	rootEl.classList[light ? 'remove' : 'add']('dark');
	rootEl.style.colorScheme = light ? 'light' : 'dark';

	if (themeColors) {
		const themeMetaEl = document.querySelector('meta[name="theme-color"]');
		if (themeMetaEl) {
			themeMetaEl.setAttribute('content', mode === 'light' ? themeColors.light : themeColors.dark);
		}
	}

	localStorage.setItem('mode', mode);
}

and that still does not work...

[ollema]

I don't get it.

I copied the exact code from skeleton now, just to verify:

<svelte:head>
	<!-- Workaround for a svelte parsing error: https://github.com/sveltejs/eslint-plugin-svelte/issues/492 -->
	{@html `<\u{73}cript nonce="%sveltekit.nonce%">(${setInitialClassState.toString()})();</script>`}
</svelte:head>

and

// mode.ts
export function setInitialClassState() {
	const elemHtmlClasses = document.documentElement.classList;
	// Conditions
	const condLocalStorageUserPrefs = localStorage.getItem('modeUserPrefers') === 'false';
	const condLocalStorageUserPrefsExists = !('modeUserPrefers' in localStorage);
	const condMatchMedia = window.matchMedia('(prefers-color-scheme: dark)').matches;
	// Add/remove `.dark` class to HTML element
	if (condLocalStorageUserPrefs || (condLocalStorageUserPrefsExists && condMatchMedia)) {
		elemHtmlClasses.add('dark');
	} else {
		elemHtmlClasses.remove('dark');
	}
}

still not minified when running pnpm build && pnpm preview. I have no idea what's going on

[huntabyte]

Let me know once this is ready for re-review/merge!

[ollema]

Let me know once this is ready for re-review/merge!

should be ready now! all questions/concerns have been resolved I think except for the first one included in the original PR description:

right now, based on the existing code in #40, this requires an existing <meta name="theme-color" content="..." /> entry since we check for it's existing before overwriting it. is this intentional or should we always write (create a meta element for theme-color if needed) if themeColors are set?

I'm tempted to leave as is and solve with better documentation, I think that will be the least surprising option

[huntabyte]

Let me know once this is ready for re-review/merge!

should be ready now! all questions/concerns have been resolved I think except for the first one included in the original PR description:

right now, based on the existing code in #40, this requires an existing <meta name="theme-color" content="..." /> entry since we check for it's existing before overwriting it. is this intentional or should we always write (create a meta element for theme-color if needed) if themeColors are set?

I'm tempted to leave as is and solve with better documentation, I think that will be the least surprising option

I think we can create one if themeColors is provided as a prop. If we didn't it would be like us ensuring the user has a style or class on their root element already, which we don't.

The code in #40 is more pseudo I believe.

[huntabyte]

Was able to get the head tag to minify :) merging now!

[Sija]

There's a typo either in the implementation or docs, since the former use themeColors property, while the latter the themeColor (singular).

[ollema]

There's a typo either in the implementation or docs, since the former use themeColors property, while the latter the themeColor (singular).

thanks! fixed in #58