[Epic] Lightweight Styling #854
Comments
|
the proposed naming convention is expanded from the current style naming convention: eg: note: most styles are also aliased with a new key without the {design} prefix, so "MaterialFilledButtonStyle" can also referenced with "FilledButtonStyle" key. |
|
Something to consider here, overriding these resources is going to be tricky. If my Button Style is referencing MaterialFilledButtonBackground, and I override MaterialFilledButtonBackground within my App.xaml, the aliased FilledButtonBackground resource will continue to resolve to the original value of MaterialFilledButtonBackground and not the overridden value. The same issue applies in the other direction. If I were to override FilledButtonBrush in my App.xaml, the MaterialFilledButtonBrush will not resolve to the new overridden value, only usages of the shared key, FilledButtonBrush, are affected by the override. Right now, the Material styles contain a mishmash of resource references using both the Material specific keys but also the aliased "shared" keys. <Setter Property="Foreground"
Value="{StaticResource PrimaryBrush}" />
<Setter Property="BorderThickness"
Value="{StaticResource MaterialOutlinedButtonBorderThickness}" />
<Setter Property="BorderBrush"
Value="{StaticResource MaterialOutlinedButtonBorderBrush}" />We need to figure out how we should be dealing with overrides. Should all Material styles only reference Material-specific resource keys? So, we'd use MaterialPrimaryBrush, MaterialOutlinedButtonBorderThickness, etc. In that case, we would provide aliases for these resources, but you would need to specifically override the resources using the Material prefix in order for the included styles to adapt properly. Overriding MaterialPrimaryBrush from Purple to Red will be reflected in the styles but any place that is directly referencing the aliased shared key of PrimaryBrush will still show Purple. Otherwise, we get rid of the Material/Cupertino prefixes for resources, remove all the aliases, and they just get redefined in the Uno.Cupertino and Uno.Material libraries. There will have to be some special cases for resources that may be specific to one design system. In this case, we can reference PrimaryBrush, OutlinedButtonBorderThickness , etc. and those keys would also be the ones to override. I feel like maintaining the design system prefixes are kind of pointless. Unless there are some resources that exist in only one design-system, then those can be specially documented usages with the Material/Cupertino prefix. Problem with getting rid of the prefix is that it'll be a big breaking change for anyone that is currently using the Material prefixed keys. For the most part, people would most probably be using the shared aliases, but I'm assuming this'd need to be introduced as a new major version. @francoistanguay @Xiaoy312 @jeromelaban thoughts? |
|
Assuming current users don't override styles using Material-prefixed resources (which currently does not work with Uno), we could invert their definitions for the generic ones to be the original styles and the material prefixed ones to be the aliases. We may also need to consider the scenario about the resolution in the gallery app, which contains both definitions simultaneously (if included at the app.xaml level), but may still resolve properly when using material-prefixed styles. |
|
Yeah I've already inverted the font resources in my branch: TypographyOverrides.xaml (should be called aliases, not overrides) For the actual Styles themselves I don't think this is going to be an issue, it's more for the resources that the styles reference. Like for the Typography changes in the above links, I changed all usages of those resources within the styles to the non-prefixed/non-aliased ones. So either,
My feeling is people should not use prefixed resource keys within the app, same way they should be using the non-prefixed style keys. Only thing that messes things up are apps like Gallery where you need a way to directly reference the proper design system resources. |
|
FYI for the issue of resource aliases not reflecting the overridden values, it seems that this is a known issue and pain point: microsoft/microsoft-ui-xaml#2913 |
|
Looking back on this, we should be going with the original design and allowing lightweight styling at a per-control level. Where we would be able to override something like a Brush in the Material style for NavigationViewItem called Proposal updated and copied to the original post: Proposed naming convention:
|
|
Closing this epic issue as the only left is #1063 which is already in progress |
Instead of having to override the ControlTemplate of any given Material/Cupertino style we should allow for Lightweight Styling
All styles should be refactored to reference resource keys that follow an accepted standard naming scheme.
All Default styles (those without a
style-variantshould follow the resource key naming coming from the matching Fluent themeresourcesProposed naming convention:
{?:style-variant}{control-name}{member}{?:visual-state}
style-variantcontrol-namemembervisual-stateVisualStatethat this resource will be applied to (PointerOver, Checked, Disabled, etc.)Ensure all color resources for the controls and their visual states follow the design specs from our Uno Material Figma file
Required Styles
EDIT: 2023-05-30
style-variant), we should be using the same key names as the ones coming from the Fluent resources as a base. We can go ahead and create additional lightweight resource keys, but we should at least be supporting the ones coming from Fluent. Example: the CheckBox style in Material should define the same resource keys that are defined here, as well as any additional keys that may be useful to have. Certain extra lightweight resource keys can be created that are specific to the Material template, and in those cases we would prefix those with "Material". Example:The text was updated successfully, but these errors were encountered: