Add routing of BindingContext of Popup

[cat0363]

This PR solves the problem that the BindingContext of Popup is not routed to ContentPage.

Description of Change

I added the following code to pass the Popup's BindingContext to the ContentPage.

[src\CommunityToolkit.Maui\HandlerImplementation\Popup\Popup.macios.cs]

public static void MapOnOpened(PopupHandler handler, IPopup view, object? result)
{
    handler.PlatformView?.CreateControl(CreatePageHandler, view);
    view.OnOpened();

    static PageHandler CreatePageHandler(IPopup virtualView)
    {
        var mauiContext = virtualView.Handler?.MauiContext ?? throw new NullReferenceException(nameof(IMauiContext));
        var view = (View?)virtualView.Content ?? throw new InvalidOperationException($"{nameof(IPopup.Content)} can't be null here.");
        view.BindingContext = ((Popup)virtualView).BindingContext;    // <= here
        var contentPage = new ContentPage
        {
            Content = view
        };

        contentPage.SetBinding(BindingContextProperty, new Binding { Source = virtualView, Path = BindingContextProperty.PropertyName });

        return (PageHandler)contentPage.ToHandler(mauiContext);
    }
}

This will take over the BindingContext and not initialize it with null.

Linked Issues

• Fixes [BUG] Picker not work correctly on popup #1166

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pull/XYZ

Additional information

I simplified the program of Issue #1166 and verified it.
Below are the verification results.

iPhone.14.iOS.16.4.2023-07-28.17-30-09.mp4

[VladislavAntonyuk]

Does the issue happen only on Apple devices?

[cat0363]

Does the issue happen only on Apple devices?

Hi, @VladislavAntonyuk
At least it was working fine on Android.
I can't confirm for Windows. Could someone please confirm?
As for Windows, if it's not urgent, I'll check next week.

[pictos]

the view is a child of the ContentPage and when we set the Binding for that ContentPage it should propagate to the view, if I didn't miss anything. What's the issue you're seeing?

[cat0363]

view.BindingContext = ((Popup)virtualView).BindingContext;

Thank you for your review.
You're right, that's how it should be. I thought so too until I debugged it.
When the following is executed, the ContentPage's BindingContext is propagated and initialized.

ContentView = view

The cause is that the content of the Popup is replaced with the content of the newly generated ContentPage.
Binding is set in ContentPage with SetBinding method, but its Source is virtualView, not view.

contentPage.SetBinding(BindingContextProperty, new Binding { Source = virtualView, Path = BindingContextProperty.PropertyName });

For this, I added the code below.

view.BindingContext = ((Popup)virtualView).BindingContext;

But that is wrong. Add the following instead of the above

view.SetBinding(BindingContextProperty, new Binding { Source = (Popup)virtualView, Path = BindingContextProperty.PropertyName });

And I should have changed SetBinding of ContentPage as below.

contentPage.SetBinding(BindingContextProperty, new Binding { Source = view, Path = BindingContextProperty.PropertyName });

From the above, I think it will be as follows.

static PageHandler CreatePageHandler(IPopup virtualView)
{
    var mauiContext = virtualView.Handler?.MauiContext ?? throw new NullReferenceException(nameof(IMauiContext));
    var view = (View?)virtualView.Content ?? throw new InvalidOperationException($"{nameof(IPopup.Content)} can't be null here.");
    view.SetBinding(BindingContextProperty, new Binding { Source = (Popup)virtualView, Path = BindingContextProperty.PropertyName });

    var contentPage = new ContentPage
    {
        Content = view
    };

    contentPage.SetBinding(BindingContextProperty, new Binding { Source = view, Path = BindingContextProperty.PropertyName });

    return (PageHandler)contentPage.ToHandler(mauiContext);
}

By correcting it as above, it propagates correctly.
I will upload it again next week.

[cat0363]

Hi, @VladislavAntonyuk
Fixed the BindingContext propagation as per the previous post.
Below is the execution result on iOS.

[iOS]

iPhone.14.iOS.16.4.2023-07-31.09-03-21.mp4

This issue does not occur on Windows and Android. This issue occurs only on iOS.
Below are the execution results on Android and Windows.

[Android]

Android.Emulator.-.pixel_2_-_api_30_5554.2023-07-31.09-07-45.mp4

[Windows]

2023-07-31.09-13-58.mp4

[cat0363]

Below is the reproduction code I created to confirm that there is a problem with the method of BindingContext propagation.

https://github.com/cat0363/MauiComm-ReproPopupPicker.git

The above partially mimics a portion of the Popup.macios.cs file.

Instead of displaying the Popup on the screen, I set the Content of the ContentView imitating the Popup to the
Content of cvContent in the layout below.

[MainPage.xaml]

<Grid RowDefinitions="44,44,*">
    <Button x:Name="btnShow" Grid.Row="0" Text="Show" BackgroundColor="Blue" TextColor="White" Clicked="btnShow_Clicked" />
    <ContentView x:Name="cvContent" Grid.Row="1" />
</Grid>

[Popup.xaml]

<Grid>
    <Picker
        Title="Select an Outlet"
        ItemDisplayBinding="{Binding Name}"
        ItemsSource="{Binding Outlets}"
        SelectedItem="{Binding SelectedItemOutlet}" />
</Grid>

The processing when the Show button is pressed is as follows.

private void btnShow_Clicked(object sender, EventArgs e)
{
    var popup = new Popup();
    var view = popup.Content;
    // view.SetBinding(BindingContextProperty, new Binding { Source = popup, Path = BindingContextProperty.PropertyName });
    cvContent.Content = view;
    // cvContent.SetBinding(BindingContextProperty, new Binding { Source = view, Path = BindingContextProperty.PropertyName });
    cvContent.SetBinding(BindingContextProperty, new Binding { Source = popup, Path = BindingContextProperty.PropertyName });
}

If you run it as above, the same phenomenon will be reproduced.

Therefore, modify the code as follows. This is equivalent to the solution I posted today.

private void btnShow_Clicked(object sender, EventArgs e)
{
    var popup = new Popup();
    var view = popup.Content;
    view.SetBinding(BindingContextProperty, new Binding { Source = popup, Path = BindingContextProperty.PropertyName });
    cvContent.Content = view;
    cvContent.SetBinding(BindingContextProperty, new Binding { Source = view, Path = BindingContextProperty.PropertyName });
    // cvContent.SetBinding(BindingContextProperty, new Binding { Source = popup, Path = BindingContextProperty.PropertyName });
}

You can see that the BindingContext is propagated correctly.

[VladislavAntonyuk]

It looks weird why we need it only for macios. Could it be a .NET MAUI issue as they don't set the binding context?

[cat0363]

@VladislavAntonyuk, Thank you for your reply.
This issue occurs even if I run the code that causes the same phenomenon that I created on Android and Windows.
So it's not just iOS.

https://github.com/cat0363/MauiComm-ReproPopupPicker.git

Below are the results of running the above code on Android and Windows.
iOS, Android, and Windows all have the same results.

[Android]

[Windows]

If it is made similarly, this problem will occur on Android and Windows.
As for why this issue is only for macios, it is because Android and Windows are not made like this.

On Android, the LayoutViewGroup obtained by calling the ToPlatform method of the Popup's Content is set to the Dialog's ContentView.

[src\CommunityToolkit.Maui.Core\Views\Popup\MauiPopup.android.cs] : TryCreateContainer

container = popup.Content.ToPlatform(mauiContext);
SetContentView(container);

On Windows, the LayoutPanel obtained by calling the View's ToPlatform method is added to the Panel's child elements.
That Panel is set as the Flyout's Content.

[src\CommunityToolkit.Maui\Views\WrapperControl.windows.cs] : Constructor

FrameworkElement = view.ToPlatform(mauiContext);
Children.Add(FrameworkElement);

[src\CommunityToolkit.Maui.Core\Views\Popup\MauiPopup.windows.cs] : CreateControl

Control = createControl(handler);
Content = Control;

At least, Android and Windows are built differently than iOS.
If you don't mind, can you give me a specific reason why it's a .NET MAUI issue?

Does it mean that the behavior that the SelectedItem of the Picker is initialized when the parent is replaced is not what you intended? I'm sorry if my understanding is wrong.

[cat0363]

I tried to find out why SelectedItem is initialized when the parent is replaced.

When setting the Popup's Content to the ContentPage's Content, the Popup's Content is removed from the Popup.
At this time, the Popup's Content BindingContext is initialized with null. When the Content's BindingContext
is initialized with null, the Picker's OnItemsSourceChanged method is called and the Picker's SelectedIndex
is initialized with -1.

Then add that Content to the ContentPage.
At this time, the BindingContext of ContentPage, which is the new parent of Content, is inherited by Content
removed from Popup. But the ContentPage's BindingContext is null. Also, the BindingContext of the Content
removed from the Popup is null. So the value of BindingContext doesn't change and nothing is done. The Picker's
SelectedIndex remains -1. The timings above are as follows.

var contentPage = new ContentPage
{
    Content = view
};

BindingContext is propagated when the following is executed.

contentPage.SetBinding(BindingContextProperty, new Binding { Source = virtualView, Path = BindingContextProperty.PropertyName });

When the above is executed, BindingContext is propagated from Popup to Content of ContentPage.
However, SelectedIndex is already initialized to -1 when we remove the Content from the Popup.
In other words, the value bound to SelectedItem of Picker is null. Picker's ItemsSource is bound as it is,
but SelectedItem is bound with a value rewritten to null, so SelectedIndex remains -1.

The above is the result of analyzing the source code of .NET MAUI.
The source code references are as follows.

[src\Controls\src\Core\Element\Element.cs] OnChildAdded
[src\Controls\src\Core\Element\Element.cs] OnChildRemoved
[src\Controls\src\Core\Element\Element.cs] SetParent
[src\Controls\src\Core\BindableObject.cs] SetInheritedBindingContext
[src\Core\BindableObjectExtensions.cs] PropagateBindingContext
[src\Controls\src\Core\Picker\Picker.cs] OnItemSourceChanged
[src\Controls\src\Core\Picker\Picker.cs] OnSelectedItemChanged

So what should be,

Only the code below is sufficient to solve this issue.

static PageHandler CreatePageHandler(IPopup virtualView)
{
    var mauiContext = virtualView.Handler?.MauiContext ?? throw new NullReferenceException(nameof(IMauiContext));
    var view = (View?)virtualView.Content ?? throw new InvalidOperationException($"{nameof(IPopup.Content)} can't be null here.");
    view.SetBinding(BindingContextProperty, new Binding { Source = virtualView, Path = BindingContextProperty.PropertyName });

    var contentPage = new ContentPage
    {
        Content = view
    };

    return (PageHandler)contentPage.ToHandler(mauiContext);
}

I didn't need the code below.

contentPage.SetBinding(BindingContextProperty, new Binding { Source = virtualView, Path = BindingContextProperty.PropertyName });

By setting the Binding of the Content of the Popup in advance, the BindingContext of the Content will not be
initialized with null when the Content is deleted from the Popup. So the Picker's ItemsSource is not temporarily
initialized to null nor is the SelectedIndex initialized to -1.

iPhone.14.iOS.16.4.2023-08-01.17-10-07.mp4

Check out the implementation below to see why.

[src\Controls\src\Core\BindableObject.cs] SetInheritedBindingContext

[pictos]

@cat0363 thanks for all explanation and the fix... I think this is a fine change to do. But looks like a bug on Maui side of the BindingContext, so let's say that you have a ContentPage with a Label on it, something like this:

<ContentPage>
    <Label Text="{Binding Title}" />
</ContentPage>

Then on your code-behind you have this:

class MainPage : ContentPage
{
    public MainPage()
    {
        InitializeComponent();
        BindingContext = new MainViewModel();
    }
}

So, we have the Label.TextProperty bind to the Title property on the ViewModel, but at the moment the Label is initialized (on the InitializeComponent method) the BindingContext for the page and Label is null, the Binding changed to MainViewModel, for both Label and Page, after the initialization and the Label will show whatever the Title is.

With that, the behavior of the popup and picker should be the same, don't you agree? Because of that, I believe there's a bug or the Picker control works in a different way.

[cat0363]

@pictos, Thank you for the easy-to-understand explanation.

I agree with your explanation. Certainly it's not that the BindingContext isn't
propagated. As proof, when you tap the Picker, you can see that the ItemsSource
is correctly bound.

When the parent is replaced, the ItemsSource is initialized to null, at which time
the Picker's ItemsSourceChanged method is called and the SelectedItem is initialized
to null, but the Picker's ItemsSource is set again.

I've found that the initialization that occurs when the parent is swapped causes
this problem, but I'm not sure if that initialization was intended. Presumably,
if this BindingContext initialization hadn't happened, this problem wouldn't have occurred.

In the Discussions below, I am asking a similar question to the .NET MAUI side.
So far I haven't received an answer from the .NET MAUI side.

dotnet/maui#16516

If it's a bug, it seems that BindingContext initialization and Picker behavior are deeply related. . .
By the way, if it's a .NET MAUI bug, is there any way for users to avoid this problem?

One workaround I can think of is something like this:
The following is an implementation example on the side that uses Popup.

void Button_Clicked(object sender, EventArgs e)
{
    var popup = new PopupPage();
    popup.Opened += new EventHandler<CommunityToolkit.Maui.Core.PopupOpenedEventArgs>(popup_Opend);
    Shell.Current.ShowPopup(popup);		
}

void popup_Opend(object? sender, CommunityToolkit.Maui.Core.PopupOpenedEventArgs e)
{
    if (sender is PopupPage popup)
    {
        popup.Opened -= new EventHandler<CommunityToolkit.Maui.Core.PopupOpenedEventArgs>(popup_Opend);
        popup.BindingContext = new ViewModelTest();
    }
}

Instead of setting the BindingContext in the constructor of the class that inherits
Popup, you can avoid this by setting the BindingContext in the Popup's Opend event.

I will leave the closing of this PR to you. If there is a workaround that can be considered at this time,
it may be better to present it to the person who posted the issue.