Skip to content

Flat Accent: A theme that follows the windows theme color#731

Merged
dremin merged 8 commits intocairoshell:masterfrom
Blastd:master
Dec 20, 2022
Merged

Flat Accent: A theme that follows the windows theme color#731
dremin merged 8 commits intocairoshell:masterfrom
Blastd:master

Conversation

@Blastd
Copy link
Contributor

@Blastd Blastd commented Oct 6, 2022

By getting the Windows's theme color through the method that I've explained here, this theme is pretty much the Flat theme, but with the addition of the theme color as a highlight for certain things.
By using a MarkupExtension, I've implemented the Opacity attribute in order to change the Opacity of the SolidColorBrush that the class returns (So that we can have different tones of the same color, such as in the Taskbar or the different button states)
immagine
immagine
immagine

The issue:
As there is not listener to any theme changes, an user might have to restart Cairo to change to a new hue

A listener has been added

@dremin
Copy link
Collaborator

dremin commented Oct 29, 2022

Nice! I'd like to get this merged with some improvements:

  • Adding a listener for changes, caching the value, and read from the cache. You could monitor the registry key HKCU\Software\Microsoft\Windows\DWM\AccentColor.
  • On Windows 7, the color ends up black. EnvironmentHelper should be used to check the OS, and it should probably fall back to the current Flat theme color for now since that OS doesn't have a similar concept of an accent color.

Honestly, this should probably be the default Flat theme; we could append 'Blue' or something similar to the current theme's name. We should also move much of the accent color code into ManagedShell so that other shells can benefit.

@Blastd
Copy link
Contributor Author

Blastd commented Oct 29, 2022

Checking for changes can be done through SystemEvents.UserPreferenceChanged.
I'll look for a way to get it to work on windows 7 in the near future, as of now I agree on the blue fallback

@Blastd
Copy link
Contributor Author

Blastd commented Oct 29, 2022

By using power toys and checking registry values
The colorizationColor hex value should represent the actual colorization
accentColor returns a darker value

Also another thing
Is setting the UserPreferenceChanged listener in CairoApplicationThemeService's constructor okay?
In the event by calling SetThemeFromSettings() it changes the color to the new theme's
that is surely less expensive than checking a registry value change.
As of right now I don't think caching the value should increase performance, the color is retrieved only when the MarkupExtension is loaded, and when UserPreference_changed is triggered

Blastd and others added 4 commits December 5, 2022 21:19
[*] Changed the behavior of the UserPreferenceChanged event:
      it will subscribe ONLY when there's a compatible theme that supports theme colors
[!] This version still lacks a debouncer for the event. Changing the theme might invoke the event multiple times (4/5 times)
[+] added a supporting class that watches for registry changes at the accent color
Copy link
Collaborator

@dremin dremin left a comment

Choose a reason for hiding this comment

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

This approach seems good to me! Just a couple tweaks left to avoid impacting performance for other themes.

…recording, manually set an event handler and disposing

[CairoApplicationThemeService]: Extracted the compatible theme checking into a separate method.
Now when a compatible theme is set, it will create a new Watcher and start catching events. If an incompatible theme is set. the previous watcher is disposed.
Copy link
Collaborator

@dremin dremin left a comment

Choose a reason for hiding this comment

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

Thanks!

@dremin dremin merged commit 90bb849 into cairoshell:master Dec 20, 2022
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.

2 participants