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

[BUG] When a Popup is displayed, a warning is displayed and the content of the page from which the Popup is displayed disappears. #1416

Closed
2 tasks done
cat0363 opened this issue Sep 19, 2023 · 8 comments · Fixed by #1361
Labels
bug Something isn't working unverified

Comments

@cat0363
Copy link
Contributor

cat0363 commented Sep 19, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Did you read the "Reporting a bug" section on Contributing file?

Current Behavior

Build the latest source code of main and display Popup on iOS.
The content of the page from which the Popup is displayed disappears when the Popup is displayed.

iPhone.14.iOS.16.4.2023-09-19.12-33-56.mp4

The following warning message is output when displaying the popup.

[Presentation] Presenting view controller <CommunityToolkit_Maui_Core_Views_MauiPopup: 0x7f97cff7b010> from detached view controller <Microsoft_Maui_Platform_PageViewController: 0x7f97cffa0590> is discouraged.

The above warning message occurs in the following locations.

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

void AddToCurrentPageViewController(UIViewController viewController)
{
    viewController.PresentViewController(this, true, null);
}

Expected Behavior

I was hoping that the content of the page from which the Popup was displayed would not disappear when the Popup was displayed, and that the Popup would be displayed as intended.

Steps To Reproduce

The steps to reproduce are as follows.

  1. Launch apps uploaded to GitHub on iOS
  2. Press the Show button

In step 2, the content of the page from which the Popup is displayed will disappear at the same time the Popup is displayed.
Please use the latest source code of main to reproduce.

Link to public reproduction project repository

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

Environment

- .NET MAUI CommunityToolkit: Latest version of main at the time this issue was posted
- OS: iOS 16.4
- .NET MAUI: 7.0.92

Anything else?

No response

@cat0363 cat0363 added bug Something isn't working unverified labels Sep 19, 2023
@cat0363
Copy link
Contributor Author

cat0363 commented Sep 19, 2023

I was initially expecting what you posted.
This is because the affected scope is only MacCatalyst.

I was wondering if there was a way to make both MacCatalyst and iOS work as intended, but by doing the following, it works as long as Popups are not displayed nested.

var pagehandler = VirtualView.Parent.Handler as PageHandler;
var rootViewController = pagehandler?.PlatformView.Window.RootViewController ?? WindowStateManager.Default.GetCurrentUIViewController() ?? throw new InvalidOperationException($"{nameof(PageHandler.ViewController)} cannot be null.");

However, if you display Popups nested, the second Popup and subsequent Popups will not be displayed.
I wish I could solve that problem, but I haven't gotten a solution yet.

@babenkovitaliy
Copy link
Contributor

@cat0363 you commented on your PR with this:

If PR #1409 is applied, this PR will no longer work as intended ...
It's a good fix for MacCatalyst, but it seems to have a big impact on iOS.
Popups can no longer be displayed nested. The view displaying the Popup disappears at the same time the Popup is displayed.

If my PR affected just iOS, then what is preventing you from using my solution posted in that PR as a comment?

iOS apps are single window apps, so this line of code under the #else preprocessor directive would have the application function on iOS just how it was prior to my PR:
var rootViewController = WindowStateManager.Default.GetCurrentUIViewController() ?? throw new InvalidOperationException($"{nameof(PageHandler.ViewController)} cannot be null.");

Just curious on why that solution won't work?

@pictos
Copy link
Member

pictos commented Sep 19, 2023

Does ipad support multi-window apps? If so, we need to care about it on iOS world, since it's compiled to iPad.

@cat0363
Copy link
Contributor Author

cat0363 commented Sep 20, 2023

@babenkovitaliy , Thanks for your comment.

If my PR affected just iOS, then what is preventing you from using #1361 (comment) posted in that PR as a comment?

This is because I don't know of any clear reason not to do conditional branching.
Is it because you tested the operation on both MacCatalyst and iOS and there are no problems?

At least, on the iOS Shell app that I tested, a warning was output and it did not work as intended.
I'd like to know why they decided it was OK at the time of approval.
After knowing that, if there is no deep intention, I think that it should be the conditional branch description you presented.
(Because movement as before is at least guaranteed.)

However, what I am concerned about is whether the Popup nesting that I was trying to achieve on iOS will also work on MacCatalyst if I do that. I think that PR #1361 may not work on MacCatalyst.

In any case, we need to know the concept behind not using conditional branching. Your PR that you first submitted had a conditional branch.

@pictos , I don't know if iPad multitasking is equivalent to MacCatalyst. If you launch multiple apps, aren't the instances different? Is it possible to display multiple windows in the same instance?

@cat0363
Copy link
Contributor Author

cat0363 commented Sep 20, 2023

The code below also works if you nest popups on iOS, but I don't know if popups can be nested on MacCatalyst.

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)} or PhoneFlyoutPageRenderer.");
    }

    VirtualView = element;
    ModalPresentationStyle = UIModalPresentationStyle.Popover;

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

    var pagehandler = VirtualView.Parent.Handler as PageHandler;
    var pvc = pagehandler?.ViewController?.PresentedViewController;
    while(pvc?.PresentedViewController is not null)
    {
        pvc = pvc?.PresentedViewController;
    }
    var rvc = pagehandler?.PlatformView.Window.RootViewController;
    var rootViewController = pvc ?? rvc ?? WindowStateManager.Default.GetCurrentUIViewController() ?? throw new InvalidOperationException($"{nameof(PageHandler.ViewController)} cannot be null.");
    ViewController ??= rootViewController;
    SetDimmingBackgroundEffect();
}

It may be better to clearly separate them using conditional branching ...

@cat0363
Copy link
Contributor Author

cat0363 commented Sep 20, 2023

As a side note, we have fixed the Popup position problem for iOS, but the Popup position problem still exists for MacCatalyst.
Popup is not displayed in the intended position on MacCatalyst. It seems better to use separate logic instead of using the same logic for both MacCatalyst and iOS.

@babenkovitaliy
Copy link
Contributor

@cat0363 Just an FYI. I did see all of your responses. However, I've only had time to read the responses, but not go anywhere past that. Too many work and life related things happening that is preventing me from looking into this a little more.

@cat0363
Copy link
Contributor Author

cat0363 commented Sep 20, 2023

@babenkovitaliy , Thank you for your reply despite your busy schedule.
I don't have a lot of time either, so I'll put this issue on hold until I can confirm the concept.
Probably, but as you said, I think it's best to use conditional branching. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working unverified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants