Skip to content
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

Parse all branding colors from appstream (elementary#2140) #2226

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

edwood-grant
Copy link
Contributor

Short summary

Aims to fix #2140

This will parse all the branding possibilities of the metadata info according to the freedesktop docs here.

The branding tag has a must-have color tag with primary_color attribute (always set its value to primary), with an optional scheme_preference attribute which shows a preference for a particular color scheme (withe light or dark) and this should take precedence.

But, as the issue says, there can be multiple setups here, and this PR aims to account for all of these in the following order:

  • If the color tag has a scheme_preference, either set to light or dark, it should be taken as precedent, according to the desktop current color scheme (via gtk_application_prefer_dark_theme).
  • If not, look for the color tag that has no scheme_preference (i.e. unknown). This was the original call.
  • If the last call returns null, use the local color scheme setting.

Tests

Apps that I tested with this change:

  • Reco: Has all color branding setups (light, dark and without scheme_preference attribute). Different colors per tag.
  • KonbuCase: Only has light and dark branding tags. Different colors per tag.
  • Larawan: I believe it has no branding tag of any kind.
  • Mail: Has all tags, but all with the same color.

I couldn't find an application that had only the no scheme_preference attribute, but it was the default original call, so it should cover that case as well.

italo-capasso added 2 commits December 8, 2024 21:58
This will attempt to find if the package branding has a  scheme_preference attribute, then select the appropiate primary color matching the prefer dark theme or not.
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your branch! This looks really good besides one problem: it caches the brand color so if you change between light and dark mode you get the old color even if you exit and re-enter the page.

I think the way to fix this would be to cache each color separately and then only check if its null after we figure out which color we're looking for

This ensures that the correct primary color will always get updated when asking for a color even when changing the style mid-app-running.
@edwood-grant
Copy link
Contributor Author

edwood-grant commented Dec 9, 2024

Ah, I see, got it!

So what I did now was to create separate color values for each primary color as private members, that is:

  • Primary light
  • Primary Dark
  • Primary Unknown (the one without any scheme preference)
  • Fallback (when everything fails, the settings appcenter primary)

This is encapsulated in a private method (cache_primary_colors), which runs before asking for available colors. This should guarantee it to work every time you switch to dark mode and then enter/exit the page.

Hopefully this works better! 👍

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works here!

@danirabbit danirabbit merged commit e0641dc into elementary:main Dec 10, 2024
4 checks passed
@edwood-grant edwood-grant deleted the italo-capasso/issue-2140 branch December 17, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theme specific branding colors
2 participants