maui icon indicating copy to clipboard operation
maui copied to clipboard

TemplateBinding with CornerRadius on Border does not work anymore

Open WMLPB opened this issue 4 months ago • 7 comments

Description

Hi all,

I use my own Button control inherited from TemplatedView; with a CornerRadius property.

public static readonly BindableProperty CornerRadiusProperty =
      BindableProperty.Create(nameof(CornerRadius), typeof(CornerRadius), typeof(Button), new CornerRadius(6.0));

In the button’s ControlTemplate I have a Border with a RoundRectangle StrokeShape and a TemplateBinding to the before mentioned CornerRadius property.

<Border.StrokeShape>
      <RoundRectangle CornerRadius="{TemplateBinding CornerRadius}" />
</Border.StrokeShape>

And I have a Style to set the CornerRadius property.

<Style TargetType="e:Button" ApplyToDerivedTypes="True">
      <Setter Property="CornerRadius" Value="6,6,6,6" />
      <Setter Property="ControlTemplate" Value="{StaticResource ButtonTemplate}" />
</Style>

After updating to MAUI 8.0.20 this doesn’t work anymore – tested on Android. The button in the UI doesn’t have round corners anymore. If I remove the TemplateBinding and set the CornerRadius in the ControlTemplate directly, it works again.

<Border.StrokeShape>
      <RoundRectangle CornerRadius="6,6,6,6" />
</Border.StrokeShape>

I can live with this work around for the moment. Thank you very much for reading.

Steps to Reproduce

No response

Link to public reproduction project repository

No response

Version with bug

8.0.20 SR4

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.14 SR3.1

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

No response

WMLPB avatar Apr 10 '24 08:04 WMLPB

We run into the exact same issue that our TemplateBinding for a Border.CornerRadius does not work anymore after updating MAUI to version 8.0.20.

Regression issue:

  • Version it still worked: 8.0.14
  • Version this got broken: 8.0.20

We see this issue on all our supported platforms

  • Android (v14)
  • iOS (v17)
  • Windows (v10)

omghb avatar Apr 15 '24 14:04 omghb

@omghb can you please attach a repro?

PureWeen avatar Apr 15 '24 17:04 PureWeen

I managed to repro this, but looks to be the Border.CornerRadius specifically. I also tested with a label Text:

<VerticalStackLayout>
    <local:MyButton CornerRadius="10,10,10,10" Text="New Text">
        <local:MyButton.ControlTemplate>
            <ControlTemplate>
                <Border Stroke="#C49B33"
                        StrokeThickness="4"
                        Background="#2B0B98"
                        Padding="16,8"
                        HorizontalOptions="Center">
                    <Border.StrokeShape>
                        <RoundRectangle CornerRadius="{TemplateBinding CornerRadius}" />
                    </Border.StrokeShape>
                    <Label Text="{TemplateBinding Text}"
                        TextColor="White"
                        FontSize="24"
                        FontAttributes="Bold" />
                </Border>
            </ControlTemplate>
        </local:MyButton.ControlTemplate>
    </local:MyButton>
</VerticalStackLayout>
public class MyButton : TemplatedView
{
	public static readonly BindableProperty CornerRadiusProperty = BindableProperty.Create(
		nameof(CornerRadius), typeof(CornerRadius), typeof(MyButton), new CornerRadius(6.0),
		propertyChanged: (bindable, oldValue, newValue) =>
		{
			Debug.WriteLine($"CornerRadius changed from '{oldValue}' to '{newValue}'");
		});

	public static readonly BindableProperty TextProperty = BindableProperty.Create(
		nameof(Text), typeof(string), typeof(MyButton), "Default",
		propertyChanged: (bindable, oldValue, newValue) =>
		{
			Debug.WriteLine($"Text changed from '{oldValue}' to '{newValue}'");
		});

	public CornerRadius CornerRadius
	{
		get => (CornerRadius)GetValue(CornerRadiusProperty);
		set => SetValue(CornerRadiusProperty, value);
	}

	public string Text
	{
		get => (string)GetValue(TextProperty);
		set => SetValue(TextProperty, value);
	}
}

I see the text is applied, and the events for BOTH properties are firing. So, something is not moving from the property to the UI.

image

mattleibow avatar Apr 15 '24 17:04 mattleibow

If I make the entire StrokeShape property a template binding, it also works.

mattleibow avatar Apr 15 '24 19:04 mattleibow

Looks like this change: https://github.com/dotnet/maui/pull/21151/files#r1523373115

Seems that we used to:

AddLogicalChild(visualElement);

Now we

SetInheritedBindingContext(visualElement, BindingContext);

This makes sense as the stroke is not meant to be a "child" of the border, but rather is just a set of values.

mattleibow avatar Apr 15 '24 19:04 mattleibow

Seems that this is broken now that we are no longer making the shape a CHILD, and as a result, it never gets the PARENT that is used here:

https://github.com/dotnet/maui/blob/7f85d2a544f0cf6c60bccf2d746e2995208af6a7/src/Controls/src/Core/TemplateUtilities.cs#L11-L33

Basically, the templating system ONLY looks up the parent-child path, and we have removed the shape from that path.

mattleibow avatar Apr 15 '24 20:04 mattleibow

@PureWeen @StephaneDelcroix was the change in #21151 to no longer parent the shape the correct one? If it was correct, should there be an alternate path for non-parented-but-still-logical-children? Is that just the normal logical children and there is a flaw in the way the logical/virtual child trees are being re-used?

mattleibow avatar Apr 15 '24 20:04 mattleibow

In this change: https://github.com/dotnet/maui/pull/21151/files#r1523373115 We stopped using the AddLogicalChild method which is responsible for setting the Parent, inheriting the BindingContext, subscribing to ResourcesChanged. In that same PR we apply changes to set the BC and subscribing to ResourcesChanged but we never set the Parent. Without setting the parent DynamicResources or TemplateBinding won't work. Created a PR here https://github.com/dotnet/maui/pull/21997 setting the Parent using a WeakRef but is not enough. Using a StrokeShape from resources with several Borders, each time that reference is being used we are overriding the previous parent. See #https://github.com/dotnet/maui/issues/18925#issuecomment-1824058832

Thoughts?, Ideas?

For example, use a weak reference to set the parent, and also implement x:Shared Attribute - XAML | Microsoft Learn

jsuarezruiz avatar Apr 23 '24 13:04 jsuarezruiz

Using x:Shared may be a good idea as long as there's an analyzer reporting an error when it's not used on values subclassing BindableObject. Otherwise the only option is a deep clone like it's done here https://github.com/dotnet/maui/blob/994117b2070f4ba98bf2acc675ba898328cf1732/src/Controls/src/Core/Setter.cs#L78

albyrock87 avatar Apr 23 '24 15:04 albyrock87

This comment was really good a few months* ago and come with two solutions, we applied one, what do you think of the second one? (Solution B): https://github.com/dotnet/maui/issues/18925#issuecomment-1824058832

JulioVillanueva avatar Apr 25 '24 07:04 JulioVillanueva

Follow up of my comment above. https://github.com/dotnet/maui/issues/21747#issuecomment-2072766914

We can introduce a new interface:

public interface IStyleSetterValueProvider {
    object ProvideValue();
}

This interface would be explicitly implemented in VisualStateGroups class and *Shape classes. The method will basically return a deep clone.

When Setter encounters this kind of values, it will use the returned value of that method.

Well, we can find better names, I just wanted to give an idea.

albyrock87 avatar Apr 25 '24 08:04 albyrock87