-
Notifications
You must be signed in to change notification settings - Fork 34
Make server-side aware of dark mode setting #574
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
Conversation
This allows Rails to render the view with the correct theme. By doing this, a flicker of the wrong theme should not occur.
Pages are cached in CloudFlare. The goal is that the Rails app doesn't receive many requests. I'm unsure what Colby will think of this. |
In this PR, I fixed a similar issue with FontAwesome. #381 I think we could do the same thing -- move the JS out to its own pack that's not deferred and loads before the Or if not its own pack, maybe we make a new single "now" type pack that the icons and the theme are in, so that there's only 2 files. |
This is what I've done in my website: AlexWayfer/alexwayfer.name@65832f4 Summary:
|
The crux of the issue is how long it takes for the From what I see, Stimulus controllers are executed alphabetically. Renaming The thing to keep in mind is that some of the JS is progressive enhancement (clicking the search box does something, for example), and some of it is vital to the layout of the page (choosing the correct theme, adding icons to the HTML). You want the layout stuff to happen immediately, and the progressive enhancements (event listeners) to be deferred. The CSS downloads and gets processed fast enough. I would personally recommend against inlining any JS / CSS. When JS and CSS are in their own files, their contents can be cached with a really long cache expiration. When it's in the HTML, you end up wasting bandwidth redownloading something that you could have just cached, because the HTML has a much shorter cache duration (because it's volatile due to layout changes of a static URL). I'm not sure exactly how to solve this problem. If you move the theme switcher JS out of the When the theme switcher was made, I remember wishing there was a way to make it work with normal Here's the JS in question: Here's the corresponding PR: Here's my comment about wishing there was an "auto" option (though that would only work around this problem): |
Thanks for your feedback! I didn't consider that there might be a server-side cache. It looks like this approach isn't going to work out, so I'll close this and give it another shot soon, using the advice in your comments. |
👋 The really large JS file size is my fault. the TL;DR is when I added the REPL page, I added the monoco-editor JS library which included a ton of stuff I'm not using. I've addressed this in #577 But this doesn't solve the theme switcher causing the flickering if the JS file size get's big again, which I would like to resolve. It's really late here and I'll comment on this specifically in the next few days. |
Oh man, good call @colby-swandale |
This PR switches the dark mode setting to be stored in a cookie rather than in
localStorage
. The server-side can then bake-in themode-dark
class if the dark mode cookie is true, removing the flicker of light theme (shown in #573) before the JS runs!The theme is still checked on page load in JS, in case the theme is changed but the browser has cached the page with the wrong theme or anything like that. It will also move the old
localStorage
dark mode setting into a cookie, if one is found.Fixes #573.