wpf icon indicating copy to clipboard operation
wpf copied to clipboard

[API Proposal]: FluentWindows Theme Switch in WPF

Open dipeshmsft opened this issue 1 year ago • 48 comments
trafficstars

Background and motivation

With the introduction of FluentWindows ( Win11 ) theme, we want to allow developers to be able to enable disable and switch themes in their WPF applications. Irrespective of if we provide an API in .NET 9, I believe that we should provide a way for developers to choose only Light, Dark theme or respond to system theme changes.

Part of effort - https://github.com/dotnet/wpf/discussions/8655

This is inspired and a minified version of the #8759

Before going through the alternatives, I would like to bring to the point, since we are so close to the code completion deadline for .NET Preview 7, that if we choose to implement the API, we keep it experimental, so that we have room for further discussion in future.

API Proposal

Current Proposal


    public class Application
    {
        [Experimental]
        public string Theme { get; set; }
    }

    public class Window
    {
        [Experimental]
        public string Theme { get; set; }
    }

The accepted values for the theme properties can be - None, System, Light and Dark

As mentioned above, these APIs will be Experimental until we have further discussions and have a better way to handle theme ( default ) styles in .NET 10.

Behavior of the APIs

  1. When Window.Theme = System / Light / Dark it will take precdence over Application.Theme and we will load the Fluent styles for window. In case, of System Window will respond to System theme changes.
  2. When Application.Theme = System / Light / Dark and Window.Theme = None then window will follow the application.
  3. In case none of the above properties are set, we will get Aero2 by default.
  4. Accent color changes will be received by every window irrespective of the Theme property values. If Theme value is None, then Accent colors don't come into play.
  5. Default values for both Theme properties is None.

Alternative Designs

No response

Risks

No response

cc: @dotnet/dotnet-wpf-triage @pomianowski

dipeshmsft avatar Mar 20 '24 09:03 dipeshmsft

Will this property automatically affect all windows for which current one is Parent? I think that changing e.g. MainWindow should affect all additional windows for this below

pomianowski avatar Mar 20 '24 10:03 pomianowski

The default should come from Application.Current, if there is one.

I don't think getting it from the parent window is a good idea.

batzen avatar Mar 20 '24 10:03 batzen

@pomianowski @batzen I have updated the proposal. The new DP was meant to be at the application level.

dipeshmsft avatar Mar 20 '24 10:03 dipeshmsft

@dipeshmsft Only having it at the application level won't be enough for all use-cases, i think.

batzen avatar Mar 20 '24 13:03 batzen

@batzen, apart from the application one place where we should have this, I guess is at the window level. Do you have some other use cases for this, that we can consider ?

dipeshmsft avatar Mar 20 '24 15:03 dipeshmsft

I think Application and Window should be enough.

batzen avatar Mar 24 '24 14:03 batzen

@batzen, what is the behavior that you expect when a developer sets the Theme property on only one window in the application ?

dipeshmsft avatar Mar 27 '24 19:03 dipeshmsft

@dipeshmsft That only that one specific window, and thus it's content, changes it's theme.

batzen avatar Mar 28 '24 09:03 batzen

@dipeshmsft Only having it at the application level won't be enough for all use-cases, i think.

@batzen can you please elaborate on which use cases are you expecting for having different theme per window in one app? I don't think I have seen an app that offers that. It would also mess up the precedence of resources I think...

miloush avatar Mar 28 '24 15:03 miloush

A prominent example is Visual Studio. Visual Studio supports dark and light themes for quite some time now, but the options window still ignores it, most probably due to its complexity.

In general I can see where it can be useful to disable dark theme (force light theme) for some windows where it just will take some time to fix all the issues to correctly support dark theming. Or when you embed 3rd party views where you are not in control whether theming is correctly implemented.

Then it may be better to force a light theme for a specific window rather than mixing dark and light themes.

MichaeIDietrich avatar Mar 29 '24 22:03 MichaeIDietrich

@MichaeIDietrich That's exactly what i would have said. And we must not forget that WPF has to compete with MahApps.Metro, MaterialDesignInXAML, WPFUI, Fluent.Ribbon, ControlzEx etc. all of those allow changing the theme even further down at the FrameworkElement level.

batzen avatar Mar 30 '24 06:03 batzen

Visual Studio case is more about "themed" window vs "non-themed" window. They probably just did not bother investing in theming the legacy Settings window, focusing on the new settings tab experience instead (which is themed). Same with some of the dialogs that were "forgotten" unthemed but newer ones are themed. Note that if you turn on one of the high contrasty themes, they will apply to the settings too. This is not a very convincing example.

WPF has to compete

It does not. People can still keep using 3rd party libraries to fill in functionality that the framework does not provide.

I am not against having window/element level theming in principle if it was a low-hanging fruit, despite me not seeing a good use case for it yet, but it is not. You would have to design and change the whole DP precedence system to squeeze per window/element themes in. The team is already too busy with delivering this feature and I would rather if we not increase the release bar further. Let's get application-wide theme switching out and if there is enough calls for having this controlled more granularly, we can do add this later.

As for the proposed API itself, I would prefer if this was a string property where you can put any of the themes available, such as "Classic" or "Royale", which would require no changes for future themes (or possibly when custom ones can be shipped with apps).

miloush avatar Mar 30 '24 11:03 miloush

Visual Studio case is more about "themed" window vs "non-themed" window.

Not quite sure whether I would agree on the "themed" vs "non-themed" argument. From the WPF point of view every visual representation is a theme, so there isn't really something like "non-themed". That's why I would differentiate between light and dark theming, mixing two different light themes (e.g. Aero and Aero2) is visually less an issue than mixing dark and light themes.

They probably just did not bother investing in theming the legacy Settings window, focusing on the new settings tab experience instead (which is themed). Same with some of the dialogs that were "forgotten" unthemed but newer ones are themed.

I'm pretty sure that the idea of moving the settings to a new tab experience is by far not as old as the Settings window not supporting a different theme. So they had their reasons to not support it, we can only guess at that point.

Note that if you turn on one of the high contrasty themes, they will apply to the settings too. This is not a very convincing example.

Good point indeed. I'm pretty surprised it doesn't look as scrambled as I would have expected it.

Still, my point stays that we need to keep in mind real world applications that consists of more than 3 windows and have developed over years with hard coded colors at several places and stuff that cannot be fixed within two days.

I just see applications like from the company I work at that has support for 3rd party plugins bringing their own UI. Such plugins won't support dark theme on day one. And now there will be only the option to disable dark theme for the whole application or to let the user live with the result.

if it was a low-hanging fruit

I agree with that. It's more work to do if we want to support this. And I'm also on your side that even with the current state I'm not 100% confident that Win11 themes will make it to the next .NET release seeing the progress so far.

As for the proposed API itself, I would prefer if this was a string property where you can put any of the themes available, such as "Classic" or "Royale", which would require no changes for future themes (or possibly when custom ones can be shipped with apps).

Same for me, I would also prefer to make all the themes just identifiable by a key and not implement any specific logic for Win11 into WPF. That way custom themes could also be implemented.

But this doesn't mean the theme to use couldn't be also resolved on window level, since resources are already resolved that way by walking up the hierarchy, this could be also done for themes.

This all being said, focus should be on bringing Win11 themes with the next .NET version and if such feature wishes would delay the release, then it can be, of course, argued to give those a lower priority.

MichaeIDietrich avatar Mar 30 '24 15:03 MichaeIDietrich

@miloush Another argument for having it at least at the window level: What should i do if i don't have an application object? That's often the case for unit tests and for things like office addins.

WPF has to compete

It does not. People can still keep using 3rd party libraries to fill in functionality that the framework does not provide.

In regards to dark/light flexibility it has to compete. If it does not have to compete with those, or at least delivering the building blocks, i hardly see any reason copy/pasting the code from WPFUI to be WPF code base. This time WPF slept for a very long time and people got used to use third party libraries to fill those gaps. If we now want to fill parts of those gaps it has to meet the expectations people have from those third party libraries. It's also quite strange to me that we haven't heard from larger companies building WPF solutions here. Do they even know what's currently planned here? What's the expectation for future development? If it does not have to compete i can't rely on the then WPF provided dark/light setting in my libraries/apps and will have to keep all the code that manages that. Dark/Light should be totally unrelated to the concrete theme being loaded, but just the color values being provided for a theme.

@dipeshmsft So i guess what we really need is not None, System, FluentWindowsLight and FluentWindowsDark but System, Dark, Light as the enum values and an additional DP that reflects the current real value for those. Without that additional DP we can't trigger on anything that's system because we don't know if it's dark or light. We also need a way to load additional RDs for Dark/Light in the app/library itself as apps/libraries might have custom dark/light color values. The theming system we came up with in ControlzEx (see https://github.com/ControlzEx/ControlzEx/blob/develop/src/ControlzEx/Theming/ThemeManager.cs) does exactly that. It's far from perfect, but at least ensures/delivers that flexibility.

@miloush

You would have to design and change the whole DP precedence system to squeeze per window/element themes in. The team is already too busy with delivering this feature and I would rather if we not increase the release bar further. Let's get application-wide theme switching out and if there is enough calls for having this controlled more granularly, we can do add this later.

I am unable to see any work being done in a direction that implements a theming system different from regular resource dictionaries, so i totally don't get that point. If there is work being done in that direction could you point me to it? There are bugs/quirks in WPF that should be fixed along the way to fully unblock RD based theming though For example https://github.com/dotnet/wpf/issues/8860

Also https://github.com/dotnet/wpf/pull/5610 should be merged if all the DynamicResource usages increase in the system provided theme which currently enable dark/light switching.

batzen avatar Mar 30 '24 16:03 batzen

What should i do if i don't have an application object?

OK that is an argument I can understand.

So i guess what we really need is not None, System, FluentWindowsLight and FluentWindowsDark but System, Dark, Light.

I can work with that. So you would have one property for the theme selection (Classic/Royale/Aero2/Fluent etc.) that would pick up control templates and one property for System/Dark/Light that would say substitute a set of colors/brushes used as dynamic resources (technically this wouldn't need to be enums either). The colors/brushes set would do nothing for themes that don't use them, but app could still look up the values in resources. As for the extra DP, this could be a value in SystemParameters or somewhere. There already seems to be UxThemeName and UxThemeColor.

The alternative is kind of what Visual Studio is doing (and possibly what @dipeshmsft was originally proposing), you have one property with a set of "themes" that include dark, system and anything in-between.

image

I think I am starting to like the separation of colors and templates more.

It is my impression that the intention is that the Fluent theme would be in all respects equivalent to the existing themes such as Aero2, the RD method is just used for testing it. We cannot leave it as RD because that messes up with the precedence of resources. An opt-in to Fluent is needed, and providing a property that allows users to pick a specific theme seems to be the best option to me and an enhancement on its own.

As a possibly slightly off-topic, how does ThemeDictionaryExtension fit into this? I don't think I have used that one personally.

miloush avatar Apr 02 '24 09:04 miloush

@batzen @MichaeIDietrich @pomianowski

@miloush, Although I like the idea of having separate properties for theme name and color, but this will open up a new layer for styling to the developers. With the current system in place, any WPF developer can use explicit and implicit styles, however providing theme name would open a new layer ThemeStyle for the developers. So, I am not very convinced if we should do that.

I like the idea of having ThemeColor property as string, as other themes in past have different names ( Luna - Metallic, Cobalt, etc. ).

I am unable to see any work being done in a direction that implements a theming system different from regular resource dictionaries, so I totally don't get that point. If there is work being done in that direction could you point me to it?

@batzen, as miloush mentioned earlier the current method was a testing method and we want to make it similar to other themes. I have been doing experiments with ThemeStyle and it doesn't work well with DynamicResource's . When we want to do changes in ThemeStyle, the current infrastructure invalidates all the resource and loads them again, and if I haven't missed anything there is no way as of now to load two theme style resource dictionaries at once. So, if we want to enable different theme for window and application ( which we definitely want to do ), it will need some changes in StyleHelper, SystemResources and related classes.

One thing that I can do is, quickly try creating PresentationFramework.Fluent.Light and Dark combined resource dictionaries and move accent colour resources to SystemResources as then we can allow DynamicResource, but rest of the resources will have to be made static. However this stops developers from using the brush and color resources defined in Fluent theme to create new styles for custom controls. What are your views on this ?

dipeshmsft avatar Jun 12 '24 10:06 dipeshmsft

I have looked into this a bit and I would support a property on FrameworkElement. We currently have ThemeDictionaryExtension that can be used for ResourceDictionary.Source. If I understand correctly, this can already be used today to switch theme assemblies on individual elements, e.g.

<Button>
    <Button.Resources>
        <ResourceDictionary Source="{ThemeDictionary PresentationFramework}" />
    </Button.Resources
</Button>

The only catch is that you cannot change the theme name and color, it will always try to load Component/Themes/ThemeName.ThemeColor.baml resource from the assembly (where in most cases ThemeName=Aero2 and ThemeColor=NormalColor).

  1. The first thing we could do is to extend the ThemeDictionaryExtension to support theme name and color properties. This should allow people to load Fluent (or other) themes as proper themes themselves using the existing infrastructure.

  2. We could then add a property, let's say ThemeDictionary on FrameworkElement. This would be a syntactic sugar to the above. So basically

<Button ThemeDictionary="{ThemeDictionary Name=Fluent, Color=Dark}" />

would be equivalent to

<Button>
    <Button.Resources>
        <ResourceDictionary Source="{ThemeDictionary Name=Fluent, Color=Dark}" />
    </Button.Resources>
</Button>

(or merged in if the Button already has resource dictionary set). This would allow per-element theme without Application object existing. I was skeptical about per-element themability because I thought it would require significant work. However, it seems everything should already be in place with the help of ThemeDictionaryExtension.

  1. Finally, we could add application-wide way to change the theme name and color. There is SystemParameters.UxThemeName and SystemParameters.UxThemeColor which drive the theme selection described above. One option is to make them settable, however, they would no longer reflect the system settings, which might be undesirable (and setting them would not change the Ux theme, which is also confusing). SystemParameters has a lot of useful infrastructure so it might be beneficial to put the properties on SystemParameters, like ThemeName or UxThemeNameOverride or similar, even though they would not be system per se. The other option could be static properties on Application EDIT:, although it might be desirable to set them at the same time using a method. We could also read these from app.config.

miloush avatar Jun 22 '24 16:06 miloush

@miloush, the idea of extending ThemeDictionaryExtension seems reasonable and above all it sits well with the existing infrastructure, but to do this right now, we would need to make changes in infrastructure like - loading multiple theme dictionaries for PresententationFramework to start with. It was also the intention of the original developers, for ThemeDictionary to react to theme changes and we would have to sever that and take care of coercion in case of high contrast. This will take some time.

However, IMHO I don't think we should make SystemParameters settable. They are meant to reflect the actual values from the system and by making them settable we will deviate from the actual behavior.

dipeshmsft avatar Jun 24 '24 13:06 dipeshmsft

I assume the property is meant to cause a particular resource dictionary to be merged in?

If so, what is the extensibility story? Would a developer set the property to None and manage their resource dictionaries themselves, just like they do right now?

Alternatively, you could have a design where the theme is a string and the user can register additionally dictionaries under a given name. This would allow custom themes to benefit from the same theme selection logic if you support both Application and Window scopes.

Strings aren't great for discovery, but we have recently started using what we call string-based enums which are just struct wrappers around a string:

namespace System.Windows;

public partial class Application 
{
    public static readonly DependencyProperty ThemeProperty;
    public ApplicationTheme Theme { get; set; }
    public void RegisterTheme(ApplicationTheme theme, ResourceDictionary resources);
}

public partial class Window
{
    public static readonly DependencyProperty ThemeProperty;
    public ApplicationTheme Theme { get; set; }
}

public readonly struct ApplicationTheme : IEquatable<ApplicationTheme>
{
    public static ApplicationTheme None { get; } = new(nameof(None));
    public static ApplicationTheme System { get; } = new(nameof(System));
    public static ApplicationTheme FluentWindowsLight { get; } = new(nameof(FluentWindowsLight));
    public static ApplicationTheme FluentWindowsDark { get; } = new(nameof(FluentWindowsDark));

    public ApplicationTheme(string value);
    public string Value { get; }

    public static bool operator==(ApplicationTheme left, ApplicationTheme right);
    public static bool operator!=(ApplicationTheme left, ApplicationTheme right);
}

This allows developers to define their own themes:

public static class ImmosThemes
{
    public static ApplicationTheme ImmoDark { get; } = new(nameof(ImmoDark));
    public static ApplicationTheme ImmoLight { get; } = new(nameof(ImmoDark));
}
...
ResourceDictionary immoDarkResources = ...
ResourceDictionary immoLightResources = ...
Application.Current.RegisterTheme(ImmosThemes.ImmoDark, immoDarkResources);
Application.Current.RegisterTheme(ImmosThemes.ImmoLight, immoLightResources);
Application.Current.Theme = ImmosThemes.ImmoDark;

We could also decide that having a strongly typed theme is overkill and just purely go with strings:

namespace System.Windows;

public partial class Application 
{
    public static readonly DependencyProperty ThemeProperty;
    public string Theme { get; set; }
    public KeyedCollection<string, ResourceDictionary> Themes { get; }
}

public partial class Window
{
    public static readonly DependencyProperty ThemeProperty;
    public string Theme { get; set; }
}

Usage:

...
ResourceDictionary immoDarkResources = ...
ResourceDictionary immoLightResources = ...
Application.Current.Themes.Add("ImmoDark", immoDarkResources);
Application.Current.RegisterTheme("ImmoLight", immoLightResources);
Application.Current.Theme = "ImmoDark";

terrajobst avatar Jun 24 '24 17:06 terrajobst

@terrajobst

I assume the property is meant to cause a particular resource dictionary to be merged in?

Yes, although for now this will be to merge the resource dictionary, however the current way is not how system themes are loaded in WPF. I have started a discussion on another thread (https://github.com/dotnet/wpf/issues/9283), whether we should provide a way to enable Fluent as a system theme.

If so, what is the extensibility story? Would a developer set the property to None and manage their resource dictionaries themselves, just like they do right now?

None, here means that the framework will use the current default theme ( i.e. Aero2 ) for the application, and none of the Fluent resources will be loaded in Application resources. Regarding extensibility, historically system ( default ) theme has been linked to the OS theme and developers have managed the customization themselves.

However, I guess the string-based enum is suitable here, because even earlier we have themes have been structured as "ThemeName.ThemeColor" and the ThemeColor part keeps on varying from one theme to another.

dipeshmsft avatar Jun 24 '24 17:06 dipeshmsft

Thanks @terrajobst we do have a few "string-based enums" in WPF already.

However, before we spend more cycles on this, perhaps @dipeshmsft can update the API proposal to what the current thinking is, it has been a while since this got originally posted.

miloush avatar Jun 24 '24 18:06 miloush

@terrajobst @miloush

I have updated the proposal inculcating my thoughts and the inputs from the community. However, I want to reiterate that, I would like to keep any API introduced now as 'Experimental'

dipeshmsft avatar Jun 25 '24 20:06 dipeshmsft

Thanks @dipeshmsft. Option 1 feels overly convoluted for an experimental thing to be replaced in the future. For that option I would just add a simple plain string property (which I am guessing is what you marked as alternative approach).

miloush avatar Jun 25 '24 20:06 miloush

Without reading the full discussion, I don't think the names/keys are correct.

  1. You should provide System (default), Aero, Aero2, Fluent2Light, Fluent2Dark, etc. with the current convention. Older themes should all be included too IMO.
  2. Not sure how you plan for high-contrast support.
  3. Naming needs to be updated "FluentWindows" should just be "Fluent" as re-named elsewhere.
  4. If we are making this official I still think we should name it "Fluent2". If Microsoft releases a "Fluent3" you are going to have an issue with naming and off-by-ones otherwise.

I like alternative 1 supporting both Application-level and Window-level theme properties. But why stop there? I wonder if it should be a control-level, inherited property.

robert-abeo avatar Jul 01 '24 20:07 robert-abeo

After doing implementation and testing out both approaches, I have come to the following conclusion for the shape of API for .NET 9


    public class Application
    {
        [Experimental]
        public string Theme { get; set; }
    }

    public class Window
    {
        [Experimental]
        public string Theme { get; set; }
    }

The accepted values for the theme properties can be - None, System, Light and Dark

As mentioned above, these APIs will be Experimental until we have further discussions and have a better way to handle theme ( default ) styles in .NET 10.

Behavior of the APIs

  1. When Window.Theme = System / Light / Dark it will take precdence over Application.Theme and we will load the Fluent styles for window. In case, of System Window will respond to System theme changes.
  2. When Application.Theme = System / Light / Dark and Window.Theme = None then window will follow the application.
  3. In case none of the above properties are set, we will get Aero2 by default.
  4. Accent color changes will be received by every window irrespective of the Theme property values. If Theme value is None, then Accent colors don't come into play.
  5. Default values for both Theme properties is None.

PS: We want to get the experimental API implementation by Preview 7 code complete. ( 7/19 )

cc @miloush @MichaeIDietrich @robert-abeo @batzen @lindexi @ThomasGoulet73 @pomianowski

dipeshmsft avatar Jul 16 '24 18:07 dipeshmsft

These are properties, right? i.e. public string Theme { get; set; }

"Default" sounds like a better default value than "None" because as you said the default is Aero2. However, since this is experimental it doesn't matter much.

Do these properties cause the theme to be loaded into a merged dictionary or as a proper theme?

miloush avatar Jul 16 '24 18:07 miloush

Changed the above to properties. I haven't reached to the implementation of how Applicaiton.Theme will behave with default theme behavior. But for the default scenario, only Application.Theme will play the role in deciding the behavior of theme styles. Window.Theme will still load the themes in merged dictionary.

dipeshmsft avatar Jul 16 '24 18:07 dipeshmsft

Should this be marked as api-ready-for-review then? If so, please update the issue description to reflect the current proposal.

terrajobst avatar Jul 16 '24 19:07 terrajobst

IMHO System/Dark/Light are too vague. What happens when we get Windows 12 or Fluent3 or whatever comes next?

batzen avatar Jul 16 '24 20:07 batzen

@batzen, as mentioned this is experimental and we will rethink this for .NET 10.

dipeshmsft avatar Jul 16 '24 20:07 dipeshmsft