microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.26k stars 673 forks source link

ColorPaletteResources doesn't apply to custom control #3783

Closed alexdi220 closed 1 year ago

alexdi220 commented 3 years ago

Describe the bug Hi, I've tried to sort out the ColorPaletteResources and how it can be used in theming of the custom control. I found that the palette's accent color doesn't apply to brushes of control where used SystemAccentColor. For example <SolidColorBrush x:Key="CustomControl1BackgroundBrushFocused" Color="{ThemeResource SystemAccentColor}"/> . It works perfectly for standard controls (TextBox) and standalone brushes (which I use for a border) but doesn't for custom control with style in Generic.xaml.

Steps to reproduce Project to reproduce ColorPalettesTest.zip MainPage.xaml

<Page
    x:Class="ColorPalettesTest.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:ColorPalettesTest"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d"
    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

    <Page.Resources>
        <ResourceDictionary>
            <ResourceDictionary.MergedDictionaries>
                <ColorPaletteResources Accent="Red"/>
            </ResourceDictionary.MergedDictionaries>
            <SolidColorBrush x:Key="MyAccentBrush" Color="{StaticResource SystemAccentColor}"/>
        </ResourceDictionary>
    </Page.Resources>

    <StackPanel Orientation="Vertical" HorizontalAlignment="Center" VerticalAlignment="Center">

        <TextBox Text="TestText" Margin="5"/>

        <Border Width="100" Height="100" Margin="5" 
                Background="{StaticResource MyAccentBrush}">
            <TextBlock Text="It's a border"/>
        </Border>

        <local:CustomControl1 Width="100" Height="100" Margin="5"/>
    </StackPanel>
</Page>

Generic.xaml

<ResourceDictionary
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:ColorPalettesTest">

    <ResourceDictionary.ThemeDictionaries>
        <ResourceDictionary x:Key="Default">
            <SolidColorBrush x:Key="CustomControl1BackgroundBrushFocused" Color="{ThemeResource SystemAccentColor}"/>
        </ResourceDictionary>
    </ResourceDictionary.ThemeDictionaries>

    <Style TargetType="local:CustomControl1" >
        <Setter Property="Background" Value="{ThemeResource CustomControl1BackgroundBrushFocused}"/>
        <Setter Property="Template">
            <Setter.Value>
                <ControlTemplate TargetType="local:CustomControl1">
                    <Border
                        Background="{TemplateBinding Background}"
                        BorderBrush="{TemplateBinding BorderBrush}"
                        BorderThickness="{TemplateBinding BorderThickness}">
                    </Border>
                </ControlTemplate>
            </Setter.Value>
        </Setter>
    </Style>
</ResourceDictionary>

Expected behavior The ColorPaletteResources should apply to the custom control.

Screenshots image

Version Info

NuGet package version: [Microsoft.WinUI 3.0.0-preview3.201113.0]

Windows app type: UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT

Additional context

StephenLPeters commented 3 years ago

I'm surprised this works at all, in the code you've posted CustomControl1BackgroundBrushFocused is defind only for the default (dark) theme. Your app looks like its in light theme so I would expect the system to have failed to find the CustomControl1BackgroundBrushFocused resource. @kikisaints and @MikeHillberg ?

mdtauk commented 3 years ago

I'm surprised this works at all, in the code you've posted CustomControl1BackgroundBrushFocused is defind only for the default (dark) theme. Your app looks like its in light theme so I would expect the system to have failed to find the CustomControl1BackgroundBrushFocused resource. @kikisaints and @MikeHillberg ?

Perhaps the Default theme is used as a fallback, and that is why it works, whilst not working.

But yes, values should be provided for Default, Light, and HighContrast themes to be as compatible as possible

alexdi220 commented 3 years ago

Thanks for the quick answers. I've modified my sample by your recommendations and it still not working. Generic.xaml

    <ResourceDictionary.ThemeDictionaries>
        <ResourceDictionary x:Key="Default">
            <SolidColorBrush x:Key="CustomControl1BackgroundBrushFocused" Color="{ThemeResource SystemAccentColor}"/>
        </ResourceDictionary>
        <ResourceDictionary x:Key="Light">
            <SolidColorBrush x:Key="CustomControl1BackgroundBrushFocused" Color="{ThemeResource SystemAccentColor}"/>
        </ResourceDictionary>
        <ResourceDictionary x:Key="HighContrast">
            <SolidColorBrush x:Key="CustomControl1BackgroundBrushFocused" Color="{ThemeResource SystemAccentColor}"/>
        </ResourceDictionary>
    </ResourceDictionary.ThemeDictionaries>

    <Style TargetType="local:CustomControl1" >
        <Setter Property="Background" Value="{ThemeResource CustomControl1BackgroundBrushFocused}"/>
        <Setter Property="Template">
            <Setter.Value>
                <ControlTemplate TargetType="local:CustomControl1">
                    <Border
                        Background="{TemplateBinding Background}"
                        BorderBrush="{TemplateBinding BorderBrush}"
                        BorderThickness="{TemplateBinding BorderThickness}">
                    </Border>
                </ControlTemplate>
            </Setter.Value>
        </Setter>
    </Style>

MainPage.xaml

...
    <Page.Resources>
        <ColorPaletteResources x:Key="Default" Accent="Red" BaseLow="Black"/>
    </Page.Resources>
    <StackPanel Orientation="Vertical" HorizontalAlignment="Center" VerticalAlignment="Center">

        <Button Content="Change Palette" Click="myButton_Click"/>
        <TextBox Text="TestText" Margin="5"/>
        <local:CustomControl1 Width="100" Height="100"/>

    </StackPanel>

Is the x:Key="Default" dark theme? I supposed that is a Light and that if there are no specified resources for other themes (Dark, HighContrast) the Default resources will use instead. At now, it's not clear. Do you have any guide or any docs for the best practices to theming of the custom controls? Also, for the previous sample (with Default theme only) the palette works perfectly if I place it App.xaml:

<Application.Resources>
        <ResourceDictionary>
            <ResourceDictionary.MergedDictionaries>
                <XamlControlsResources xmlns="using:Microsoft.UI.Xaml.Controls" />
            </ResourceDictionary.MergedDictionaries>
            <ResourceDictionary.ThemeDictionaries>
                <ResourceDictionary x:Key="Default">
                    <ResourceDictionary.MergedDictionaries>
                        <ColorPaletteResources Accent="Red" BaseLow="Black"/>
                    </ResourceDictionary.MergedDictionaries>
                </ResourceDictionary>
            </ResourceDictionary.ThemeDictionaries>
        </ResourceDictionary>
    </Application.Resources>

ColorPalettesTest.zip

Upd: this topic describes Although you can create a ResourceDictionary with "Default" as the key, it’s preferred to be explicit and instead use "Light", "Dark", and "HighContrast". In this case, the Default looks like resources that are used for all themes, if nothing specified.

alexdi220 commented 3 years ago

Any updates? Could we expect the correctly working of the ColorPalettes to use for our themes engine?

StephenLPeters commented 3 years ago

@alexdi220 do you know if this issue is unique to Winui3 or it it occurs with system xaml (winui2) as well?

alexdi220 commented 3 years ago

@StephenLPeters I didn't check it on WinUI 2. So I don't have an answer.

StephenLPeters commented 3 years ago

@kikisaints FYI.

jschwizer99 commented 3 years ago

I observe a similar behavior when using the theme resources in the code.

<ResourceDictionary>
    <ResourceDictionary.ThemeDictionaries>
        <ResourceDictionary x:Key="Dark">
            <SolidColorBrush x:Key="TitleBarFocusColorBrush" Color="{ThemeResource SystemAccentColorDark3}"/>
      </ResourceDictionary>
        <ResourceDictionary x:Key="Light">
            <SolidColorBrush x:Key="TitleBarFocusColorBrush" Color="{ThemeResource SystemAccentColorLight3}"/>
        </ResourceDictionary>
    </ResourceDictionary.ThemeDictionaries>
</ResourceDictionary>

I always will get the TitleBarFocusColorBrush of the Light definition independent of the sequence of definition or the app theme configuration. The following code is executed multiple times as triggered by a focus event.

    winrt::Microsoft::UI::Xaml::Media::Brush OperationEdit::FocusColor() const
    {
        // Currently we can not use {Themeresource ...} to set a propery value
        // as there seems no set change on the property.
        const auto resources = winrt::Microsoft::UI::Xaml::Application::Current().Resources();
        if (resources.HasKey(winrt::box_value(L"TitleBarFocusColorBrush")))
        {
            winrt::Windows::Foundation::IInspectable color = resources.Lookup(winrt::box_value(L"TitleBarFocusColorBrush"));
            return winrt::unbox_value<winrt::Microsoft::UI::Xaml::Media::SolidColorBrush>(color);
        }
        else
        {
            // This code is just a fall-back (is never executed)
                        return winrt::unbox_value<winrt::Microsoft::UI::Xaml::Media::Brush>(GetValue(FocusColorProperty()));
        }
    }

Originally, I've planned to use the FocusColor() property with a XAML definition {Themeresource ...}. But it seems this will never trigger a property change (never hit a breakpoint). Instead, I tried to extract the proper brush from the resource directories directly. But in this case, I will always get the Light brush color.

Are there any updates on the ticket status?

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.