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

Changed to call Popup's MapOnClosed method #2202

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

cat0363
Copy link
Contributor

@cat0363 cat0363 commented Sep 12, 2024

In this PR, by applying PR #2166, we resolve the issue where Popup is not closed when the Close or CloseAsync method is called.

Description of Change

I added the following code to PR #2166.

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

	public static CommandMapper<IPopup, PopupHandler> ControlPopUpCommandMapper = new(PopupHandler.PopUpCommandMapper)
	{
#if IOS || MACCATALYST
		[nameof(IPopup.OnOpened)] = MapOnOpened,
		[nameof(IPopup.OnClosed)] = MapOnClosed          // <= here
#endif
	};

The above code will no longer call the following method.

[src\CommunityToolkit.Maui.Core\Handlers\Popup\PopupHandler.macios.cs]

	public static async void MapOnClosed(PopupHandler handler, IPopup view, object? result)
	{
		var presentationController = handler.PlatformView.PresentationController;
		if (presentationController?.PresentedViewController is UIViewController presentationViewController)
		{
			await presentationViewController.DismissViewControllerAsync(true);
		}

		view.HandlerCompleteTCS.TrySetResult();

		handler.DisconnectHandler(handler.PlatformView);
	}

Since the above method is no longer called, the Popup will continue to wait.

Modify the code as below so that the MapOnClosed method is called.

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

	public static void MapOnClosed(PopupHandler handler, IPopup view, object? result)
	{
		PopupHandler.MapOnClosed(handler, view, result);

		var parent = view.Parent as Element;
		if (parent is not null)
		{
			if (handler.VirtualView is Popup popup)
			{
				if (popup.Content is not null)
				{
					if (popup.Content.Parent is ContentPage contentPage)
					{
						parent.RemoveLogicalChild(contentPage);
					}
				}
				parent.RemoveLogicalChild(popup);
			}
		}
	}

Also, remove the MapOnClosed definition from PopupHandler's PopUpCommandMapper only for iOS and MacCatalyst.
PR #2166 lacked this consideration.

[src\CommunityToolkit.Maui.Core\Handlers\Popup\PopupHandler.shared.cs]

	public static CommandMapper<IPopup, PopupHandler> PopUpCommandMapper = new(ElementCommandMapper)
	{
#if !(IOS || MACCATALYST)
		[nameof(IPopup.OnOpened)] = MapOnOpened,
		[nameof(IPopup.OnClosed)] = MapOnClosed,
#endif
		[nameof(IPopup.OnDismissedByTappingOutsideOfPopup)] = MapOnDismissedByTappingOutsideOfPopup
	};

Linked Issues

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

The verification results are shown below.

[Case to call Close method]

iPhone.15.iOS.17.0.2024-09-12.11-12-47.mp4

[Case to call CloseAsync method]

iPhone.15.iOS.17.0.2024-09-12.11-13-09.mp4

You can see that the Popup is closed in both cases.

Below are the verification results for Issue #2149.

[Case of tapping outside of Popup]

iPhone.15.iOS.17.0.2024-09-12.11-26-17.mp4

[Case of intentionally calling the close method]

iPhone.15.iOS.17.0.2024-09-12.11-30-07.mp4

In both cases, you can see that Issue #2149 is resolved.

maonaoda added a commit to maonaoda/CommunityToolkit-Maui that referenced this pull request Sep 12, 2024
Copy link
Contributor

@bijington bijington left a comment

Choose a reason for hiding this comment

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

@cat0363 thank you again! Sorry I don't know how I missed this bug from the last PR review.

@bijington bijington merged commit 4d1d049 into CommunityToolkit:main Sep 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Popup is not closed by calling Close or CloseAsync method.
2 participants