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

Always propagate the BC to the ControlTemplate #26072

Merged
merged 4 commits into from
Nov 27, 2024
Merged

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Nov 22, 2024

Description of Change

With the initial design of Xamarin.Forms the BC would never propagate to the ControlTemplate as we can see from the test DoesNotInheritBindingContextToTemplate that you can reference inside this PR.

I'm not really clear on the initial reasoning here. I've compared the behavior to a ContentControl in WinUI and the DataContext in WinUI always propagates to the Template and the DataContext on the ContentView always propagates to the ContentPresenter even if the template changes the DataContext

image

We changed this behavior in a slightly awkward way in net8.0 #12536 where we are only setting the ControlTemplate BC when the Content isn't set and it only works for the initial ControlTemplate since we didn't copy this logic into ControlTemplateChanged

It's also a bit weird now because the ordering will change how the BC gets assigned. For example,

contentView.ControlTemplate = new ControlTemplate(typeof(SimpleTemplate));
var bc = "Test";
contentView.BindingContext = bc;
contentView.Content = child;

This will cause the BC to propagate to the ControlTemplate but now it won't propagate when you change the ControlTemplate. Which is basically what the issue this is fixing is seeing happen.

This PR fully commits in the direction of just setting the ControlTemplates binding context always from the ContentView's BindingContext

Issues Fixed

Fixes #25934
Fixes #22607
Fixes #14919

Directory.Build.props Outdated Show resolved Hide resolved
Copy link
Contributor

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provided we restore the warnings as errors if possible

@PureWeen PureWeen force-pushed the contentview_propagation branch from 9fd12e7 to fc7ab11 Compare November 22, 2024 20:45
@PureWeen PureWeen marked this pull request as ready for review November 22, 2024 20:46
@PureWeen PureWeen requested a review from a team as a code owner November 22, 2024 20:46
@PureWeen PureWeen added this to the .NET 9 SR2 milestone Nov 22, 2024
@PureWeen PureWeen merged commit 1ce909f into main Nov 27, 2024
104 checks passed
@PureWeen PureWeen deleted the contentview_propagation branch November 27, 2024 03:46
@samhouts samhouts added fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release!
Projects
Status: Done
5 participants