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

Fluent2: Missing SystemAccentColorBrushes #10457

Open
robert-abeo opened this issue Feb 14, 2025 · 6 comments
Open

Fluent2: Missing SystemAccentColorBrushes #10457

robert-abeo opened this issue Feb 14, 2025 · 6 comments
Labels
Investigate Requires further investigation by the WPF team. Win 11 Theming

Comments

@robert-abeo
Copy link

Description

For improved usability of the Fluent theme I think Light, Dark and HC need new brush resources:

    <SolidColorBrush
        x:Key="SystemAccentColorBrush"
        Color="{StaticResource SystemAccentColor}" />
    <SolidColorBrush
        x:Key="SystemAccentColorDark1Brush"
        Color="{StaticResource SystemAccentColorDark1}" />
    <SolidColorBrush
        x:Key="SystemAccentColorDark2Brush"
        Color="{StaticResource SystemAccentColorDark2}" />
    <SolidColorBrush
        x:Key="SystemAccentColorDark3Brush"
        Color="{StaticResource SystemAccentColorDark3}" />
    <SolidColorBrush
        x:Key="SystemAccentColorLight1Brush"
        Color="{StaticResource SystemAccentColorLight1}" />
    <SolidColorBrush
        x:Key="SystemAccentColorLight2Brush"
        Color="{StaticResource SystemAccentColorLight2}" />
    <SolidColorBrush
        x:Key="SystemAccentColorLight3Brush"
        Color="{StaticResource SystemAccentColorLight3}" />

Reproduction Steps

See above

Expected behavior

See above

Actual behavior

See above

Regression?

No response

Known Workarounds

No response

Impact

No response

Configuration

No response

Other information

No response

@miloush
Copy link
Contributor

miloush commented Feb 14, 2025

What is the usability improvement, that you type {StaticResource SystemAccentColorDark2Brush} instead of {x:Static SystemColors.AccentColorDark2Brush}?

Do you not need them to be dynamic?

@siagupta0202 siagupta0202 added Win 11 Theming Investigate Requires further investigation by the WPF team. labels Feb 14, 2025
@robert-abeo
Copy link
Author

@miloush

What is the usability improvement

You misunderstand -- what are included right now are ONLY colors:

<!-- AccentColor Brushes -->
<DynamicResource x:Key="SystemAccentColor" ResourceKey="{x:Static SystemColors.AccentColorKey}" />
<DynamicResource x:Key="SystemAccentColorLight1" ResourceKey="{x:Static SystemColors.AccentColorLight1Key}" />
<DynamicResource x:Key="SystemAccentColorLight2" ResourceKey="{x:Static SystemColors.AccentColorLight2Key}" />
<DynamicResource x:Key="SystemAccentColorLight3" ResourceKey="{x:Static SystemColors.AccentColorLight3Key}" />
<DynamicResource x:Key="SystemAccentColorDark1" ResourceKey="{x:Static SystemColors.AccentColorDark1Key}" />
<DynamicResource x:Key="SystemAccentColorDark2" ResourceKey="{x:Static SystemColors.AccentColorDark2Key}" />
<DynamicResource x:Key="SystemAccentColorDark3" ResourceKey="{x:Static SystemColors.AccentColorDark3Key}" />

The vast majority of code we really need to use brushes instead (setting Background, Foreground, etc.). In fact the accent colors are listed under the brushes section so that's kind-of a mistake.

It is much easier to reference an existing brush than create a new SolidColorBrush every time an accent color is needed.

Do you not need them to be dynamic?

No, these are BASE resources. Base resources are completely swapped out for light/dark/hc. Only in the control styles would they need to be referenced as DynamicResource.

@miloush
Copy link
Contributor

miloush commented Feb 14, 2025

Well the colors are already DynamicResource, how are the brushes different? If the current system is to be followed, why not

 <DynamicResource x:Key="SystemAccentColorLight2Brush" ResourceKey="{x:Static SystemColors.AccentColorLight2BrushKey}" /> 

The accent color can change without theme change.

@robert-abeo
Copy link
Author

The accent color can change without theme change.

Yes, I suppose that's true:

If the current system is to be followed, why not
<DynamicResource x:Key="SystemAccentColorLight2Brush" ResourceKey="{x:Static SystemColors.AccentColorLight2BrushKey}" />

I do NOT think we should include Brushes in the SystemColors class. Aside from the name mismatch with the class itself it violates convention here.

The following in the Dark/Light/HC resource XAML files is just fine:

<SolidColorBrush
        x:Key="SystemAccentColorBrush"
        Color="{DynamicResource SystemAccentColor}" />

@miloush
Copy link
Contributor

miloush commented Feb 14, 2025

I do NOT think we should include Brushes in the SystemColors class.

The SystemColors class already contains plenty of brushes corresponding to various system colors, so it is a convention to include brushes there and the accent brushes follow that convention and are already defined there (API review). I would have probably preferred separate SystemBrushes class but it's a bit late for that now.

If you lock the brush into a solid color brush, themes/users cannot replace it with different kind of brushes (like a gradient one).

@robert-abeo
Copy link
Author

If you lock the brush into a solid color brush, themes/users cannot replace it with different kind of brushes (like a gradient one).

This is fine, all the other resources are done this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Investigate Requires further investigation by the WPF team. Win 11 Theming
Projects
Status: No status
Development

No branches or pull requests

3 participants