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.

Sign up for GitHub

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

Fixing Button resources #10442

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixing Button resources #10442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/Dark.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,6 @@
<!-- Elevation border brushes -->

<LinearGradientBrush x:Key="ControlElevationBorderBrush" MappingMode="Absolute" StartPoint="0,0" EndPoint="0,3">
<!--<LinearGradientBrush.RelativeTransform>
<ScaleTransform CenterY="0.5" ScaleY="-1" />
</LinearGradientBrush.RelativeTransform>-->
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.33" Color="{StaticResource ControlStrokeColorSecondary}" />
<GradientStop Offset="1.0" Color="{StaticResource ControlStrokeColorDefault}" />
Expand Down Expand Up @@ -312,15 +309,25 @@
<SolidColorBrush x:Key="AccentButtonBackground" Color="{StaticResource SystemAccentColorLight2}" />
<SolidColorBrush x:Key="AccentButtonBackgroundPointerOver" Color="{StaticResource SystemAccentColorLight2}" Opacity="0.9" />
<SolidColorBrush x:Key="AccentButtonBackgroundPressed" Color="{StaticResource SystemAccentColorLight2}" Opacity="0.8" />
<!--<SolidColorBrush x:Key="AccentButtonBackgroundDisabled" ResourceKey="AccentFillColorDisabledBrush" />-->
<SolidColorBrush x:Key="AccentButtonBackgroundDisabled" Color="{StaticResource AccentFillColorDisabled}" />
<SolidColorBrush x:Key="AccentButtonForeground" Color="{StaticResource TextOnAccentFillColorPrimary}" />
<SolidColorBrush x:Key="AccentButtonForegroundPointerOver" Color="{StaticResource TextOnAccentFillColorPrimary}" />
<SolidColorBrush x:Key="AccentButtonForegroundPressed" Color="{StaticResource TextOnAccentFillColorSecondary}" />
<!--<SolidColorBrush x:Key="AccentButtonForegroundDisabled" ResourceKey="TextOnAccentFillColorDisabled" />-->
<!--<SolidColorBrush x:Key="AccentButtonBorderBrush" Color="{DynamicResource AccentControlElevationBorder}" />-->
<!--<SolidColorBrush x:Key="AccentButtonBorderBrushPointerOver" Color="{DynamicResource AccentControlElevationBorder}" />-->
<SolidColorBrush x:Key="AccentButtonForegroundDisabled" Color="{StaticResource TextOnAccentFillColorDisabled}" />
<LinearGradientBrush x:Key="AccentButtonBorderBrush" MappingMode="RelativeToBoundingBox" StartPoint="0,1" EndPoint="0,0">
Copy link

Choose a reason for hiding this comment

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

Same comment as for ButtonBorderBrush, ButtonBorderBrushPointerOver -- re-use the pre-existing brushes.

<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.025" Color="{StaticResource ControlStrokeColorOnAccentSecondary}" />
<GradientStop Offset="0.075" Color="{StaticResource ControlStrokeColorOnAccentDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<LinearGradientBrush x:Key="AccentButtonBorderBrushPointerOver" MappingMode="RelativeToBoundingBox" StartPoint="0,1" EndPoint="0,0">
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.025" Color="{StaticResource ControlStrokeColorOnAccentSecondary}" />
<GradientStop Offset="0.075" Color="{StaticResource ControlStrokeColorOnAccentDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<SolidColorBrush x:Key="AccentButtonBorderBrushPressed" Color="{DynamicResource ControlFillColorTransparent}" />
<!--<SolidColorBrush x:Key="AccentButtonBorderBrushDisabled" Color="{DynamicResource ControlFillColorTransparentBrush}" />-->
<SolidColorBrush x:Key="AccentButtonBorderBrushDisabled" Color="{DynamicResource ControlFillColorTransparent}" />
<SolidColorBrush x:Key="ButtonBackground" Color="{StaticResource ControlFillColorDefault}" />
<SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{StaticResource ControlFillColorSecondary}" />
<SolidColorBrush x:Key="ButtonBackgroundPressed" Color="{StaticResource ControlFillColorTertiary}" />
Expand All @@ -329,8 +336,18 @@
<SolidColorBrush x:Key="ButtonForegroundPointerOver" Color="{StaticResource TextFillColorPrimary}" />
<SolidColorBrush x:Key="ButtonForegroundPressed" Color="{StaticResource TextFillColorSecondary}" />
<SolidColorBrush x:Key="ButtonForegroundDisabled" Color="{StaticResource TextFillColorDisabled}" />
<!--<SolidColorBrush x:Key="ButtonBorderBrush" Color="{StaticResource ControlElevationBorder}" />-->
<!--<SolidColorBrush x:Key="ButtonBorderBrushPointerOver" Color="{StaticResource ControlElevationBorder}" />-->
<LinearGradientBrush x:Key="ButtonBorderBrush" MappingMode="Absolute" StartPoint="0,0" EndPoint="0,3">
Copy link

@robert-abeo robert-abeo Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These should just re-use ControlElevationBorderBrush. I don't think there is any reason to duplicate this complicated Brush. Otherwise, it is duplicate code and any updates would then have to be done multiple places -- bad practice.

WinUI does the same: https://github.com/microsoft/microsoft-ui-xaml/blob/589ced40542b9b4de2fddf0e58a1e3eecd164e2f/src/controls/dev/CommonStyles/Button_themeresources.xaml#L26-L27

I would ALWAYS review WinUI when making these types of changes.

Edit: I believe the below syntax works in WPF. I spend more time with more modern XAML frameworks so this needs to be tested.

    <StaticResource
        x:Key="ButtonBorderBrush"
        ResourceKey="ControlElevationBorderBrush" />
    <StaticResource
        x:Key="ButtonBorderBrushPointerOver"
        ResourceKey="ControlElevationBorderBrush" />

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that works, but let me give it a try.

Copy link
Member Author

Choose a reason for hiding this comment

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

image

I get the following error when I do the above. However, I have seen this syntax in other places, but the StaticExtension there is a subclass of StaticResourceExtension for that : https://github.com/Kinnara/ModernWpf/blob/master/ModernWpf/Markup/StaticResourceExtension.cs

After that they can use the above syntax.

Copy link

Choose a reason for hiding this comment

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

I'm not actually sure what is going on here. The existing StaticResourceExtensions has this constructor as well:

wpf/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/StaticResourceExtension.cs

Lines 35 to 39 in a792086

public StaticResourceExtension(
object resourceKey)
{
ArgumentNullException.ThrowIfNull(resourceKey);
_resourceKey = resourceKey;

Either way, this is useful functionality. Perhaps it is a good idea to support it in Vanilla WPF as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was looking further, the error seems to be thrown by a Debug.Assert statement here :

wpf/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Markup/BamlRecordWriter.cs

Line 563 in a792086

Debug.Assert(!InStaticResourceSection, "We do not support x:Key within a StaticResource Section");

I don't know why this is here. I will open up an issue for this and try to find out what's the reasoning behind this.

By the way, would you know how this works ? Does it create a copy of the actual resource or is just a reference to the original resource ?

<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.33" Color="{StaticResource ControlStrokeColorSecondary}" />
<GradientStop Offset="1.0" Color="{StaticResource ControlStrokeColorDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<LinearGradientBrush x:Key="ButtonBorderBrushPointerOver" MappingMode="Absolute" StartPoint="0,0" EndPoint="0,3">
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.33" Color="{StaticResource ControlStrokeColorSecondary}" />
<GradientStop Offset="1.0" Color="{StaticResource ControlStrokeColorDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<SolidColorBrush x:Key="ButtonBorderBrushPressed" Color="{StaticResource ControlStrokeColorDefault}" />
<SolidColorBrush x:Key="ButtonBorderBrushDisabled" Color="{StaticResource ControlStrokeColorDefault}" />

Expand Down
18 changes: 9 additions & 9 deletions src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/HC.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,15 @@
<SolidColorBrush x:Key="AccentButtonBackground" Color="{StaticResource SystemColorHighlightColor}" />
<SolidColorBrush x:Key="AccentButtonBackgroundPointerOver" Color="{StaticResource SystemColorButtonTextColor}" />
<SolidColorBrush x:Key="AccentButtonBackgroundPressed" Color="{StaticResource SystemColorHighlightColor}" />
<!--<SolidColorBrush x:Key="AccentButtonBackgroundDisabled" ResourceKey="AccentFillColorDisabledBrush" />-->
<SolidColorBrush x:Key="AccentButtonBackgroundDisabled" Color="{StaticResource SystemColorButtonFaceColor}" />
<SolidColorBrush x:Key="AccentButtonForeground" Color="{StaticResource SystemColorHighlightTextColor}" />
<SolidColorBrush x:Key="AccentButtonForegroundPointerOver" Color="{StaticResource SystemColorButtonFaceColor}" />
<SolidColorBrush x:Key="AccentButtonForegroundPressed" Color="{StaticResource SystemColorHighlightColor}" />
<!--<SolidColorBrush x:Key="AccentButtonForegroundDisabled" ResourceKey="TextOnAccentFillColorDisabled" />-->
<!--<SolidColorBrush x:Key="AccentButtonBorderBrush" Color="{StaticResource AccentControlElevationBorder}" />-->
<!--<SolidColorBrush x:Key="AccentButtonBorderBrushPointerOver" Color="{StaticResource AccentControlElevationBorder}" />-->
<SolidColorBrush x:Key="AccentButtonForegroundDisabled" Color="{StaticResource SystemColorGrayTextColor}" />
<SolidColorBrush x:Key="AccentButtonBorderBrush" Color="{StaticResource SystemColorHighlightColor}" />
<SolidColorBrush x:Key="AccentButtonBorderBrushPointerOver" Color="{StaticResource SystemColorHighlightColor}" />
<SolidColorBrush x:Key="AccentButtonBorderBrushPressed" Color="{StaticResource SystemColorHighlightColor}" />
<!--<SolidColorBrush x:Key="AccentButtonBorderBrushDisabled" Color="{StaticResource ControlFillColorTransparentBrush}" />-->
<SolidColorBrush x:Key="AccentButtonBorderBrushDisabled" Color="{StaticResource SystemColorGrayTextColor}" />
<SolidColorBrush x:Key="ButtonBackground" Color="{StaticResource SystemColorButtonFaceColor}" />
<SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{StaticResource SystemColorHighlightTextColor}" />
<SolidColorBrush x:Key="ButtonBackgroundPressed" Color="{StaticResource SystemColorHighlightTextColor}" />
Expand All @@ -201,10 +201,10 @@
<SolidColorBrush x:Key="ButtonForegroundPointerOver" Color="{StaticResource SystemColorHighlightColor}" />
<SolidColorBrush x:Key="ButtonForegroundPressed" Color="{StaticResource SystemColorHighlightColor}" />
<SolidColorBrush x:Key="ButtonForegroundDisabled" Color="{StaticResource SystemColorGrayTextColor}" />
<!--<SolidColorBrush x:Key="ButtonBorderBrush" Color="{StaticResource ControlElevationBorder}" />-->
<!--<SolidColorBrush x:Key="ButtonBorderBrushPointerOver" Color="{StaticResource ControlElevationBorder}" />-->
<SolidColorBrush x:Key="ButtonBorderBrushPressed" Color="Transparent" />
<SolidColorBrush x:Key="ButtonBorderBrushDisabled" Color="Transparent" />
<SolidColorBrush x:Key="ButtonBorderBrush" Color="{StaticResource SystemColorButtonTextColor}" />
<SolidColorBrush x:Key="ButtonBorderBrushPointerOver" Color="{StaticResource SystemColorHighlightColor}" />
<SolidColorBrush x:Key="ButtonBorderBrushPressed" Color="{StaticResource SystemColorHighlightTextColor}" />
<SolidColorBrush x:Key="ButtonBorderBrushDisabled" Color="{StaticResource SystemColorGrayTextColor}" />

<!-- Calendar -->
<SolidColorBrush x:Key="CalendarViewForeground" Color="{StaticResource SystemColorButtonTextColor}" />
Expand Down
34 changes: 27 additions & 7 deletions src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Resources/Theme/Light.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,25 @@
<SolidColorBrush x:Key="AccentButtonBackground" Color="{StaticResource SystemAccentColorDark1}" />
<SolidColorBrush x:Key="AccentButtonBackgroundPointerOver" Color="{StaticResource SystemAccentColorDark1}" Opacity="0.9" />
<SolidColorBrush x:Key="AccentButtonBackgroundPressed" Color="{StaticResource SystemAccentColorDark1}" Opacity="0.8" />
<!--<SolidColorBrush x:Key="AccentButtonBackgroundDisabled" ResourceKey="AccentFillColorDisabledBrush" />-->
<SolidColorBrush x:Key="AccentButtonBackgroundDisabled" Color="{StaticResource AccentFillColorDisabled}" />
<SolidColorBrush x:Key="AccentButtonForeground" Color="{StaticResource TextOnAccentFillColorPrimary}" />
<SolidColorBrush x:Key="AccentButtonForegroundPointerOver" Color="{StaticResource TextOnAccentFillColorPrimary}" />
<SolidColorBrush x:Key="AccentButtonForegroundPressed" Color="{StaticResource TextOnAccentFillColorSecondary}" />
<!--<SolidColorBrush x:Key="AccentButtonForegroundDisabled" ResourceKey="TextOnAccentFillColorDisabled" />-->
<!--<SolidColorBrush x:Key="AccentButtonBorderBrush" Color="{DynamicResource AccentControlElevationBorder}" />-->
<!--<SolidColorBrush x:Key="AccentButtonBorderBrushPointerOver" Color="{DynamicResource AccentControlElevationBorder}" />-->
<SolidColorBrush x:Key="AccentButtonForegroundDisabled" Color="{StaticResource TextOnAccentFillColorDisabled}" />
<LinearGradientBrush x:Key="AccentButtonBorderBrush" MappingMode="RelativeToBoundingBox" StartPoint="0,1" EndPoint="0,0">
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.025" Color="{StaticResource ControlStrokeColorOnAccentSecondary}" />
<GradientStop Offset="0.075" Color="{StaticResource ControlStrokeColorOnAccentDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<LinearGradientBrush x:Key="AccentButtonBorderBrushPointerOver" MappingMode="RelativeToBoundingBox" StartPoint="0,1" EndPoint="0,0">
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.025" Color="{StaticResource ControlStrokeColorOnAccentSecondary}" />
<GradientStop Offset="0.075" Color="{StaticResource ControlStrokeColorOnAccentDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<SolidColorBrush x:Key="AccentButtonBorderBrushPressed" Color="{DynamicResource ControlFillColorTransparent}" />
<!--<SolidColorBrush x:Key="AccentButtonBorderBrushDisabled" Color="{DynamicResource ControlFillColorTransparentBrush}" />-->
<SolidColorBrush x:Key="AccentButtonBorderBrushDisabled" Color="{DynamicResource ControlFillColorTransparent}" />
<SolidColorBrush x:Key="ButtonBackground" Color="{StaticResource ControlFillColorDefault}" />
<SolidColorBrush x:Key="ButtonBackgroundPointerOver" Color="{StaticResource ControlFillColorSecondary}" />
<SolidColorBrush x:Key="ButtonBackgroundPressed" Color="{StaticResource ControlFillColorTertiary}" />
Expand All @@ -326,8 +336,18 @@
<SolidColorBrush x:Key="ButtonForegroundPointerOver" Color="{StaticResource TextFillColorPrimary}" />
<SolidColorBrush x:Key="ButtonForegroundPressed" Color="{StaticResource TextFillColorSecondary}" />
<SolidColorBrush x:Key="ButtonForegroundDisabled" Color="{StaticResource TextFillColorDisabled}" />
<!--<SolidColorBrush x:Key="ButtonBorderBrush" Color="{StaticResource ControlElevationBorder}" />-->
<!--<SolidColorBrush x:Key="ButtonBorderBrushPointerOver" Color="{StaticResource ControlElevationBorder}" />-->
<LinearGradientBrush x:Key="ButtonBorderBrush" MappingMode="RelativeToBoundingBox" StartPoint="0,1" EndPoint="0,0">
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.025" Color="{StaticResource ControlStrokeColorSecondary}" />
<GradientStop Offset="0.075" Color="{StaticResource ControlStrokeColorDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<LinearGradientBrush x:Key="ButtonBorderBrushPointerOver" MappingMode="RelativeToBoundingBox" StartPoint="0,1" EndPoint="0,0">
<LinearGradientBrush.GradientStops>
<GradientStop Offset="0.025" Color="{StaticResource ControlStrokeColorSecondary}" />
<GradientStop Offset="0.075" Color="{StaticResource ControlStrokeColorDefault}" />
</LinearGradientBrush.GradientStops>
</LinearGradientBrush>
<SolidColorBrush x:Key="ButtonBorderBrushPressed" Color="{StaticResource ControlStrokeColorDefault}" />
<SolidColorBrush x:Key="ButtonBorderBrushDisabled" Color="{StaticResource ControlStrokeColorDefault}" />

Expand Down
16 changes: 8 additions & 8 deletions src/Microsoft.DotNet.Wpf/src/Themes/PresentationFramework.Fluent/Styles/Button.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<Setter Property="FocusVisualStyle" Value="{DynamicResource DefaultControlFocusVisualStyle}" />
<Setter Property="Background" Value="{DynamicResource ButtonBackground}" />
<Setter Property="Foreground" Value="{DynamicResource ButtonForeground}" />
<Setter Property="BorderBrush" Value="{DynamicResource ControlElevationBorderBrush}" />
<Setter Property="BorderBrush" Value="{DynamicResource ButtonBorderBrush}" />
<Setter Property="BorderThickness" Value="{StaticResource ButtonBorderThemeThickness}" />
<Setter Property="Padding" Value="{StaticResource ButtonPadding}" />
<Setter Property="HorizontalAlignment" Value="Left" />
Expand Down Expand Up @@ -55,7 +55,7 @@
<ControlTemplate.Triggers>
<Trigger Property="IsMouseOver" Value="True">
<Setter TargetName="ContentBorder" Property="Background" Value="{DynamicResource ButtonBackgroundPointerOver}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource ControlElevationBorderBrush}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource ButtonBorderBrushPointerOver}" />
<Setter TargetName="ContentPresenter" Property="TextElement.Foreground" Value="{DynamicResource ButtonForegroundPointerOver}" />
</Trigger>
<Trigger Property="IsEnabled" Value="False">
Expand All @@ -78,7 +78,7 @@
<Setter Property="FocusVisualStyle" Value="{DynamicResource DefaultControlFocusVisualStyle}" />
<Setter Property="Background" Value="{DynamicResource AccentButtonBackground}" />
<Setter Property="Foreground" Value="{DynamicResource AccentButtonForeground}" />
<Setter Property="BorderBrush" Value="{DynamicResource AccentControlElevationBorderBrush}" />
<Setter Property="BorderBrush" Value="{DynamicResource AccentButtonBorderBrush}" />
<Setter Property="BorderThickness" Value="{StaticResource ButtonBorderThemeThickness}" />
<Setter Property="Padding" Value="{StaticResource ButtonPadding}" />
<Setter Property="HorizontalAlignment" Value="Left" />
Expand Down Expand Up @@ -114,17 +114,17 @@
<ControlTemplate.Triggers>
<Trigger Property="IsMouseOver" Value="True">
<Setter TargetName="ContentBorder" Property="Background" Value="{DynamicResource AccentButtonBackgroundPointerOver}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource ControlElevationBorderBrush}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource AccentButtonBorderBrushPointerOver}" />
</Trigger>
<Trigger Property="IsPressed" Value="True">
<Setter TargetName="ContentBorder" Property="Background" Value="{DynamicResource AccentButtonBackgroundPressed}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource AccentControlElevationBorderBrush}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource AccentButtonBorderBrushPressed}" />
<Setter TargetName="ContentPresenter" Property="TextElement.Foreground" Value="{DynamicResource AccentButtonForegroundPressed}" />
</Trigger>
<Trigger Property="IsEnabled" Value="False">
<Setter TargetName="ContentBorder" Property="Background" Value="{DynamicResource ButtonBackgroundDisabled}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource ButtonBorderBrushDisabled}" />
<Setter TargetName="ContentPresenter" Property="TextElement.Foreground" Value="{DynamicResource ButtonForegroundDisabled}" />
<Setter TargetName="ContentBorder" Property="Background" Value="{DynamicResource AccentButtonBackgroundDisabled}" />
<Setter TargetName="ContentBorder" Property="BorderBrush" Value="{DynamicResource AccentButtonBorderBrushDisabled}" />
<Setter TargetName="ContentPresenter" Property="TextElement.Foreground" Value="{DynamicResource AccentButtonForegroundDisabled}" />
</Trigger>
</ControlTemplate.Triggers>
</ControlTemplate>
Expand Down
Loading