-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[API Proposal]: FluentWindows Theme Switch in WPF #8932
Comments
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 |
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. |
@pomianowski @batzen I have updated the proposal. The new DP was meant to be at the application level. |
@dipeshmsft Only having it at the application level won't be enough for all use-cases, i think. |
@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 ? |
I think Application and Window should be enough. |
@batzen, what is the behavior that you expect when a developer sets the Theme property on only one window in the application ? |
@dipeshmsft That only that one specific window, and thus it's content, changes it's theme. |
@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... |
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. Then it may be better to force a light theme for a specific window rather than mixing dark and light themes. |
@MichaeIDietrich That's exactly what i would have said. |
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.
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). |
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.
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.
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.
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.
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. |
@miloush
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. @dipeshmsft
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? Also #5610 should be merged if all the DynamicResource usages increase in the system provided theme which currently enable dark/light switching. |
OK that is an argument I can understand.
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 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. 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 |
@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. ).
@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 ? |
I have looked into this a bit and I would support a property on <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).
<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
|
@miloush, the idea of extending However, IMHO I don't think we should make |
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 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 Strings aren't great for discovery, but we have recently started using what we call 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"; |
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 (#9283), whether we should provide a way to enable Fluent as a system theme.
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 |
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. |
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' |
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). |
Without reading the full discussion, I don't think the names/keys are correct.
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. |
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 - As mentioned above, these APIs will be Behavior of the APIs
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 |
These are properties, right? i.e.
Do these properties cause the theme to be loaded into a merged dictionary or as a proper theme? |
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. |
Should this be marked as api-ready-for-review then? If so, please update the issue description to reflect the current proposal. |
IMHO System/Dark/Light are too vague. What happens when we get Windows 12 or Fluent3 or whatever comes next? |
@batzen, as mentioned this is experimental and we will rethink this for .NET 10. |
How about add the public static class Themes
{
public readonly static string None = "None";
public readonly static string System = "System";
public readonly static string Dark = "Dark";
public readonly static string Light = "Light";
} |
I don't want to add any unnecessary API for experimental purposes, especially since we know we don't actually want to do this this way in production. The aim here is to get more testing coverage, and class like this won't help in XAML anyway. |
I am going ahead and marking this API as ready-for-review now |
I think that largely defeats the purpose of an experimental API. I'd say: design your experimental API as if that's your final API. You typically use an experimental API if you that don't have a lot of runway to incorporate custome feedback, or if you know of complex interactions between your feature and the ecosystem, and thus want to ship it such that enough people use it in production or production-like scenarios to unveil any design limitations. Yes, that means you expect that your design needs to make some changes, but you generally don't know where or what they might be. Even with an experimental API, you still want to minimize breaking changes. Therefore, I wouldn't ship something as experimental if I already know that's not the design I want -- especially because that also means you don't get feedback on the thing you actually want to build. That said, I think it makes sense to scope the size of an experimental API such that it's a good V1. You generally want to include affordances that help with productivity / address the common cases but also sufficient coverage for advanced scenarios as that's usually where the rubber hits the road and feedback is the most useful. Experimental APIs shouldn't be toys or prototypes. They should be well-designed and viable V1 products that need more validation. |
@terrajobst agree with you completely, however that is simply not the case here. It is now apparent that this feature will need to roll out at least over two major versions. The team has spent all resources on creating the content and barely any discussions started on how the content should be made available to developers. There is generally two ways how to do this: one is merging in a dictionary at app/window level, and the other one is turning it into a "proper" theme the same way existing themes have been done. The former is what has been done so far for testing the content and can still be done manually by developers in XAML. It is, however, not a single liner. The latter will need more work, design and API changes to get done, certainly not doable for .NET 9. To be honest I thought this API proposal would enable to start testing the latter, while the former would remain available the same way it has been so far. This seems to not be the case though and the API proposal is now basically a much shorter syntax for the former (+ automatically swapping between dark and light themes). It is indeed a prototype at best. If that is not acceptable for an experimental API, then I would be pressured to push back on this API proposal. Do you have any alternative mechanisms for prototypes? |
Apologies if my commentary came across as harsh; I didn't mean to criticize the work that is being done here. It just seemed to me that you were suggesting to keep the size of the experimental API surface as small as possible, maybe because it somehow feels risky. My main point was that I wouldn't approach it like this but rather treat it like any other feature work (compelling, good UX, etc) and treat the experimental-ness merely as an insurance policy that allows you to tweak the API based on customer feedback. But that's an ideal. Sometimes ideal isn't workable and our job is "to pick the least bad option". If you think your design is only at a prototype level, you might want a solution that allows you to iterate more quickly. That's where shipping a NuGet package might be helpful. We've done this with some of the generic math APIs in .NET 6, which then got added in-box with .NET 7. Sometimes shipping an OOB isn't easy or even feasible because you need to modify types that have to be in-box; that's partially why we came up with the I'm not involved closely enough with your design and constraints to answer what approach makes the most sense. However, it seems we're all in agreement that all things being equal, having customers be able to use this new feature in .NET 9 is preferrable over another solution that defers it to .NET 10. So in that sense I'm very supportive of any experimental API shape we can ship in .NET 9. Hope this makes sense. |
That is indeed the case here. Sounds like experimental is the best we can do at the moment. |
namespace System.Windows
{
public class Application
{
[Experimental]
public ThemeMode ThemeMode { get; set; }
}
public class Window
{
[Experimental]
public ThemeMode ThemeMode { get; set; }
}
[Experimental]
public readonly struct ThemeMode : IEquatable<ThemeMode>
{
public static ThemeMode None => new ThemeMode();
public static ThemeMode System => new ThemeMode("System");
public static ThemeMode Light => new ThemeMode("Light");
public static ThemeMode Dark => new ThemeMode("Dark");
public string Value => _value ?? "None";
public ThemeMode(string value) => _value = value;
// + IEquatable members
}
} |
Since this is experimental I realize you are avoiding most feedback; however, for the future:
|
I have to agree the strongly-typed string was an unfortunate decision. The value of having a loose string was especially to support the experimental nature of the API and avoid having to approve further API changes when any of the strings or their set needs to change, for example to include more information than just a theme name. The name While WPF has cases of enumerated strings as static members, this way of strongly typing is unprecedented in the framework. As also noted above, it doesn't bring any value for the testing, since it would be used mostly in XAML without any autocompletion support. The type will need to eventually go away and if the aim is to minimize future breaking changes, removing a public type and members feels more disruptive than just changing a property type, although formally that is probably equivalent. Thanks for approving the API though, it would be good to make it easier for more people to try out the theming! (@robert-abeo WPF has ThemeColor) |
Please elaborate as I'm not familiar with |
(FYI, I'm not a WPF expert but was part of the API review group, so my comments are a reflection of how we understood the feature.)
I think using loose strings to avoid review time isn't a good reason. Rather, we should think about the desired UX and ship that. I'd rather cut corners in internal processes than compromise the UX. However, if we do strings then the value of this type is to allow an open-ended set while also making well-known names more discoverable. This pattern is used in the .NET platform in various places and has proven to be an effective UX.
That is a very fair point and one that wasn't considered in the review. If the primary use is indeed from XAML, then this design has no benefits. My assumption was that this would be part of the app's start up code i.e in code-behind.
If you already know that you don't want this type because you don't think you want to use strings but some other to-be-designed type, then one can argue that this design doesn't help much. However, it also doesn't hurt much because you're basically saying "I believe this feature will work completely differently, I just don't know what it will look like". IOW, you're saying "I expect all users of this features having to completely change what code I expect them to write".
In general yes, removing types is more disruptive than adding/removing members. However, in your case the entire feature (type and member) is opt-in. The only people that can use it are the ones that suppressed the error "this type is marked as experimental, suppress me if you want to use it anyway". So the break only impacts people that have agreed to be broken. You still want to minimize breaking changes for early adopters but it sounds to me like any design we cook up right now is guaranteed to change anyway, so we might as well choose one that we think is the easiest to use. Whether or not this approved design will be easier to use is a different question; we didn't consider usage from XAML. If this is the primary use and having this type shape makes it harder, then I think it's reasonable to revert back to the original plain string design. |
I know this may be wrong BUT leaving aside what is currently implemented there and what maybe we should be added, it seems to me that from the point of view of the UX of a new developer the division into enum ThemeMode
{
System,
Light,
Dark
} default(ThemeMode) = ThemeMode.System We can consider that we have an Application Theme that is simply light or dark (High contrast is more difficult for me because it can simply be a variant of light or dark), there are no other combinations. And its default value can be By default, the new Variant can be some Maybe something like: public class ThemeVariant
{
public const string HighContrastWhite = "HCWhite";
// or
public const string HighContrast = "HC";
// or
public const string Desert = "HCWhite";
} namespace System.Windows
{
public class Application
{
[Experimental]
public ThemeMode Theme { get; set; }
[Experimental]
public string ThemeVariant { get; set; }
// or composite like public sealed record ThemeContext(ThemeMode Mode, ThemeVariant Variant);
[Experimental]
public ThemeContext Theme { get; set; }
}
public class Window
{
[Experimental]
public ThemeMode Theme { get; set; }
[Experimental]
public string ThemeVariant { get; set; }
}
} From
<Button ThemeDictionary="{ThemeDictionary Mode=Dark, Variant=HighContrast}" />
<Button ThemeDictionary="{ThemeDictionary Mode=Light, Variant=Default}" /> From the Windows 11 system themes there's also a table
|
@terrajobst Thanks for some background
No haven't seen the video and probably won't have time. I'm glad you discussed the nuance here. I would just try to use existing conventions in WinUI as much as possible in the future.
API context is VERY important here. When you set
That does make some sense. Nobody has solved this problem in any XAML framework as far as I know. You just have to manually insert the resource dictionaries.
Ok, that is how it's handled most places anyway so as long as it's supported automatically behind the scenes it makes sense to me. |
More people arrived to the same conclusion, which is probably why WPF originally distinguishes "theme name" and "theme color" (which is also why adding another "theme X" is adding to the confusion). For example, the Luna theme has 3 variants, NormalColor, Homestead, Metallic. More recent themes (AeroLite/Aero2 etc.) have only one color variant, NormalColor. One option is to make the Fluent theme fit into this pattern with color variants of Dark, Light and System. The existing themes typically have high contrast look built-in using triggers, which might or might not provide enough flexibility for the modern concept of high-contrast themes. I can see HC variants being treated as another color variants. The WPF already seems to have a way to load not only a different theme, but theme from any 3rd party assembly, possibly per-element, using Ultimately, we might want a way for developers to specify 1) an assembly 2) theme name 3) theme color, while keeping the XAML syntax to a minimum. Adding 3 new properties to everything is my least favorite option, especially since not all combinations of values will be valid. At the moment I think it will either have to be a custom type carrying a combination of these values, or, quite WPF-naturally and as the existing infrastructure uses, an
That very accurately describes my understanding of the current state. Not only that, but I am realizing that changing a property type might be a worse outcome if we will have another year of people documenting and blogging about how to use the current API (it's easier to figure out that the required type/property does not exist than that it subtly changed). My worry also was that having more complex experimental API is an investment in everyone's time and there will be more reluctance to "fix" it later, while I very strongly believe it needs to be fixed. Knowing that the API review team is happy to engage with the WPF team and open to the future changes is much appreciated. With the approved API being experimental and learning the expectations from both sides I don't have objections going ahead as approved. Thanks @terrajobst for your time and expertise! |
@terrajobst, for setting namespace System.Windows;
[Experimental]
public class ThemeModeConverter : TypeConverter
{
public ThemeModeConverter();
public override bool CanConvertFrom(ITypeDescriptorContext typeDescriptorContext, Type sourceType);
public override bool CanConvertTo(ITypeDescriptorContext typeDescriptorContext, Type destinationType);
public override object ConvertFrom(ITypeDescriptorContext typeDescriptorContext, CultureInfo cultureInfo, object source);
public override object ConvertTo(ITypeDescriptorContext typeDescriptorContext, CultureInfo cultureInfo, object value, Type destinationType);
} |
I assume this would go into the @dotnet/fxdc unless anyone objects I'll consider this change request approved as well. |
Consider it approved. |
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 - #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
The accepted values for the theme properties can be -
None
,System
,Light
andDark
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
Window.Theme = System / Light / Dark
it will take precdence overApplication.Theme
and we will load the Fluent styles for window. In case, ofSystem
Window will respond to System theme changes.Application.Theme = System / Light / Dark
andWindow.Theme = None
then window will follow the application.Aero2
by default.None
, then Accent colors don't come into play.Theme
properties isNone
.Alternative Designs
No response
Risks
No response
cc: @dotnet/dotnet-wpf-triage @pomianowski
The text was updated successfully, but these errors were encountered: