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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
3 participants