Add prefers-color-scheme support

[psvenk]

This should fix #559.

The cookie storage format has been changed from boolean ("true"/"false") to tri-state ("dark"/"light"/""), so that users without a cookie set will get dark mode if they have enabled the dark theme in their operating system. The code for handling the cookie state, along with the user's operating system theme, has been factored out into a new function update_mode, which is called both at window load and at the "storage" event listener, because the "storage" event listener is only trigerred when a change is made to the localStorage from another tab/window (for more info, see https://stackoverflow.com/a/4679754).

This has worked in my limited testing using Firefox with the "Dark Website Forcer" add-on, using the Docker install instructions on GNU+Linux.

[psvenk]

I just noticed that the "dark mode" checkbox in the preferences doesn't work if the user has not selected a preference for dark mode (i.e. the user has not clicked on the toggle) and goes into the preferences. This can be seen if the OS is in light mode and one goes to the preferences and checks "dark mode"—dark mode is enabled for a fraction of a second and then light mode takes effect again due to the system theme. I think a tri-state preference that affects both the cookie and the current appearance is the solution for that, mirroring what has been done in assets/js/themes.js.

[omarroth]

Very much appreciated! I'll hopefully have time to test this out properly sometime tomorrow.

As you mentioned I expect some handling server-side needs to be added before this is merged.

Rather than change the type for dark_mode, I think it makes sense to add it as a new field theme which overrides the current option, which would then allow any additional themes to be added as "theme": "x" in future.

[psvenk]

Thank you. I was just now trying to see if I could change the preferences page, but as you mentioned there is some server-side code (including a .ecr file—I am assuming that is the Crystal equivalent of embedded Ruby) that needs to be changed. I am not familiar with embedded Ruby or embedded Crystal and I saw that these files deal with the locale files, so I think that it would be best if someone more qualified than me writes the code for that.

As for the theme field, I think we are on the same page. Here is a quick mockup that I made for the frontend (from Firefox developer tools), with HTML source code in case it would be helpful:

	Before	After
Screenshot
Source code	Click to show <div class="pure-control-group"><label for="dark_mode">Dark mode: </label><input name="dark_mode" id="dark_mode" type="checkbox" checked=""></div>	Click to show <div class="pure-control-group"><label>Theme: </label><input id="theme_auto" name="theme" value="theme_auto" type="radio"><label for="theme_auto" style="text-align: left; width: auto;">Auto</label><input id="theme_light" name="theme" value="light_mode" type="radio"><label for="theme_light" style="text-align: left; width: auto;">Light</label><input id="theme_dark" name="theme" value="theme_dark" type="radio"><label for="theme_dark" style="text-align: left; width: auto;">Dark</label></div>

Maybe instead of the dark/light button in the top right, there could be a dropdown with the same theme options available from the settings.

[omarroth]

Apologies for the delay. I pushed the necessary changes for this to be supported with existing preferences, cookies, etc.

I encountered an issue when testing without localStorage set. To reproduce:

• Open new tab without cookies or localStorage
• Go to preferences and change theme to dark (without using the "Dark Website Forcer" addon)

The page will have the desired dark theme while loading, then immediately switch to light after themes.js has loaded, since dark_mode in localStorage is undefined.

[omarroth]

I think the only issue is when first visiting the site without prefers-color-scheme: dark. Otherwise this should be ready to merge 👍

[psvenk]

I'm sorry, I just now saw your comment detailing the issue with localStorage unset. It seems like this occurs because the "Dark mode" preference, stored in the Crystal backend, is not synchronized properly with the dark_mode preference in localStorage. Because this is not an issue in current omarroth:master (the status quo), I imagine that the frontend and backend are communicating to keep this preference synchronized, but I cannot find where this happens. Would you please point me to the specific lines of code?

[omarroth]

I imagine that the frontend and backend are communicating to keep this preference synchronized, but I cannot find where this happens.

When changing a user's theme, the client will send a /toggle_theme XHR and a storageEvent to other tabs to update the current theme. It may be necessary to update the /toggle_theme endpoint to support switching to a specific theme, although you may have to clarify what you mean here.

[psvenk]

Thank you for the information, and sorry for the delay. I have made some changes so that the dark-mode preference (on the preferences page) now defaults to "" and the client now updates localStorage with the value from the dark-mode preference on page load. The bug that you pointed out is gone now (at least on Firefox 68.0.1 on Arch Linux, private browsing, no add-ons enabled), and I have not found any other issues. Is this ready to merge?

[omarroth]

From some quick testing it LGTM. I'd like to test it out a bit more but otherwise I think this should be ready to merge.

[psvenk]

The above comments are comments that I had written previously in response to the review comments that the two of you had posted. I did not fully understand how GitHub's review system works and I had not noticed that these comments were "Pending" and that I had to "submit" the review to make them public. In any case, all of these review comments concern outdated code or are otherwise resolved.

[omarroth]

Thank you! ❤️

[github-actions]

This pull request has been automatically locked since there has not been any activity in it in the last 30 days. If you want to tell us about needed or wanted changes or if problems related to this code are discovered, feel free to open an issue or a new pull request.