Filter screenshots by style and environment (round 2) #2235
+63
−16
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short summary
This aims for a second round to complete the work to complete #2210
This uses
XDG_SESSION_DESKTOP
and use either LIGHT or DARKColorSchemeKind
according togtk_application_prefer_dark_theme
.Then it gets the screenshot environment metadata (described here: https://www.freedesktop.org/software/appstream/docs/chap-Metadata.html#tag-screenshots) to filter screenshots that match according to priority of environment first, color scheme second.
This triggers every time you open the AppView, so you should be able to exit the Appview, then change the color scheme then go back and see changes.
The environment IDs that currently exist in AppStream can be found here: https://github.com/ximion/appstream/blob/main/data/desktop-style-ids.txt
Notice that dark style specific the
:dark
suffix, whereas light scheme simply does not. So I assignColorSchemeKind.LIGHT
instead ofColorSchemeKind.UNKNOWN
What I did was to get all screenshot in a temporary list and make two passes on the list:
The reason this is done this way is that the idea that any screenshot available is better than no screenshot. But of course we do want to have precedence on the matching desktop environment and the style, so we can technically get rid of screenshots that do not match if we know that there are existing matching ones in the list, hence the two passes.
I'm not sure if there is a better way to do this though, let me know if the PR makes sense or if it needs a better approach to the screenshots.
By the way I removed the filtering from the screenshots
foreach
loop and put everything in the constructor, so when it gets to thatforeach
loop everythig is already filtered out inscreenshots
. This should avoid clutter in theforeach
loop and separates concerns.Tests
Oddly enough, it's a bit difficult to test this, since there are not that many applications that use the environment tag. But doing a GitHub search, I could find some:
Harvey
Has the
pantheon
andpantheon:dark
environment tags. It should match properly between light and dark setups.Taxi
Has only one
pantheon
environment tag. It should show that one regardless of desktop color scheme.Nicotine+
Nicotine+ has
gnome
andgnome:Dark
screenshots. It will never match the pantheon environment, but should default to style and work with light/dark schemes.Ruffle
Similar to Nicotine+, it has
gnome
andgnome:dark
screenshots. There is only onegnome
screenshot, so in light color scheme, it should show only one screenshot.Synfig studio
This is the reverse of Ruffle, only one
gnome:dark
screenshot.Euterpe
Has only
gnome:dark
screenshots. This means that it should always show the dark screenshots regardless of color scheme style.These are the most representative applications that I could find, if you can find more, the better to test.