Remove Bindings on Popup Close

[cat0363]

In this PR, we will correct it so that SetColor of a Popup that has already been destroyed is not called.

Description of Change

If the SetColor method is called after closing the Popup, an exception will occur at the following location.

[src\CommunityToolkit.Maui.Core\Views\Popup\PopupExtensions.android.cs]

public static void SetColor(this Dialog dialog, in IPopup popup)
{
    if (popup.Color is null)
    {
        return;
    }

    var window = GetWindow(dialog);    // <- here
    window.SetBackgroundDrawable(new ColorDrawable(popup.Color.ToPlatform(AColorRes.BackgroundLight, dialog.Context)));
}

Add a guard condition to the call to the SetColor method.

[src\CommunityToolkit.Maui.Core\Handlers\Popup\PopUpHandler.android.cs]

public static void MapColor(PopupHandler handler, IPopup view)
{
    if (!handler.PlatformView.IsDisposed())
    {
        handler.PlatformView.SetColor(view);
    }
}

Similarly, add guard conditions to calls to the SetAnchor and SetSize methods.

public static void MapAnchor(PopupHandler handler, IPopup view)
{
    if (!handler.PlatformView.IsDisposed())
    {
        handler.PlatformView.SetAnchor(view);
    }
}

public static void MapSize(PopupHandler handler, IPopup view)
{
    ArgumentNullException.ThrowIfNull(handler.Container);

    if (!handler.PlatformView.IsDisposed())
    {
        handler.PlatformView.SetSize(view, handler.Container);
        handler.PlatformView.SetAnchor(view);
    }
}

void OnLayoutChange(object? sender, EventArgs e)
{
    if (VirtualView?.Handler?.PlatformView is Dialog dialog && Container is not null)
    {
        if (!dialog.IsDisposed())
        {
            PopupExtensions.SetSize(dialog, VirtualView, Container);
        }
    }
}

Linked Issues

• Fixes [BUG] Application crashes on theme change after using Popup with AppThemeBinding under Android #1608

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/pulls

Additional information

Below is the verification video.

Android.Emulator.-.pixel_2_-_api_30_5554.2023-12-20.15-32-08.mp4

Even if I change the AppTheme, I can see that it is working as intended without any crashes.

[pictos]

Hey @cat0363, thanks for PR... For me, this looks more like a workaround than an actual fix. We dispose of the PlatformView when the DisconnectHandler is called, so the Map methods shouldn't trigger anymore, for that specific instance. So I believe the error is elsewhere. I suggest we check if there's another path to PlatformView.Dispose first, then start working from there

[cat0363]

@pictos , Thank you for your comment.
I will reconsider. I would like to try to find the root cause of the problem.

[cat0363]

The root cause of this problem is that the Binding is not released after the Popup is closed.
Because the Binding was not released, the MapColor method was called via the Color property when the theme was changed after the Popup was closed.

Revert the modifications you made previously and make the following changes.

[src\CommunityToolkit.Maui\Views\Popup\Popup.shared.cs]

protected virtual async Task OnClosed(object? result, bool wasDismissedByTappingOutsideOfPopup, CancellationToken token = default)
{
	((IPopup)this).OnClosed(result);
	((IResourceDictionary)resources).ValuesChanged -= OnResourcesChanged;

	RemoveBinding(Popup.ContentProperty);
	RemoveBinding(Popup.ColorProperty);
	RemoveBinding(Popup.SizeProperty);
	RemoveBinding(Popup.CanBeDismissedByTappingOutsideOfPopupProperty);
	RemoveBinding(Popup.VerticalOptionsProperty);
	RemoveBinding(Popup.HorizontalOptionsProperty);
	RemoveBinding(Popup.StyleProperty);

	await popupDismissedTaskCompletionSource.Task.WaitAsync(token);
	Parent = null;

	dismissWeakEventManager.HandleEvent(this, new PopupClosedEventArgs(result, wasDismissedByTappingOutsideOfPopup), nameof(Closed));
}

Below is the verification video.

Android.Emulator.-.pixel_2_-_api_30_5554.2024-01-19.11-42-36.mp4

You can see that it doesn't crash even after changing the theme.