-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
WebUI: Add color scheme switcher #21613
WebUI: Add color scheme switcher #21613
Conversation
d63f6f8
to
3e05854
Compare
Honestly, this would look a lot better if you simply had a sun/moon toggle button. No need to take up all that space with text and a checkbox. I'd go with that instead and put the toggle at the bottom right corner. If you want to keep it using text, I'd put it under the view dropdown. Anything but the checkbox and text, which looks very out of place. |
I would vote for not inventing anything special here. Just make a regular selector in the settings. |
Do you mean in the Settings/Preferences dialog or in the menu (eg |
Its a humble request. Please merge this for webui ASAP. I have chronic floaters in my eye (what are floaters?) and I cant see well in situations like inverted dark mode. Seemingly, the current webui torrent list and rss list look like inverted dark mode and I am so lost when looking at the screen because I cant read. Currently, I am using a android client to connect to the local docker. Due to my setup, I am not able to downgrade as well. Thank you @sledgehammer999 @HanabishiRecca |
Wdym by "inverted dark mode"? And if you have issues with dark modes, why do you have the dark mode enabled in your system/browser? |
3e05854
to
8967b01
Compare
I have addressed the comments. |
9cbec54
to
e2fb7f3
Compare
f386261
to
784a8c7
Compare
if (colorScheme === 0) | ||
LocalPreferences.remove("color_scheme"); | ||
else if (colorScheme === 1) | ||
LocalPreferences.set("color_scheme", "light"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I use camelCase for the settings' key too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have rules for it. Snake case is fine for now.
I think I am done with the modifications now. |
There is still one issue present - white theme flash on page load when using a dark theme (and vice versa). I can guarantee users will complain about that right after release if not further changes are made. |
I am not well versed in the webui world. What's the preferred solution here for time run and subsequent user change? |
Which could have a large delay, depending on network connection speed. If you want to fire immediately after the page is rendered, use
|
I don't know if it advisable to do so but a solution is:
|
No, that's a terrible advice. Just use
But actually, as we use |
I can't really check it right now but IIRC by the time DOMContentLoaded will fire, the browser may already have painted page's background. Adding a tiny render blocking script in the head will work for sure. |
Applying the following patch I notice 2 things:
diff --git a/src/webui/www/private/scripts/client.js b/src/webui/www/private/scripts/client.js
index d3a8b783b..684c2cf95 100644
--- a/src/webui/www/private/scripts/client.js
+++ b/src/webui/www/private/scripts/client.js
@@ -25,6 +25,19 @@
"use strict";
+// Setup color scheme switching
+const updateColorScheme = () => {
+ const root = document.documentElement;
+ const colorScheme = LocalPreferences.get("color_scheme");
+ const validScheme = (colorScheme === "light") || (colorScheme === "dark");
+ const isDark = window.matchMedia("(prefers-color-scheme: dark)").matches;
+ root.classList.toggle("dark", ((!validScheme && isDark) || (colorScheme === "dark")));
+};
+
+const colorSchemeQuery = window.matchMedia("(prefers-color-scheme: dark)");
+colorSchemeQuery.addEventListener("change", updateColorScheme);
+updateColorScheme();
+
window.qBittorrent ??= {};
window.qBittorrent.Client ??= (() => {
const exports = () => {
@@ -1681,19 +1694,6 @@ window.addEventListener("load", () => {
window.qBittorrent.Cache.preferences.init();
window.qBittorrent.Cache.qbtVersion.init();
- // Setup color scheme switching
- const updateColorScheme = () => {
- const root = document.documentElement;
- const colorScheme = LocalPreferences.get("color_scheme");
- const validScheme = (colorScheme === "light") || (colorScheme === "dark");
- const isDark = window.matchMedia("(prefers-color-scheme: dark)").matches;
- root.classList.toggle("dark", ((!validScheme && isDark) || (colorScheme === "dark")));
- };
-
- const colorSchemeQuery = window.matchMedia("(prefers-color-scheme: dark)");
- colorSchemeQuery.addEventListener("change", updateColorScheme);
- updateColorScheme();
-
// switch to previously used tab
const previouslyUsedTab = LocalPreferences.get("selected_window_tab", "transfers");
switch (previouslyUsedTab) { |
That's a bit complicated topic yeah.
|
We could do it vice versa though: load the dark theme initially, then switch to the light. |
Another way: simply setting --- a/src/webui/www/private/index.html
+++ b/src/webui/www/private/index.html
@@ -1,5 +1,5 @@
<!DOCTYPE html>
-<html lang="${LANG}">
+<html lang="${LANG}" class="dark">
<head>
<meta charset="UTF-8"> |
@Chocobo1 should I follow @HanabishiRecca recommendation (apply dark class) or do you have something else to propose? |
I think it is fine. I would also add a code comment about avoiding the bright flash. |
537bf28
to
4ab3218
Compare
I added the |
if (colorScheme === 0) | ||
LocalPreferences.remove("color_scheme"); | ||
else if (colorScheme === 1) | ||
LocalPreferences.set("color_scheme", "light"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have rules for it. Snake case is fine for now.
4ab3218
to
13e3192
Compare
Hmm, seems like all iframe dialogs ( |
Addressed in a new PR: #21750 |
…witcher WebUI: Add color scheme switcher (v5_0_x) Bacport of #21613
Closes #21600
Sometimes I am bored and want to try new things. This time I wanted to code the WebUI.
If user hasn't explicitly set validly his preference -> Use system/browser preference and react to changes to it
If user has explicitly and validly set his preference -> Use the user's settings and don't react to system/browser preference change
Obligatory pic: