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

Proposal: Improve the XAML styling system to target template attributes and states #279

Open
kikisaints opened this issue Feb 7, 2019 · 22 comments
Assignees
Labels
area-Styling feature proposal New feature proposal team-Controls Issue for the Controls team

Comments

@kikisaints
Copy link

kikisaints commented Feb 7, 2019

Proposal: Improve the XAML styling system to target template attributes and states

Summary

Provide a simplier styling system within XAML that enables a user to customize animations, template properties and attributes, and visual states without the need to re-template or copy numerous resources to a dictionary.

Rationale

Styling or customizing XAML controls today, outside of properties on a control, is very dependent on the developer's knowledge of the control's template and our framework. Although having a deep knowledge of our system isn't a bad thing, it can lead to some unwanted workarounds when it comes to customization and branding our controls.

There are two major sticking points that developers can run into when customizing our controls:

  • Desire to change visuals during a state (colors, attributes, ContentPresenter properties, etc.)
  • Need to alter default template attributes (spacing, corner radius, fixed widths, etc.)

To do either of these the above customization, a developer would need to maintain ResourceDictionary(ies) with hundreds of resources (if changing colors and/or control properties), or take complete ownership of the control through re-templating (if changing attributes).

Having numerous resources to maintain during design changes is a lot of developer cost and overhead, while the re-templating option means a higher risk of breaking high contrast as well as tedious, file diff changes when we roll out new updates to our default control templates.

With so many ways to customize a control, it's almost impossible for us to enable a property for each permutation that a developer could need. Therefore, it would make more sense to simply improve our current styling system to enable more versatility.

Functional Requirements

# Feature Priority
1 Easily access control template properties, components and attributes within a style Must
2 Customize controls based on the template markup, not through resource keys or APIs Must
3 Gracefully fallback or throw exceptions when template attribute markup has changed Must
4 Anything not specified in the (non-template) style will default to their generic.xaml definition(s) Must
5 Can target any visual state within a style Must
6 Animations can be added, removed, or changed within a style Must
7 Does not remove or make obsolete today's method of styling, instead offering a lighter weight alternative Must
8 Intellisense supports styling system Should

Important Notes

Let's look at common styling scenarios and how we might be able to change the way we can style them.

Styling a Control Locally

Styling a control is simplified and it's easy to target the named parts within that control's template.

<RadioButton Content="RadioButton">
    <RadioButton.Style>
        <Style TargetType="RadioButton">
            <Setter Path="Background" Value="White"/>
            <Setter TargetName="CheckOuterEllipse" Property="Fill" Value="Green"/>
            <Setter TargetName="CheckOuterEllipse" Property="Width" Value="18"/>
       </Style>
   </RadioButton.Style>
</RadioButton>

Changing Properties on Visual States

When a developer wants properties to changed only within a state, they can target that state directly without needing to specify the entire VisalState tree/manager.

<RadioButton Content="RadioButton">
    <RadioButton.Style>
        <Style TargetType="RadioButton">
            <VisualState x:Name="PointerOver">
                <Setter Path="Background" Value="White"/>       
            </VisualState>

            <Setter TargetName="CheckOuterEllipse" Property="Fill" Value="Green"/>
            <Setter TargetName="CheckOuterEllipse" Property="Width" Value="18"/>          
        </Style>
    </RadioButton.Style>
</RadioButton>

Since the style defined above has no states for Pressed and Disabled, the button control in this case will use it's generic.xaml definitions for those states, and the PointerOver definition specified above for hover.

The more states defined in the style, the less is being defaulted to in generic.xaml.

Styles at a Page-Level

This is the same behavior today and I am calling it out as something we will maintain with this new styling system as well.

Just as you can define styles and templates at a page-level, this new styling system can also be defined there.

<Page.Resources>
    <Style TargetType="RadioButton">
        <Setter Path="Background" Value="White"/>
        <Setter TargetName="CheckOuterEllipse" Property="Fill" Value="Green"/>
        <Setter TargetName="CheckOuterEllipse" Property="Width" Value="18"/>         
    </Style>
</Page.Resources>

Specifying Animations

Animations specified using the improved styling method.

<Style TargetType="Button">
    <VisualState x:Name="PointerOver">
        <Storyboard>
            <PointerUpThemeAnimation Storyboard.TargetName="ContentPresenter" />
        </Storyboard>
    </VisualState>

    <Setter Path="Background" Value="White"/>
    <Setter TargetName="ContentPresenter" Property="CornerRadius" Value="4"/>
</Style>

Styling in Code-Behind

This new styling method would also modify how we style controls from code-behind. Although similar to what can be done today, there would need to be a few changes to account for visual states and targeting template attributes.

//Create style and add a PointerOver state using a previously defined storyboard and setters
Style style = new Style(typeof(AppBarButton));
style.Setters.Add(new Setter(AppBarButton.ContentViewbox.Height, "14"));
style.VisualStates.Add(new VisualState("PointerOver")
    { Setters = mySetterBaseCollection, Storyboard = myStoryboard });

//Add that style to container (or page) resources
rootGrid.Resources.Add(typeof (AppBarButton), style);

Intellisense Support

Intellisense will also scan and pick up the named parts within a control's template and display them in a Setter's property option dropdown. This would remove the need to dig through generic.xaml and would greatly increase the speed of app development and productivity.

image

Using this Path attribute will also allow "dotting" into the named element within that template and accessing it's properties directly.

image

Open Questions/Issues

  • In this model, when we do change our default template attributes, the styles relying on those x:Key names will no longer be valid.
    Even if we do fallback gracefully and intellisense detects which attributes don't exist anymore, it could still be a bit tedious for the developer to update those attribute names individually.
  • This includes a major change to our VisualStates and how they're architected.
    VisualStates as they are today were built to help immensely with VisualStudio's tooling features, something we would not want to regress if we were to modify how and where VisualStates can be defined.
  • How would this work or use XAML direct?
@kikisaints kikisaints added the feature proposal New feature proposal label Feb 7, 2019
@msft-github-bot msft-github-bot added this to New proposal in Feature tracking Feb 7, 2019
@msft-github-bot msft-github-bot added this to Needs triage in Controls Triage Feb 7, 2019
@michael-hawker
Copy link
Collaborator

This sounds like an interesting idea!

But how do you distinguish between overriding the style and only having a couple of properties vs. changing a few properties from the base style?

@chrisglein
Copy link
Member

But how do you distinguish between overriding the style and only having a couple of properties vs. changing a few properties from the base style?

I'm not sure I follow. Can you give an example?
What's outlined here isn't a fundamental change to how styles layer in XAML. More it's a suggestion for how to target properties that aren't on the control class itself (by allowing you to target known named parts of the template and the sub-properties on them).

@bschoepke
Copy link

bschoepke commented Feb 8, 2019

@kikisaints This looks great, and would go even further than "lightweight styling" did to reduce the need for re-templating. My only comment so far is that I would like to be able to replace things like PointerDownThemeAnimation with a Windows.UI.Composition-based animation.

I think any major Styling change like this would also have to take into account how you could do things like add drop shadows and other effects to specific elements if the built-in ControlTemplate doesn't have them.

I'll have to digest this a little before I can comment more.

@jevansaks jevansaks removed this from Needs triage in Controls Triage Feb 8, 2019
@thomasclaudiushuber
Copy link
Contributor

thomasclaudiushuber commented Feb 8, 2019

@kikisaints Nice idea. The first two snippets in your proposal confused me a bit, as I guess some elements are missing? :-)

First snippet

In the first snippet the element <Style TargetType="RadioButton"> is missing.

Should be

<RadioButton Content="RadioButton">
    <RadioButton.Style>
      <Style TargetType="RadioButton">
        <Setter Path="Background" Value="White"/>
        <Setter Path="CheckOuterEllipse.Fill" Value="Green"/>
        <Setter Path="CheckOuterEllipse.Width" Value="18"/>
      </Style>
   </RadioButton.Style>
</RadioButton>

instead of

<RadioButton Content="RadioButton">
    <RadioButton.Style>
        <Setter Path="Background" Value="White"/>
        <Setter Path="CheckOuterEllipse.Fill" Value="Green"/>
        <Setter Path="CheckOuterEllipse.Width" Value="18"/>
   </RadioButton.Style>
</RadioButton>

2nd snippet

The property element <RadioButton.Style> is missing:

Should be

<RadioButton Content="RadioButton">
  <RadioButton.Style>
    <Style TargetType="RadioButton">
        <VisualState x:Name="PointerOver">
            <Setter Path="Background" Value="White"/>       
        </VisualState>

        <Setter Path="CheckOuterEllipse.Fill" Value="Green"/>
        <Setter Path="CheckOuterEllipse.Width" Value="18"/>            
    </Style>
  </RadioButton.Style>
</RadioButton>

instead of

<RadioButton Content="RadioButton">
    <Style TargetType="RadioButton">
        <VisualState x:Name="PointerOver">
            <Setter Path="Background" Value="White"/>       
        </VisualState>

        <Setter Path="CheckOuterEllipse.Fill" Value="Green"/>
        <Setter Path="CheckOuterEllipse.Width" Value="18"/>            
    </Style>
</RadioButton>

Maybe you can adjust it in your proposal if I'm right. :)

Suggestion: Instead of a Path property it might be better to create a TargetName property on the Setter class

I understand that you call it "Path" because you access a property of a named element, which is a kind of property path. But here path is a bit confusing:

<Setter Path="Background" Value="White"/>       

This should be

<Setter Property="Background" Value="White"/>       

I would keep the Property-Property and I wouldn't introduce a new Path property on the Setter class. To access elements, I would create an additional property on the Setter class called TargetName (like WPF has already):

Instead of this like in your suggestion

<Setter Path="CheckOuterEllipse.Fill" Value="Green"/>

you would write this:

<Setter TargetName="CheckOuterEllipse" Property="Fill" Value="Green"/>

In my opinion this looks a bit cleaner. In addition, bringing in this concept with the TargetName property would allow us to have Trigger support in UWP at a later stage that has the same syntax like in WPF. If you start something new with a "Path"-property, that wouldn't work anymore.

Feedback on the idea :)

In general, as I wrote already, I like this idea. And actually, it was just last week when I would have had a use case for your idea. :)

The requirement was to change the check mark of a CheckBox to a simple cross when the CheckBox is checked. This is not possible in UWP without creating a copy of the ControlTemplate and changing the Glyph-property of the FontIcon element in the ControlTemplate. The FontIcon in the ControlTemplate has the name CheckGlyph. So, with your idea and my adjustment mentioned in this post I could have done this to display a cross (&#xE711) when the CheckBox is checked:

<CheckBox Content="Is developer">
  <CheckBox.Style>
    <Style TargetType="CheckBox">
      <Setter TargetName="CheckGlyph" Property="Glyph" Value="&#xE711;"/>
    </Style>
  </CheckBox.Style>
</CheckBox>

Nice!

@thomasclaudiushuber
Copy link
Contributor

Something we need to be aware of:

This feature means that the code of many developers will have dependencies to the named elements that are inside the ControlTemplates. This means if you want to change a ControlTemplate of a Control in the platform, you could break a lot of things. And with this feature enabled this is also true for any control library developer.

That means that this idea makes it much easier for the developer who uses the control, let's call this developer the Control Consumer. But if the Control Creator wants to update their default ControlTemplates, it will break things if the named elements are no more there. That means updating a default ControlTemplate gets hard. And from the Control Creator's point of view it would have been better if the Control Consumer would have created a copy of the whole ControlTemplate, which is the case that we have today. :)

So, there are two sides to consider. It looks like a really nice idea, but it has also a few drawbacks when a control creator wants to bring out a new default ControlTemplate. And maybe there are other things we need to be aware of.

@chrisglein
Copy link
Member

I would keep the Property-Property and I wouldn't introduce a new Path property on the Setter class. To access elements, I would create an additional property on the Setter class called TargetName (like WPF has already):

My assumption is that Path would support more than just TargetName.Property. As in instead of just A.B it would support A.B.C, if that makes sense. If there aren't any scenarios that require that then describing it as a 'Path' might be overkill.

@chrisglein
Copy link
Member

This feature means that the code of many developers will have dependencies to the named elements that are inside the ControlTemplates.

This is definitely true, and should be handled with intention. There's one callout to it in the requirements list:

Gracefully fallback or throw exceptions when template attribute markup has changed

The first thing that comes to mind for me are C#'s null-conditional operators. This sort of Path isn't targeting a real control type, only the template. Which as you mention is completely changeable. So there needs to be a graceful fallback when the element isn't present and/or the ability to customize that fallback. And almost certainly warnings when you are targeting an element and it is not found so if there's a silent fallback you can still find and correct this.

I think there's room for that kind of "soft dependency" though, knowing the risks. To me it seems better to enable that sort of dependency on the template rather than have so many outcomes where you're unable to write a feature without copying the entire template. For many controls those named elements aren't going anywhere. It does mean that control authors that expect this type of customization need to take care specifically when changing named elements.

@mdtauk
Copy link
Contributor

mdtauk commented Feb 11, 2019

If this is a fundamental look at how Styling should work, perhaps if there are any errors, the control should fall back onto the default template - with error/warning highlighting with debugging and Intellisense.

@lukasf
Copy link

lukasf commented Feb 11, 2019

I like this proposal a lot!

And I agree with @thomasclaudiushuber that it should be TargetName and Property. This would be more in line with other style elements. Path is always a property path until now, and it should stay that way. In the original proposal, it would be a mixture of element name and property path, which is irritating. And it would cause conflicts when there is also a real property on the style target which is named like the TargetElement of the setter.

@jevansaks jevansaks added this to Approved in Controls Triage Feb 11, 2019
@kikisaints
Copy link
Author

@lukasf The original intent to use Path with "dotting" was to reduce the verbosity of the styntax for styling and to potentially support situations like Chris has already mentioned - "dotting" more that one property deep:

<Setter Path="SliderThumbStyle.Border.CornerRadius" Value="0"/>

Harder to do if you only have one property available to you... but perhaps we could do this with nested setters. Might get messy, however.

@lukasf
Copy link

lukasf commented Feb 14, 2019

@kikisaints I don't know if it is true for 100% of controls, but usually all elements inside a stock ControlTemplate do have a name. So we can access and change every property of every element in the ControlTemplate using a setter like this:

<Setter ElementName="HorizontalThumb" Property="Width" Value="20"/>

For the example you posted, the SliderThumbStyle is not even an element in the ControlTemplate, but a style that is imported via StaticResource binding. I don't even know how your syntax whould know how to find it. And then how would it know that the next thing in the path is again a named element from the ControlTemplate?

If you wanted to change that SliderThumbStyle, you could inherit and override the style, and then use a setter like above to access the Border by ElementName and set its Properties. So it is well possible to achive it without having the combined path syntax.

I just think that it will be difficult and irritating if the same path syntax can access ControlTemplate elements by name, but also Properties, or even basically any element from application resources. And it could also lead to conflicts if there is a named element which has the same name as a property. Plus, it would give the "Path" a meaning that is different from what it has now. ElementName, Property and Value are already well known in the framework and could accomplish the same job. I agree that it would be a bit more verbose, but it would still be a major improvement, and allow us to override things without copying the ControlTemplate, in a clean way.

@kikisaints
Copy link
Author

I see your point, and also that my example wasn't great - I was trying to portray that there may be scenarios where we would want to index more than two property layers deep, with the Element/TargetName, Property, and then Value syntax route we limit ourselves a little bit in that regard.

That being said, I don't have a good scenario that I can come up with at this time for why we would want to do the "dot" method - so until further notice I updated the original proposal to feature yours and Thomas' newer syntactical method of accessing template attributes.

@mdtauk
Copy link
Contributor

mdtauk commented Feb 14, 2019

I see your point, and also that my example wasn't great - I was trying to portray that there may be scenarios where we would want to index more than two property layers deep, with the Element/TargetName, Property, and then Value syntax route we limit ourselves a little bit in that regard.

That being said, I don't have a good scenario that I can come up with at this time for why we would want to do the "dot" method - so until further notice I updated the original proposal to feature yours and Thomas' newer syntactical method of accessing template attributes.

The problem as I see it, is how the control will function if it has been re-templated and elements renamed, removed, or changed. The Style dot naming will throw up an error.

@chrisglein
Copy link
Member

That being said, I don't have a good scenario that I can come up with at this time for why we would want to do the "dot" method - so until further notice I updated the original proposal to feature yours and Thomas' newer syntactical method of accessing template attributes.

Your SliderThumb.Border.Width example above was approaching this. If there's anything that people want to style that's a property of a property we'd need some sort of "Path" like option. Isn't x:Bind the better comparison here about using more code-like syntax? As much as I agree that usually it's only as far as ElementName.Property I wouldn't want us to box ourselves out of the possibility of one more level. It's not just about compact syntax.

The problem as I see it, is how the control will function if it has been re-templated and elements renamed, removed, or changed. The Style dot naming will throw up an error.

What if it didn't throw up an error? How bad does this get if it silently doesn't apply? Could we make this more robust by having some Requires tag/attribute that states what named/typed elements you expect so that you can author intentional fallbacks (if silent failure doesn't work for you)?

@mdtauk
Copy link
Contributor

mdtauk commented Feb 15, 2019

Silently not applying is fine, but it would be useful for debugging if there was a warning when style elements will not apply because of a mis naming, or a missing target. We can assume named parts are required and would break the whole control if removed from a ControlTemplate.

Would control states be accessible via this new Styling syntax? Pressed, Hover, Disabled, etc

@chrisglein
Copy link
Member

Silently not applying is fine, but it would be useful for debugging if there was a warning when style elements will not apply because of a mis naming, or a missing target. We can assume named parts are required and would break the whole control if removed from a ControlTemplate.

Agreed. I think a warning would be very appropriate.

Would control states be accessible via this new Styling syntax? Pressed, Hover, Disabled, etc

The proposal above does touch on that in the "Changing Properties on Visual States" section. By being able to specify a VisualState scope underneath the Style you could append styling rules onto the existing state machine. Do you think that'd be helpful?

@lukasf
Copy link

lukasf commented Feb 15, 2019

I do not think that the current proposal on VisualStates is fully fleshed out. VisualStates usually have multiple animations to change values. Having only property setters for VisualStates would be very limiting, because it would prevent us from using animations.

But maybe we don't even need special syntax for VisualStates? If also all elements inside these VisualStates, including each animation, would have x:Name set, then we could modify each of them using normal Setter with ElementName+Property syntax (or property path, if it is added). Edit: Plus, we could override the complete VisualState, if required.

Generally, if this feature would be implemented, then all stock styles would need to be checked and x:Name applied to every single element that does not currently have one. Then WinUI could ship those reworked styles, so we can fully use the new override mechanism, even on older Windows versions. Maybe this should be added to the feature list @kikisaints?

@kikisaints
Copy link
Author

@lukasf I wouldn't want this to fully replace our old styling system, at least not right off the bat. I will call out in requirements that this is an alternative to what we have today.

@thomasclaudiushuber
Copy link
Contributor

thomasclaudiushuber commented Feb 15, 2019

Hi all,

fantastic discussion here. I got the influenza this week, my wife and my kids too, and I enjoyed reading all of this discussion. I'm still running on 38.5°C, so I hope everything I write here is understandable. With my overclocked brain, I got a few new inputs and thoughts. ;-)

Path

I see that point that not using a Path could lock us out of some specific scenarios. But even with TargetName we could still add that specific case/property later:

<Setter TargetName="SliderThumb" Path="Border.Width"/>

I'm still not 100% sure if a Path would be a good thing for a setter or if there shouldn't be a completely different type, for example a PathSetter.

Dependency Properties

It wasn't mentioned anywhere yet, but this new feature requires to rewrite the precedence behavior of Dependency Properties. A local value on an element has normally a higher precedence than a value that comes from a Style Setter. But this is not true for a local value of an element in a Template where you use that new Setter kind we discuss here. I haven't though deeply about it, but I think the feature like it is now involves a lot of work with Dependency Properties.

Adjusting values without a Style?

Sometimes you might have just a single Control that you want to adjust. With the feature proposal like it is now you have to create a Style to adjust the Control's Template. But wouldn't it be better if you could do it with or without style?

Mixing normal setters and template setters?

I'm not sure if it's good to mix setters in a Style

  • affecting the targeted element
  • affecting an element in the targeted element's Template

in the same Setters property of the Style. Maybe it's better to have something more explicit. Something like this:

<Style TargetType="Button">
  <Setter Property="Background" Value="White"/>
  <Style.TemplateOverrides>
    <VisualState x:Name="PointerOver">
      <Storyboard>
         <PointerUpThemeAnimation Storyboard.TargetName="ContentPresenter" />
       </Storyboard>
    </VisualState>

    <Setter TargetName="ContentPresenter" Property="CornerRadius" Value="4"/>
  </Style.TemplateOverrides>
</Style>

But this brings me to the next problem:

Styles are for FrameworkElements, not only for Controls

A Style can be applied to any FrameworkElement. But only Controls have a Template property. So in general, including logic to adjust the Template in a Style seems to abuse the Style for Controls while it is made for all FrameworkElements. A TextBlock for example is a FrameworkElement but no Control. You can style it, but you can't adjust a Template, as it isn't a Control.

That said, I think the logic to adjust parts of a template should come from the Control class itself, as it is the Control class that defines the Template property. It must be something like this:

TemplateOverrides property in the Control class

Just an idea that would solve:

  • that a Style is valid for all FrameworkElements, and not only Controls
  • that you can adjust the template with a Style, but also locally on a Control without a Style.
  • that the concept of Dependency Properties can stay as it is

The whole feature is done in the Control class.

public class Control 
{
  ...
  public TemplateOverridesCollection TemplateOverrides { get; set; }
}
<Style TargetType="Button">
  <Setter Property="Background" Value="White"/>
  <Setter Property="TemplateOverrides">
    <Setter.Value>
      <TemplateOverridesCollection>
        <TemplateOverride TargetName="ContentPresenter" Path="CornerRadius" Value="4"/>
      <TemplateOverridesCollection>
    </Setter.Value>
  </Setter>
</Style>

What are your thoughts?

@lukasf
Copy link

lukasf commented Feb 15, 2019

@kikisaints I was not saying that the styling system should be replaced. I just say that all UWP contol styles should be checked and x:Name should be applied to all elements in the ControlTemplate which currently do not have a name. This would not change any behavior. But it makes all elements accessible by the new overrides. These updated style definitions could be shipped with WinUI, to allow overrides on all supported versions. WinUI does already now ship new copies of all styles, to bring compact sizing to older Windows versions. So this is not something new.

@lukasf
Copy link

lukasf commented Feb 16, 2019

@thomasclaudiushuber Good point, style overrides should be available both in Xaml and Code-behind! Your proposal would sure solve this. We could then also have a second property VisualStateOverrides with a specialized collection that allows fully featured modification of VisualStates, including animations (we'd have to come up with a concept for how that works).

@thomasclaudiushuber
Copy link
Contributor

@lukasf I didn't mean Code-Behind. I meant also XAML, but locally without a Style. Like this:

<Button Content="My special Button, the one and only">
  <Button.TemplateOverrides>
    <TemplateOverridesCollection>
      <TemplateOverride TargetName="ContentPresenter" Path="CornerRadius" Value="4"/>
    <TemplateOverridesCollection>
  <Button.TemplateOverrides>
<Button>

Yes, I agree, there could be another property called VisualStateOverrides on the Control. But maybe it would be cleaner to have all in one place? For example, if we make TemplateOverridesCollection a TemplateOverrides class, it could be reasonable to have it directly there. What I think about looks like this:

<Button Content="My special Button, the one and only">
  <Button.TemplateOverrides>
    <TemplateOverrides>
      <TemplateOverrides.VisualStates>
        <!-- State overrides go here -->
      <TemplateOverrides.VisualStates>
      <TemplateOverride TargetName="ContentPresenter" Path="CornerRadius" Value="4"/>
    <TemplateOverrides>
  <Button.TemplateOverrides>
<Button>`

@jevansaks jevansaks moved this from Approved to Approved - Assigned in Controls Triage Feb 22, 2019
@jevansaks jevansaks moved this from Approved - Assigned to Approved in Controls Triage May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Styling feature proposal New feature proposal team-Controls Issue for the Controls team
Projects
Development

No branches or pull requests

8 participants