-
-
Notifications
You must be signed in to change notification settings - Fork 217
Add color scheme (dark and light mode) support #1595
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
|
It throws an exception for me And linux users without that portal, mac users and windows users will all see a non-functional setting in preferences, right? |
Are you perhaps using the fake dbus module? I'll add a dummy
The 'System' setting will not work in this case, but forcing the light and dark modes should work. This could be either left as is, or a non-compatible platform (fake dbus, windows, mac) could be detected and the system setting removed in that case. |
842666a to
c7529ee
Compare
|
Yes, I am using fake dbus. The setting should be removed for anyone who can't use it, otherwise we will get a lot of reports that it doesn't work. |
c7529ee to
2d12095
Compare
|
This should now have better chances of working without Dbus or Settings portal. There are 3 kinds of platforms this should work in:
In the last two cases the 'system' color scheme is interpreted as 'light' and the config key is also changed to 'light' in preferences window initialization. Unfortunately I can easily test only the case where Settings are available. |
|
So there is no way to detect I haven't yet reviewed the patch, but I would like to verify it works on Windows and Mac. If only PRs produced Mac builds, because the Windows build is currently not working. |
It's probably not possible to detect that this setting is set in the GTK config file (or not). It can be read at startup and set as default, but it would require an extra 'unset' state in the config key, so that it would only be made once. But this should only be made when a portal is not found, since the default in that case should be the 'system' setting. |
Assuming it is even possible to detect the value, the config key could default to |
Add 'ui.gtk.color_scheme' config variable with 'system', 'light' and 'dark' string values. Config observer in main.py changes the color to match the config. System color scheme is read from freedesktop.org Settings portal via Dbus. The setting can be changed with a combo box in the preferences window via new function connect_gtk_combo_box_text(). If Settings portal cannot be found, only 'light' and 'dark' options are available in preferences.
2d12095 to
92244d6
Compare
Good idea. Now this detects a non-default dark mode GtkSetting and uses it as default. The whole thing got a bit too complex for my taste, but at least it's backwards compatible. |
|
Works for me. The only problem is that it starts up briefly in the system color before it switches to the preference in gpodder. However, there is most likely no way around this, and anyone who can't easily configure 'settings.ini' probably won't mind a short white flash to get dark mode. I'll review the patch tomorrow. |
|
Thanks for implementing this. |
|
Great addition to gPodder, thanks a lot! I have some ideas regarding this:
|
Are you using Mac or Windows? The Linux build should have a system option, but Mac and Windows builds do not since gtk's settings is inside the build and probably difficult to edit. We also can't easily access a system setting on Mac and Windows without platform code for each one.
That is only possible by changing gtk's theme, on Linux. Mac and Windows users are stuck with the Adwaita theme included in the build. |
|
Thx for the background information. I am on Windows. |

Add 'ui.gtk.color_scheme' config variable with 'system', 'light' and 'dark' string values. Config observer in main.py changes the color to match the config.
System color scheme is read from freedesktop.org Settings portal via Dbus.
The setting can be changed with a combo box in the preferences window via new function connect_gtk_combo_box_text().
This fixes issue #1194, but only on Linux and with the Settings portal running.