Preview doesn't respond to light / dark theme changes in the tutorial

noam-honig commented 3 months ago

Describe the bug

In my tutorial, I want the preview window to react to changes in dark/light button that are available on the top bar - currently, the code and md change, but not my site

Would love some guidance

Link to a StackBlitz project which shows the error

No response

Steps to reproduce

In a new tutorial, click the light/dark theme and see that it doesn't affect the preview window. Even when I added media queries, it didn't respond:

/* Dark Mode Styles */
@media (prefers-color-scheme: dark) {
  body {
    background-color: #121212;
    color: #e0e0e0;
  }
}

Expected behavior

I would like a way to be aware in the preview of the light/dark change of the tutorial

Screenshots

No response

Platform

TutorialKit version: [e.g. 1.0.1]
OS: [e.g. macOS, Windows, Linux]
Browser: [e.g. Chrome, Safari, Firefox]
Version: [e.g. 91.1]

Additional context

No response

AriPerkkio commented 3 months ago

We've had some discussions about this earlier with @Nemikolh.

The way how learn.svelte does this is:

Add theme as query parameter in iframe.src https://github.com/sveltejs/learn.svelte.dev/blob/c1e1157f9f137fe63e2f9915a5453af1ccaad5ce/src/routes/tutorial/%5Bslug%5D/Output.svelte#L41-L52
Read query parameter inside iframe and set CSS class in HTML: https://github.com/sveltejs/learn.svelte.dev/blob/c1e1157f9f137fe63e2f9915a5453af1ccaad5ce/content/tutorial/common/src/app.html#L261-L266

I don't think there's a way to actually affect prefers-color-scheme of the iframe. 🤔

With TutorialKit it's not that simple, as end-users control what is running in the preview. @Nemikolh any new ideas how to solve this?

EricSimons commented 3 months ago

Another option might be for TutorialKit to postMessage the iframe to notify it whenever prefers-color-scheme changes, and though will still require you to capture that within the iframe yourself to apply it.

There's also a new WC API option @Nemikolh is working on for programmatically injecting scripts into all devservers which TutorialKit would be able to leverage for doing this automatically, but TBD on when that's going to land (hopefully in the next few weeks iirc).

Nemikolh commented 3 months ago

Hey everyone! :wave: Really interesting conversation! :smiley:

I think this is definitely something we will try to solve at the WebContainer API level under a flag so that everyone that use the WebContainer API can enjoy that feature.

I believe we might be able to solve this using the color-scheme CSS property and by patching window.matchMedia.

However, I haven't yet experimented with the idea and would need to double-check to be sure.

Also like @EricSimons said, we talked about having an API to easily inject scripts in previews. This is however very TBD at this point.

eric-burel commented 2 months ago

I think matching the prefers-color-scheme property, and allowing to manually switch from dark to light mode should be treated as slightly different issues.

The color-scheme property seems to allow forcing the use of the browser default light or dark theme, but doesn't affect the prefers-color-scheme media query, or at least not systematically.

In this this demo the "light" theme media query is still matched despite using a dark color-scheme at the root and on the button itself. However the light-dark function is affected by the root color-scheme. So using color-scheme attribute only works partially, you still need to apply an explicit top-level "dark" class at some point.

Note that the documentation of prefers-color-scheme seems to state that color-scheme does affect the media query, and shows an example here.

I've crafted a third demo, where the color-scheme property... seems to affect the color-scheme media query only partially. It seems that if "dark" and "light" media query are used together, color-scheme has no effect anymore, the "light" media query is stronger than the "dark" one.

To sum it up:

if you define only a default light style (without any media query) and use prefers-color-scheme: dark to define an alternate dark style, then color-scheme: dark will be respected
but you use both prefers-color-scheme: dark and prefers-color-scheme: light, and user is preferring light color-scheme, then color-scheme: dark has no effect. User preference has an higher priority over "color-scheme" in this case.
so only a "dark" class is a truly reliable way to enforce a toggleable light and dark style via a ".dark" selector instead of a media query

Tested on Chrome and Firefox. This might be a bug.

To my best knowledge you cannot alter the prefers-color-scheme media query with JavaScript either (which is annoying for unit tests), but this would need double checking.

I think patching matchMedia has no effect on the CSS rendering, and it's probably not a good idea as it would change the behaviour of this function which is supposed to always reflect user preferences (color-scheme doesn't seem to impact it at all, all the more that this attribute can be element-specific too and matchMedia is always global).

Fun fact: it makes it surprisingly difficult to implement a dark/light toggle in a webcontainer because there is a very strong isolation of CSS selector inheritance so a top-level ".dark" is of no use (or at least I failed to use it last time I've checked). Ironically the CSS styles themselves are less isolated and some inheritance is possible.

Nemikolh commented 2 months ago

Thanks for the detailed write up @eric-burel! And my apologies for not having sent an update regarding this issue and our current plan.

We are not going to do it using color-scheme and a matchMedia patch. When we discussed it, some raised similar concerns as you and after quick experiments it turned out to not be a very good idea.

Ou current plan is to expose at the WC API level a function, maybe called setPreviewRootAttributes that when called with:

webcontainer.setPreviewsRootAttributes({
  class: 'dark',
});

It will update all iframes <html> element to include that new attribute and newly created iframes will have those attributes set. This should be pretty flexible as it allow to set any attribute to any value (style could even be set as well if one wanted to).

At the TutorialKit level, we will essentially call that function based on the value of the theme and expose it as previewClassName property that can be set. This will be opt-in to avoid breaking tutorial that don't need this.

Let me know what you think! :smiley:

stackblitz / tutorialkit