FriendsOfFlarum / nightmode

🌙 Turn off the lights!
https://discuss.flarum.org/d/21492-friendsofflarum-night-mode
MIT License
33 stars 22 forks source link

Use LESS @config-dark-mode variable for night mode #36

Closed dsevillamartin closed 4 years ago

dsevillamartin commented 4 years ago

This is a bit WIP but it works right now. I have the code to make the body hidden while the CSS file is loading. This is needed to avoid loading 2 CSS files for automatic mode & per-device settings - do we want to load 2 files instead? not sure which is better for UX 🤔

This code is a nightmare. Please test.

This PR also makes it so admin side also has night mode - not by choice but oh well. The Assets class cannot differentiate between forum & admin.

Most code from Flarum\Content\Assets overriden class is copied - only changed stuff is populate & makeCss methods.

dsevillamartin commented 4 years ago

This PR also means that we won't have to maintain our shitty dark mode CSS any longer.

Ralkage commented 4 years ago

Been wanting this solution for a hot min, if anyone could of made it possible, it would be the mighty @datitisev 🙏

matteocontrini commented 4 years ago

I have the code to make the body hidden while the CSS file is loading. This is needed to avoid loading 2 CSS files for automatic mode & per-device settings - do we want to load 2 files instead? not sure which is better for UX 🤔

Could you clarify why the body needs to be hidden? Doesn't the Flarum UI get rendered after the CSS is loaded anyway?

dsevillamartin commented 4 years ago

@matteocontrini Two reasons

1) Flarum still renders a "Loading" text 2) Flarum could still renders the complete app before the CSS has loaded (browser won't wait for styles to load because they were added through the JS)

Normally, the whole page loads before US runs, correct. But that's because the stylesheet is linked in the header. The JS doesn't manually prevent rendering until CSS has loaded.

matteocontrini commented 4 years ago

I see. Both solutions seem to be quite hacky though...

Do you have an idea of how the rest of the world is implementing this?

Wouldn't it be a solution to swap the CSS file at runtime only when strictly necessary? Meaning that:

Are there disadvantages to this solution? The only thing I can think of is that there could be a white "flash" when loading a page, but doesn't that happen when you have the body hidden as well?

It's very likely that I'm missing something of how this works though... 😊

dsevillamartin commented 4 years ago

when the style needs to be set because of the device settings the style is swapped with JavaScript when the page is loaded

This would cause 2 CSS files to be loaded. Because you use "swapped" I assume you have a default one (e.g. light theme) and then if the per-device settings or automatic mode require dark theme, the dark theme is loaded. In this case, both light & dark theme stylesheets will be loaded, which could increase data usage & loading times.

The only thing I can think of is that there could be a white "flash" when loading a page, but doesn't that happen when you have the body hidden as well?

There would be a flash of light theme changing to dark theme (once dark theme stylesheet loads) if the per-device/automatic setting asks for dark theme.

What could be done is have a generic background color for light / dark theme for a replacement of "white flash" (when not loading either of the stylesheets) but that'd only look good with automatic, and not with a per-device day/night setting.

matteocontrini commented 4 years ago

In this case, both light & dark theme stylesheets will be loaded, which could increase data usage & loading times.

Ah yes you're right. But that would happen only the first time, then there would be appropriate local caching, I assume.

Between hiding the body and loading two files I would choose loading two files. This afterall should only affect those who use per-device settings (and only the first time), while in other cases the style could be set server-side. (I don't think this is what the code does currently, but it seems feasible.)

dsevillamartin commented 4 years ago

This afterall should only affect those who use per-device settings (and only the first time)

Also affects automatic mode.

while in other cases the style could be set server-side

This is literally what I'm introducing in this PR. For explicit day/night themes (not per-device or automatic), the stylesheet will be added through backend.

Between hiding the body and loading two files I would choose loading two files

Problem with this is that it'd still look off - the 2nd loaded stylesheet would take priority, so e.g. dark mode would be the one that shows first even if per-device/automatic decides light mode.

dsevillamartin commented 4 years ago

I've pushed some changes. Now, the per-device settings are stored in a cookie so that the web server can access them, and both CSS files are loaded when automatic mode is chosen.

Ideally, the browser would only load whichever meets the media query (because that's how the browser will choose which to actually render), but it seems that none of them do so right now.