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

Uno Doesn't Find Styles in Themes\Generic.xaml #4424

Open
10 of 24 tasks
robloo opened this issue Oct 30, 2020 · 13 comments
Open
10 of 24 tasks

Uno Doesn't Find Styles in Themes\Generic.xaml #4424

robloo opened this issue Oct 30, 2020 · 13 comments
Labels
difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/bug Something isn't working project/styling 👔 Categorizes an issue or PR as relevant to element styling

Comments

@robloo
Copy link
Contributor

robloo commented Oct 30, 2020

Current behavior

Any styles defined in Themes\Generic.xaml are not found.

Expected behavior

This is the standard location for default styles in UWP apps and needs to be supported. It is also common practice to use MergedDictionary to pull in resources from other directories referenced by Generic.xaml.

When Uno searched for an implicit/default style for any TemplatedControl defined in Generic.xaml it should be found.

How to reproduce it (as minimally and precisely as possible)

As described.

Workaround

Define styles in App.xaml. Uno correctly handles nested MergedDictionaries so the following works:

<Application
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
    <Application.Resources>
        <ResourceDictionary>
            <ResourceDictionary.MergedDictionaries>
                <XamlControlsResources
                    xmlns="using:Microsoft.UI.Xaml.Controls" />
                <ResourceDictionary
                    Source="ms-appx:///Themes/Generic.xaml" />
            </ResourceDictionary.MergedDictionaries>
        </ResourceDictionary>
    </Application.Resources>
</Application>

Environment

  • Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
  • Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
  • Uno.SourceGenerationTasks
  • Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
  • Other:

Nuget Package Version(s): 3.1.6

Affected platform(s):

  • iOS
  • Android - only one tested, likely all affected
  • WebAssembly
  • WebAssembly renderers for Xamarin.Forms
  • macOS
  • Skia
    • WPF
    • GTK (Linux)
    • Tizen
  • Windows
  • Build tasks
  • Solution Templates

IDE:

  • Visual Studio 2017 (version: )
  • Visual Studio 2019 (version: )
  • Visual Studio for Mac (version: )
  • Rider Windows (version: )
  • Rider macOS (version: )
  • Visual Studio Code (version: )

Relevant plugins:

  • Resharper (version: )

Anything else we need to know?

@robloo robloo added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification labels Oct 30, 2020
@robloo
Copy link
Contributor Author

robloo commented Oct 30, 2020

@davidjohnoliver Is this an easy one to add support for? It will help a lot of apps coming over from UWP and seems pretty important it should work right out of the box.

If you can provide a general idea of where to modify code I can look at this as well.

@davidjohnoliver
Copy link
Contributor

@robloo This is going to be a long answer :p

First of all, there might be a partial solution which addresses the bug you encountered (which might be the most pressing issue in real-world cases): detect if there's a Themes/Generic.xaml in the application, and make any styles in it globally available. Some limited patch is definitely a possibility, so long as it doesn't introduce regressions.

The full problem

First, about styles from FrameworkElement's perspective. The code in Uno is pretty close to what UWP does in this regard. There are 3 levels of styles that I refer to as 'explicit', 'implicit' and 'default.'

The 'explicit style' is whatever is explicitly set to the frameworkElement.Style property. The remaining two are best illustrated by example.

Take the following:

	<Page.Resources>
		<Style TargetType="Button">
			<Setter Property="Background"
					Value="Fuchsia" />
		</Style>
	</Page.Resources>

	<StackPanel Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
		<TextBlock Text="Hello, world!"
				   Margin="20"
				   FontSize="30" />
		<Button Content="Implicit style" />
		<Button Content="Explicit style">
			<Button.Style>
				<Style TargetType="Button">
					<Setter Property="Foreground"
							Value="ForestGreen" />
				</Style>
			</Button.Style>
		</Button>
	</StackPanel>

We're defining an 'implicit style' for Button. (The behaviour would be the same if we defined it in Application.Resources.) Note that the explicit style for the 2nd button replaces the implicit style (it has a green foreground but default gray background). But both implicit and explicit styles are additive with the 'default style'. Ie, both Buttons are inheriting the correct Template with visual states, etc, as well as all other properties set by the default framework style for Button.

The main thing I don't fully understand, and needs to be assessed to handle Theme/Generic.xaml properly, is exactly when <Style TargetType="ns:SomeControl"/> counts as an implicit style, and when it counts as a default style, across all possible cases.

By 'all possible cases' I mean that SomeControl could be defined in 3 different places:

  • the framework (Uno.UI)
    • class library (SomeLibrary)
      • app itself (MyApp)

The <Style TargetType="ns:SomeControl"/> declaration could be anywhere downstream from where it's defined (inclusive). It could be detected via FrameworkElement.Resources or Application.Resources (including recursive MergedDictionaries), or it could be detected 'magically' via Themes/Generic.xaml. The use of the control could be anywhere dowstream from where it's defined, inclusive. (Eg in the template of another control, or a Page defined an a class library, or of course in MyApp itself.)

Essentially it needs to be understood, for all combinations of type definition/style declaration/usage, whether the declared style is treated as a 'default style' or an 'implicit style'. For now Uno takes the position that a 'default style' can only be declared in the same assembly where a control is declared, but that might too simplistic, it needs to be fully investigated.

Relevant Uno code

Currently in Uno, default style registrations are generated by XamlCodeGeneration. Note that there's a subtlety that MyApp.GlobalStaticResources.RegisterDefaultStyles() is not called.

Implicit styles are resolved here.

As far as testing, there are some tests already here, and Uno.UI.Tests.ViewLibrary is set up to be able to test how code and Xaml coming from a class library is treated.

@robloo
Copy link
Contributor Author

robloo commented Oct 30, 2020

@davidjohnoliver Thanks for the response :) I'll have to read through it in more detail later. A few quick comments though:

The main thing I don't fully understand, and needs to be assessed to handle Theme/Generic.xaml properly, is exactly when <Style TargetType="ns:SomeControl"/> counts as an implicit style, and when it counts as a default style, across all possible cases.

My understanding is what is in Themes/Generic.xaml is the default style. The location itself is the differentiator. This is why it is a magic location similar to the Strings folder. Any styles here should be at the bottom of the precedence though and can be overridden anywhere downstream.

Style precedence is simply whichever is closest to the control in the following order:

  • Generic.xaml Including any MergedDictionaries contained within
  • App.xaml Resources
  • Page.Resources / <ParentControl>.Resources
  • <Control>.Style

@jeromelaban jeromelaban added project/styling 👔 Categorizes an issue or PR as relevant to element styling and removed triage/untriaged Indicates an issue requires triaging or verification labels Nov 2, 2020
@robloo
Copy link
Contributor Author

robloo commented Nov 12, 2020

@davidjohnoliver Another property recently came to my attention I've never used before: Control.DefaultStyleResourceUri. So it seems to make sense that Generic.xaml should be used for the default style unless overridden by this property. Does any of this make sense?

@davidjohnoliver
Copy link
Contributor

davidjohnoliver commented Nov 12, 2020

@davidjohnoliver Another property recently came to my attention I've never used before: Control.DefaultStyleResourceUri. So it seems to make sense that Generic.xaml should be used for the default style unless overridden by this property. Does any of this make sense?

Interesting! I'd never noticed that property before. Certainly seems like a piece of the puzzle.

Style precedence is simply whichever is closest to the control in the following order

Except that it isn't simple in practice. The thing is that saying "Style A takes precedence over Style B" is too vague, because it could mean:

  • if Style A is present, Style B is completely ignored, or
  • if Style A is present, Style A's Setters take precedence over Style B's Setters

WinUI's style resolution rules combine both types of precedence, as my markup example above was trying to illustrate.

@robloo
Copy link
Contributor Author

robloo commented Nov 12, 2020

@davidjohnoliver Yes, I understood your point :) I guess I also confused things by talking about a few different concepts at once. Let me start by just talking about default styles and then talk about each one individually.

I believe Uno does not correctly handle default styles. It would also explain the confusion of when to consider a style default or implicit.

Default Style

The base style that is ALWAYS applied to a control. This may then be merged (as the base) with any implicit or explicit style later on.

WPF, and then UWP, very much use a 'magic' location of Themes/Generic.xaml and this is where the default styles come from -- at least for the Application. That is how they are differentiated from implicit styles.

However, of course this location can be modified in the assembly information as well. At least in WPF it was possible to modify AssemblyInfo.cs: [assembly: ThemeInfo(ResourceDictionaryLocation.None, ResourceDictionaryLocation.SourceAssembly )] I haven't checked if UWP has an equivalent. For a framework like Uno it's probably OK to hard-code this location behind the scenes for framework controls-only. (App default styles still need to be resolved correctly)

This default style location can also be modified at the control-level (as opposed to the assembly-level) using the Control.DefaultStyleResourceUri property in UWP mentioned above.

Implicit Style

Any style just using TargetType is implicit. This will be applied ON TOP of the default style as you said. Searching for implicit styles is done as I mentioned before a few comments ago, minus Generic.xaml which is likely only for default or explicit styles.

  • App.xaml Resources and any merged dictionaries within. Order is likely important and only the last-defined implicit style would be used (havn't checked UWP on that last point).
  • Page.Resources / <ParentControl>.Resources

Note that an implicit style is only applied on top of the default style once. You only have to merge styles once here assuming there is no BasedOn property value.

  • The implicit style's setters take precedence over the default style's setters

Explicit Style

Any style that is used directly in a control by specifying the style key or setting the Control.Style property.

Note that explicit styles may be defined anywhere:

  • Generic.xaml Including any MergedDictionaries contained within
  • App.xaml Resources Including any MergedDictionaries contained within
  • Page.Resources / <ParentControl>.Resources
  • <Control>.Style

Explicit styles will override any implicit style but not the default style (as you stated). They will be merged on top of the default style.

  • If an explicit style is present, the implicit style is completely ignored
  • The explicit style's setters take precedence over the default style's setters

Other

  • By default only two styles will ever be merged together: default + implicit or default + explicit. However,
  • Any style may use BasedOn which will have to be taken into account as well. Styles with a key may be in any of the above 4-defined location. It is possible to chain multiple styles to merge together.

I'm probably missing a few cases but I'm quite sure a style resolution algorithm as described above would work to solve everything we've discussed.

@davidjohnoliver
Copy link
Contributor

@robloo Excellent summary :)

  • for the implicit style and explicit style I fully agree, and to my knowledge Uno handles this part pretty closely to UWP (and also for BasedOn);
  • for default style, Uno is certainly not doing the right thing in all cases. You're right that Themes/Generic.xaml is a 'magic' location with special relevance.
    What's an open question for me is whether it depends on where the type is defined. For example, in the limited testing I did, adding a style for Button in Themes/Generic.xaml of an application seemed to be completely ignored.

What would be a great first step would be some tests that shed some light on how Generic.xaml/default styles are handled in practice, either as a standalone solution or as (Ignored for now) tests in Uno.UI.Tests.

@robloo
Copy link
Contributor Author

robloo commented Nov 12, 2020

in the limited testing I did, adding a style for Button in Themes/Generic.xaml of an application seemed to be completely ignored.

This makes sense to me. I believe UWP ignores any "implicit" styles within Generic.xaml because, well, there are no implicit styles supported there. They can't be as there would be no way to differentiate them from default styles. Only the default styles are defined within Generic.xaml but any styles there may also have a key and be used explicitly later on.

Also, in a standard UWP solution, adding a new TemplatedControl will add Themes/Generic.xaml automatically and create the default style there for you. All templated controls within a UWP app or controls library search for the default styles there and pull them in automatically. I first noticed this issue for custom controls that were not getting their default style applied by Uno -- I was never putting implicit or explicit styles in Generic.xaml.

@jeromelaban jeromelaban added the difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. label Feb 15, 2021
@MartinZikmund MartinZikmund added difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI and removed difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Jun 4, 2021
@MartinZikmund MartinZikmund added this to the 3.9 milestone Jun 4, 2021
@cmonto
Copy link

cmonto commented Jul 27, 2021

I was able to make Themes\Generic.xaml work from a multi-targeted class library (containing some custom controls) for Android and WinUI3. The Project structure is given below in case this helps others:

<Project Sdk="MSBuild.Sdk.Extras/3.0.23">
  <PropertyGroup>
    <TargetFrameworks>MonoAndroid10.0;net5.0-windows10.0.19041.0</TargetFrameworks>
  </PropertyGroup>
  <PropertyGroup Condition=" $(TargetFramework.Contains('windows')) ">
	<TargetPlatformMinVersion>10.0.17763.0</TargetPlatformMinVersion>
	<RuntimeIdentifiers>win10-x86;win10-x64;win10-arm64</RuntimeIdentifiers>
	<DefineConstants>WINDOWS_WINUI3;$(DefineConstants)</DefineConstants>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Uno.WinUI" Version="3.8.13" />
  </ItemGroup>
  <ItemGroup Condition=" $(TargetFramework.Contains('windows')) ">
	<PackageReference Include="Microsoft.ProjectReunion" Version="0.8.1" />
	<PackageReference Include="Microsoft.ProjectReunion.Foundation" Version="0.8.1" />
	<PackageReference Include="Microsoft.ProjectReunion.WinUI" Version="0.8.1" />
  </ItemGroup>
  <ItemGroup Condition=" $(TargetFramework.Contains('windows')) == 'false'">
    <Page Include="Themes\Generic.xaml">
      <SubType>Designer</SubType>
      <Generator>MSBuild:Compile</Generator>
    </Page>
  </ItemGroup>
</Project>

I found that Generic.xaml does not work from a shared source project for non-Windows platforms. It has to be packaged in a class library as shown above.

@jeromelaban jeromelaban removed this from the 3.9 milestone Aug 11, 2021
@MartinZikmund
Copy link
Member

@Youssef1313 can this be addressed for single project now?

@MartinZikmund
Copy link
Member

To find WinUI's approach, search for "themes/generic.xaml" in WinUI 3 sources

@agneszitte
Copy link
Contributor

agneszitte commented Sep 12, 2024

@jeromelaban, @MartinZikmund, @Youssef1313 regarding this issue, based on what was shared recently here #18157 (comment), can this be addressed easily lately? And the priority of doing it needs to be reviewed lately?

@mcNets
Copy link
Contributor

mcNets commented Nov 28, 2024

As a workaround Generic.xaml can be moved to another folder (e.g. Styles) and then add Generic.Xaml to the global merged dictionaries in App.xaml. I tested it on WASM, Desktop and Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/bug Something isn't working project/styling 👔 Categorizes an issue or PR as relevant to element styling
Projects
None yet
Development

No branches or pull requests

7 participants