skeletonlabs / skeleton

A complete design system and component solution, built on Tailwind.
https://skeleton.dev
MIT License
5k stars 317 forks source link

Lightswitch not applying persistent state to html #2598

Open f0o opened 7 months ago

f0o commented 7 months ago

Current Behavior

After Toggling Lightswitch and reloading the page, the page will return to the default state instead of the one selected by Lightswitch.

The code https://github.com/skeletonlabs/skeleton/blob/master/packages/skeleton/src/lib/utilities/LightSwitch/LightSwitch.svelte#L77 seems to be the culprit.

Copy-pasting the code itself into the JS Console will set the correct state into HTML but that oddly formed "fake" script tag will not trigger execution (at least on current Google Chrome).

Expected Behavior

https://github.com/skeletonlabs/skeleton/blob/master/packages/skeleton/src/lib/utilities/LightSwitch/LightSwitch.svelte#L77

Should execute correctly and set the class into html element

Steps To Reproduce

  1. Toggle Lightswitch
  2. Reload Page
  3. Observe the Lightswitch Icon being correct but the tailwind theme is still the default (such as dark but icon is light or viceversa)

Link to Reproduction / Stackblitz

No response

More Information

Google Chrome: Version 123.0.6312.106 (Official Build) (64-bit)

    "@floating-ui/dom": "^1.6.3",
    "@iconify/svelte": "^3.1.6",
    "@as203038/lg-protobuf": "file:../protobuf",
    "@skeletonlabs/skeleton": "2.9.0",
    "@skeletonlabs/tw-plugin": "0.3.1",
    "@sveltejs/adapter-auto": "^3.0.0",
    "@sveltejs/adapter-static": "^3.0.1",
    "@sveltejs/kit": "^2.0.0",
    "@sveltejs/vite-plugin-svelte": "^3.0.0",
    "@tailwindcss/forms": "0.5.7",
    "@tailwindcss/typography": "0.5.12",
    "@types/node": "20.12.4",
    "autoprefixer": "10.4.19",
    "postcss": "8.4.38",
    "svelte": "^4.2.7",
    "svelte-check": "^3.6.0",
    "tailwindcss": "3.4.3",
    "tslib": "^2.4.1",
    "typescript": "^5.0.0",
    "vite": "^5.0.3",
    "vite-plugin-tailwind-purgecss": "0.2.1"
f0o commented 7 months ago

Worth to note;

Manually including

<svelte:head>{@html '<script>(' + setInitialClassState.toString() + ')();</script>'}</svelte:head>

in +layout.svelte solves the issue at the expense of having the JS included twice (once with weird script tag and once correctly here)

//EDIT: Scratch that, it's non-deterministic - sometimes it works sometimes it doesnt.

//EDIT2:

Placing (function Gn(){const e=document.documentElement.classList,t=localStorage.getItem("modeUserPrefers")==="false",n=!("modeUserPrefers"in localStorage),r=window.matchMedia("(prefers-color-scheme: dark)").matches;t||n&&r?e.add("dark"):e.remove("dark")})(); into the OnMount will actually solve it.

Seems like a race-condition somewhere?

endigo9740 commented 7 months ago

@f0o can you share a few more details about your project config. Specifically:

  1. What adapter are you using?
  2. How is the app being deployed? (static to AWS S3, via Vercel, etc)
  3. Are using using the selector (aka class) or media (aka "OS") strategy for your app?

For example, under normal circumstances when using the default auto-adapter you should not need to insert the extra script tag in the page header.

We also provide this note when using the Lightswitch with the class-based strategy approach:

...Please note the Lightswitch component must be present on page load in order to operate. For best results, insert this in a fixed element that appears on every page.

So if the ilghtswitch is not part of the normal DOM tree on page load, then it can cause behavior similar to what you're reporting. For the Skeleton documentation site for example, this is part of the header. Even though it is "hidden" within a popup menu, but that menu is just visibly hidden, not removed from the DOM.

mattdavis90 commented 6 months ago

I can't speak for f0o but I'm experiencing the same issue. I'm using the adapter-static package but experience the same issue with adapter-auto. I'm running locally with pnpm run dev, using the class strategy.

I've copied the AppBar code straight from the Skeleton site so the LightSwitch should be in the DOM.

I can see the <script> tag in the head but it doesn't appear to be running - if I manually execute the code then it works as expected.

mattdavis90 commented 6 months ago

I've had success removing the svelte:head tags and moving the call to setInitialClassState() into the onMount block on line 66

f0o commented 6 months ago

I've entirely forgotten about this bugreport; terribly sorry about it.

I'm also using adapter-static but without SSR and the class selector.

That being said; I've been running it with my OnMount workaround since I opened the bugreport and although hacky it seems to be stable across all browsers.