-
Notifications
You must be signed in to change notification settings - Fork 178
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
Support dark mode #769
base: master
Are you sure you want to change the base?
Support dark mode #769
Conversation
I just realized that the mouse has to be drawn in a way that accommodates dark mode, marking as draft. |
It already follows the system GTK theme, so does this make any useful difference?
|
The system GTK theme is not a supported way to set color schemes, this makes it so that the setting can be applied on GNOME 42 or later when using the Appearance panel in GNOME Settings and other desktop environments who support the xdg settings portal (KDE does for example). If you use GTK_THEME or similar then this will overwrite whatever the portal says so it will still do what you wanted. Note that using HdyStyleManager one is able to react to the theme and listen for changed and one can think in doing things like displaying the mouse svg in grey for example. This wasn't possible with env vars or older methods. EDIT: I added a commit to draw the svg in a more appropriate color, but I have no clue whats the correct css here. Maybe it is not even possible to do so? EDIT: I wasn't specific enough, the setting exposed in modern GNOME desktops will NOT set GTK_THEME or any similar flag, hence this app will appear as white no matter what if the user has dark mode in settings but hasn't manually set GTK_THEME or similar. Similar implementations (Elementary for example) might do the same. |
You can find more info at https://gitlab.gnome.org/GNOME/Initiatives/-/issues/32. |
@jimmac was the one to suggest this particular style of SVG, so best is probably to get some guidance here for colors in dark mode. |
I've filed an issue for the artwork and have been grinding on the dark variants as well as cleaning up the style bit. |
jimmac@823fcf9 shows the newly included dark assets also involved some fixes to the light assets. Would be nice to have the assets part of this pull request and tested working. |
b21e473
to
64f7e56
Compare
409827d
to
2ea8ff9
Compare
There are some errors in flake8 and |
4185710
to
2ea8ff9
Compare
@A6GibKm, you can the error by clicking The failing flake8 test runs without an error for me locally. |
same.
I get |
You probably configured the project with tests disabled, I suggest you change the option: Also, sorry, the build directory is |
I mean these are a few lines from the output:
are those errors and warnings the reason it is failing? if so, it is because I didn't adapt the test or because the svgs might lack something? |
I don't see any changes to the code that would need adaptation of the tests, so it's probably the problem with svgs. |
The line pointing to led0 doesn't have the id I filed a PR to bump the CI to Ubuntu 22.04 (#806), maybe that fixes the flake8 issues. |
- dark theme preference covered by `-dark.svg` assets - use colors based on the [gnome icon palette](https://developer.gnome.org/hig/reference/palette.html) See libratbag#792
Follows the system preference for dark mode. See https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Settings.
This bundles the correct libhandy
2ea8ff9
to
aec5170
Compare
Yeah, that fixed the flake8 side. |
just saw that, a bit late but then again, I have the great excuse of having been away :) please don't bump the required version. IIRC 18.04 lts is still on 3.6 and the walrus operator alone isn't really enough of a feature to justify it. Also, quick google suggests the walrus was introduced in 3.8 |
Ok, fair enough, ill remove that. |
Ubuntu 18.04 LTS is EOL now, so probably not worth holding back this PR. Would be nice to see this merged :) |
Is there anything holding this PR back from being merged? (If it's helpful, I could open up a rebased PR if @A6GibKm doesn't have the time to rebase it.) |
I can rebase today, but I do remember there was something fishy somewhere (perhaps fixed by the latest changes). |
In the quest to fix the graphic assets stylistically and keep a somewhat consistent canvas size, I've touched pretty much every single asset. While they are now great from the stylistic point of view now, I broke some of the assets in the technical aspect. The svgs depend on certain structure for the element hightlighting to work. So this requires going through the whole set and fix those warnings. However it's a moving target and very likely more assets have been introduced in the meantime. I really ran out of energy for this the whack-a-mole game :( |
Follows the system preference for dark mode.
See https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Settings.