@cat0363 Just FYI - the latest PR we merged today introduced a merge conflict in MauiPopup.macios.cs

@cat0363 Just FYI - a PR we merged today introduced a merge conflict here in this PR

@brminnick , I resolved the conflict and merged.

@cat0363 Thanks for this PR and all the others... But I don't think this is the correct solution, forcing the popup to open when we have a PageHandler is this way by design, this control doesn't support being opened for any other control type (Label, Entry, Button, ContentView, Popup, etc), so, I would say, your fix should be other... Or there's nothing here to fix. Maybe the fix should be to proxy to the Application.Current.MainPage when we don't find the current page, and I believe there's a merged PR that forces that

@pictos , Thank you for your valuable opinion.
I was waiting for that opinion. I have a question for you.
I don't understand the design concept, so I'm sorry if it's an irrelevant question.

1. Could you tell me the specific reasons for the following restrictions? And what problems does the lack of this restrictions create? I want to know that.

    public void SetElement(IPopup element)
    {
        if (element.Parent?.Handler is not PageHandler)
        {
            throw new InvalidOperationException($"The {nameof(element.Parent)} must be of type {typeof(PageHandler)}.");
        }
   
        VirtualView = element;
        ModalPresentationStyle = UIModalPresentationStyle.Popover;
   
        _ = View ?? throw new InvalidOperationException($"{nameof(View)} cannot be null.");
        _ = VirtualView ?? throw new InvalidOperationException($"{nameof(VirtualView)} cannot be null.");
   
        var rootViewController = WindowStateManager.Default.GetCurrentUIViewController() ?? throw new InvalidOperationException($"{nameof(PageHandler.ViewController)} cannot be null.");
        ViewController ??= rootViewController;
        SetDimmingBackgroundEffect();
    }
   

2. Why don't Android and Windows have similar restrictions? I think that restrictions should be placed on Android and Windows so that there is no difference between what works and what doesn't work on each platform.

As an aside, the results of verification in PR before WeakReference is applied are described below.

[Case 1]

Open popup from popup. The first popup is displayed on the top left, the second popup is displayed in the center.

[Case 2]

Open a Popup with the click event of MyButton, which inherits the Button class.

public class MyButton : Button
{
    public MyButton()
    {
        this.Clicked += (sender, args) =>
        {
            var popup3 = new PopupPage3();
            Shell.Current.ShowPopup(popup3);
        };
    }
}

The above case works equally well on Android and Windows.

As a result of discussion, if it is determined that there is no need to fix it, I will withdraw this PR.

@cat0363 there are no bad questions at all, just ask as you need (:. I'll do my best do answer your questions, since it has been a long long time since I wrote those lines...

Could you tell me the specific reasons for the following restrictions? And what problems does the lack of this restrictions create? I want to know that.

iOS doesn't have a popup concept, like Android (Dialog) and Windows (Flyout) have, so we mimic the Popup on iOS creating a ViewController and presenting it as a UIModalPresentationStyle.Popover in order to that work the parent of the popup should be anything that behaves like a Page, so because of that there's a check to make sure the element.Parent handler is a PageHandler.

Why don't Android and Windows have similar restrictions? I think that restrictions should be placed on Android and Windows so that there is no difference between what works and what doesn't work on each platform.

Since Android and Windows have the concept of Popup, everything is handled by each platform. So, the limitation on iOS is to make the popup implementation there as close as possible to other platforms.

As I mentioned, has been a long time since I wrote these lines, maybe something changes that we can modify this code. Or even if Apple releases a popup control, we can use that instead.

@pictos , Thank you for answering my question.

Since Android and Windows have the concept of Popup, everything is handled by each platform. So, the limitation on iOS is to make the popup implementation there as close as possible to other platforms.

I understand why Windows and Android don't impose restrictions. The concept is to bring the incapable side closer to the capable side as much as possible, rather than matching the incapable side to the capable side, isn't it?

iOS doesn't have a popup concept, like Android (Dialog) and Windows (Flyout) have, so we mimic the Popup on iOS creating a ViewController and presenting it as a UIModalPresentationStyle.Popover in order to that work the parent of the popup should be anything that behaves like a Page, so because of that there's a check to make sure the element.Parent handler is a PageHandler.

Is it correct to recognize that there are things that cannot be done if it is not PageHandler?
I would like to know what it is and the role of the Popup's parent.

In the issue that led to this PR, the type of element.Parent?.Handler was PhoneFlyoutPageRenderer.
It was not PageHandler. Does it mean that there is something that PhoneFlyoutPageRenderer
cannot achieve?

If the answer is yes, then you can add a condition, but the PhoneFlyoutPageRenderer cannot be
referenced as it is defined at:

Microsoft.Maui.Controls.Handlers.Compatibility.PhoneFlyoutPageRenderer

So the implementation would look something like this:

if (element.Parent?.Handler is not PageHandler &&
    element.Parent?.Handler?.GetType().Name != "PhoneFlyoutPageRenderer")
{
    throw new InvalidOperationException($"The {nameof(element.Parent)} must be of type {typeof(PageHandler)} or PhoneFlyoutPageRenderer.");
}

Wouldn't it be possible to add allowable parents as above?

@cat0363 FYI - after merging #1361, it has caused a merge conflict on this PR

@brminnick , I resolved the conflict. But, I got a build error after merging, I will fix it on Monday.

This PR does not give a clear answer to the question. I need to know a clear reason why element.Parent must be a PageHandler.
We need to write and verify an example of what happens when element.Parent is not a PageHandler. However, I haven't been able to describe the NG cases.

I think MacCatalyst at least needs to be considered separately from iOS since it is limited to PageHandler in PR #1409.
The build error seems to occur because this PR does not take into account the fixes in PR #1409.

For MacCatalyst, element.Parent must be a PageHandler, so we reinstated the restriction.

[src\CommunityToolkit.Maui.Core\Views\Popup\MauiPopup.macios.cs]

public void SetElement(IPopup element)
{
    if (element.Parent?.Handler is not PageHandler)
    {
        throw new InvalidOperationException($"The {nameof(element.Parent)} must be of type {typeof(PageHandler)}.");
    }

    VirtualView = element;
    ModalPresentationStyle = UIModalPresentationStyle.Popover;

    _ = View ?? throw new InvalidOperationException($"{nameof(View)} cannot be null.");
    _ = VirtualView ?? throw new InvalidOperationException($"{nameof(VirtualView)} cannot be null.");

#if MACCATALYST
    var pagehandler = VirtualView.Parent.Handler as PageHandler;
    var rootViewController = pagehandler?.ViewController ?? WindowStateManager.Default.GetCurrentUIViewController() ?? throw new InvalidOperationException($"{nameof(PageHandler.ViewController)} cannot be null.");
#else
    var rootViewController = WindowStateManager.Default.GetCurrentUIViewController() ?? throw new InvalidOperationException($"{nameof(PageHandler.ViewController)} cannot be null.");
#endif

    ViewController ??= rootViewController;
    SetDimmingBackgroundEffect();
}

In the case of MacCatalyst, if you set overlayView.Frame = view.Frame, the overlay does not completely cover the original view, so I fixed it as follows. For MacCatalyst, this is because there is an offset in the Y coordinate.

[src\CommunityToolkit.Maui.Core\Views\Popup\MauiPopup.macios.cs]

void SetDimmingBackgroundEffect()
{
    if (ViewController?.View is UIView view)
    {
        var overlayView = GetOverlayView(view);
        overlayView.Bounds = view.Bounds;
        overlayView.Layer.RemoveAllAnimations();
        overlayView.Frame = new CGRect(0, 0, view.Frame.Width, view.Frame.Height);
        overlayView.BackgroundColor = UIColor.Black.ColorWithAlpha(0.4f);
        view.AddSubview(overlayView);
    }
}

public override void ViewWillTransitionToSize(CGSize toSize, IUIViewControllerTransitionCoordinator coordinator)
{
    coordinator.AnimateAlongsideTransition((IUIViewControllerTransitionCoordinatorContext obj) =>
    {
        // Before screen rotate
        if (ViewController?.View is UIView view)
        {
            var overlayView = GetOverlayView(view);
            overlayView.Frame = new CGRect(0, 0, view.Frame.Width, view.Frame.Height);
        }
    }, (IUIViewControllerTransitionCoordinatorContext obj) =>
    {
        // After screen rotate
        if (VirtualView is not null)
        {
            PopupExtensions.SetSize(this, VirtualView);
            PopupExtensions.SetLayout(this, VirtualView);
        }
    });
    base.ViewWillTransitionToSize(toSize, coordinator);
}

Changed null judgment from ==null to is null.

[src\CommunityToolkit.Maui.Core\Views\Popup\MauiPopup.macios.cs]

UIView GetOverlayView(UIView view)
{
    var overlayViewTag = new IntPtr(38483);
    var overlayView = view.Subviews.AsEnumerable()
        .Where(
            x => x.Tag == overlayViewTag
        )
        .FirstOrDefault();
    if (overlayView is null)
    {
        overlayView = new UIView();
        overlayView.Tag = overlayViewTag;
    }

    return overlayView;
}