Fix display position and size of Popup #1320

cat0363 · 2023-08-04T08:06:57Z

This PR fixes an issue where popups are displayed in the wrong size and position on iOS and Android.
It also fixes an issue where the size of the displayed Popup is not updated when rotating an Android device.

Description of Change

I changed the PopupExtensions.macios.cs file to:

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

public static void SetLayout(this MauiPopup mauiPopup, in IPopup popup)
{
    if (mauiPopup.View is null)
    {
        return;
    }

    CGRect frame;

    if (mauiPopup.ViewController?.View?.Window is UIWindow window)
    {
        frame = window.Frame;
    }
    else
    {
        frame = UIScreen.MainScreen.Bounds;
    }

    if (mauiPopup.PopoverPresentationController is null)
    {
        throw new InvalidOperationException("PopoverPresentationController cannot be null");
    }

    if (popup.Anchor is null)
    {
        nfloat originY;
        if (mauiPopup.PreferredContentSize.Height < frame.Height)
        {
            originY = popup.VerticalOptions switch
            {
                Microsoft.Maui.Primitives.LayoutAlignment.Start => mauiPopup.PreferredContentSize.Height / 2,
                Microsoft.Maui.Primitives.LayoutAlignment.End => frame.Height - (mauiPopup.PreferredContentSize.Height / 2),
                Microsoft.Maui.Primitives.LayoutAlignment.Center => frame.GetMidY(),
                _ => frame.GetMidY()
            };
        }
        else
        {
            originY = -frame.GetMidY();
        }

        nfloat originX;
        if (mauiPopup.PreferredContentSize.Width < frame.Width)
        {
            originX = popup.HorizontalOptions switch
            {
                Microsoft.Maui.Primitives.LayoutAlignment.Start => mauiPopup.PreferredContentSize.Width / 2,
                Microsoft.Maui.Primitives.LayoutAlignment.End => frame.Width - (mauiPopup.PreferredContentSize.Width / 2),
                Microsoft.Maui.Primitives.LayoutAlignment.Center => frame.GetMidX(),
                _ => frame.GetMidX()
            };
        }
        else
        {
            originX = -frame.GetMidX();
        }

        mauiPopup.PopoverPresentationController.SourceRect = new CGRect(originX, originY, 0, 0);
        mauiPopup.PopoverPresentationController.PermittedArrowDirections = 0;
        mauiPopup.PopoverPresentationController.PopoverLayoutMargins = new UIEdgeInsets(1, 0, 0, 0);
    }
    else
    {
        var view = popup.Anchor.ToPlatform(popup.Handler?.MauiContext ?? throw new InvalidOperationException($"{nameof(popup.Handler.MauiContext)} Cannot Be Null"));
        mauiPopup.PopoverPresentationController.SourceView = view;
        mauiPopup.PopoverPresentationController.SourceRect = view.Bounds;
    }
}

Changed how the SourceRect position is calculated depending on whether it does not exceed the size of the
Window's frame. I specified PopoverLayoutMargins so that the display area of the Popup extends to the entire screen.
I also posted in Issue, but if the Top value of PopoverLayoutMargins is 0, the margin will be taken, so I intentionally
set it to 1.

mauiPopup.PopoverPresentationController.PopoverLayoutMargins = new UIEdgeInsets(1, 0, 0, 0);

Originally there was a calculation process for position correction for MacCatalyst, but it was deleted because it
does not make sense.

if (DeviceInfo.Current.Platform == DevicePlatform.MacCatalyst)
{
    if (popup.VerticalOptions == Microsoft.Maui.Primitives.LayoutAlignment.End)
    {
        originY -= (mauiPopup.PreferredContentSize.Height / 2);
    }

    if (popup.HorizontalOptions == Microsoft.Maui.Primitives.LayoutAlignment.End)
    {
        originX -= (mauiPopup.PreferredContentSize.Width);
    }

    if (popup.HorizontalOptions == Microsoft.Maui.Primitives.LayoutAlignment.Center)
    {
        originX -= (mauiPopup.PreferredContentSize.Width / 2);
    }
}

I changed the PopupExtensions.android.cs file to:

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

public static void SetSize(this Dialog dialog, in IPopup popup, in AView container)
{
    ArgumentNullException.ThrowIfNull(popup.Content);

    int horizontalParams, verticalParams;

    var window = GetWindow(dialog);
    var context = dialog.Context;
    var density = context.GetDisplayDensity();

    var decorView = (ViewGroup)window.DecorView;
    var child = decorView.GetChildAt(0) ?? throw new NullReferenceException();

    int realWidth = 0,
        realHeight = 0,
        realContentWidth = 0,
        realContentHeight = 0;

    CalculateSizes(popup, context, ref realWidth, ref realHeight, ref realContentWidth, ref realContentHeight);

    if (context.GetWindow() is IWindow windowManager)
    {
        var resourceIdStatus = context.Resources?.GetIdentifier("status_bar_height", "dimen", "android") ?? 0;
        var statusBarHeight = resourceIdStatus > 0 ? context.Resources?.GetDimensionPixelSize(resourceIdStatus) ?? 0 : 0;
        var resourceIdNavigation = context.Resources?.GetIdentifier("navigation_bar_height", "dimen", "android") ?? 0;
        var navigationBarHeight = resourceIdNavigation > 0 ? context.Resources?.GetDimensionPixelSize(resourceIdNavigation) ?? 0 : 0;
        if (windowManager.Height < windowManager.Width)
        {
            navigationBarHeight = 0;
        }

        realWidth = realWidth <= windowManager.Width * density ? realWidth : (int)(windowManager.Width * density);
        realHeight = realHeight <= (windowManager.Height * density - (navigationBarHeight + statusBarHeight)) ? realHeight : (int)(windowManager.Height * density - (navigationBarHeight + statusBarHeight));
        window.SetLayout(realWidth, realHeight);
    }

    var childLayoutParams = (FrameLayout.LayoutParams)(child.LayoutParameters ?? throw new NullReferenceException());
    childLayoutParams.Width = realWidth;
    childLayoutParams.Height = realHeight;
    child.LayoutParameters = childLayoutParams;

    if (realContentWidth > -1)
    {
        var inputMeasuredWidth = realContentWidth > realWidth ? realWidth : realContentWidth;
        container.Measure(inputMeasuredWidth, (int)MeasureSpecMode.Unspecified);
        horizontalParams = container.MeasuredWidth;
    }
    else
    {
        container.Measure(realWidth, (int)MeasureSpecMode.Unspecified);
        horizontalParams = container.MeasuredWidth > realWidth ? realWidth : container.MeasuredWidth;
    }

    if (realContentHeight > -1)
    {
        verticalParams = realContentHeight;
    }
    else
    {
        var inputMeasuredWidth = realContentWidth > -1 ? horizontalParams : realWidth;
        container.Measure(inputMeasuredWidth, (int)MeasureSpecMode.Unspecified);
        verticalParams = container.MeasuredHeight > realHeight ? realHeight : container.MeasuredHeight;
    }

    var containerLayoutParams = new FrameLayout.LayoutParams(horizontalParams, verticalParams);

    ... omit
}

I got the DisplayDensity to calculate the screen size and not exceed the screen size if the Popup's size
exceeds the screen size. That point is below. I'm getting the height of the status bar and navigation bar
and calculating the content area. If the screen orientation is landscape, the height of the NavigationBar is set to 0.

if (context.GetWindow() is IWindow windowManager)
{
    var resourceIdStatus = context.Resources?.GetIdentifier("status_bar_height", "dimen", "android") ?? 0;
    var statusBarHeight = resourceIdStatus > 0 ? context.Resources?.GetDimensionPixelSize(resourceIdStatus) ?? 0 : 0;
    var resourceIdNavigation = context.Resources?.GetIdentifier("navigation_bar_height", "dimen", "android") ?? 0;
    var navigationBarHeight = resourceIdNavigation > 0 ? context.Resources?.GetDimensionPixelSize(resourceIdNavigation) ?? 0 : 0;
    if (windowManager.Height < windowManager.Width)
    {
        navigationBarHeight = 0;
    }

    realWidth = realWidth <= windowManager.Width * density ? realWidth : (int)(windowManager.Width * density);
    realHeight = realHeight <= (windowManager.Height * density - (navigationBarHeight + statusBarHeight)) ? realHeight : (int)(windowManager.Height * density - (navigationBarHeight + statusBarHeight));
    window.SetLayout(realWidth, realHeight);
}

I changed the PopUpHandler.android.cs file to:

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

protected override void ConnectHandler(MauiPopup platformView)
{
    Container = platformView.SetElement(VirtualView);
    if (Container is not null)
    {
        Container.LayoutChange += OnLayoutChange;
    }
}

protected override void DisconnectHandler(MauiPopup platformView)
{
    platformView.Dispose();
    if (Container is not null)
    {
        Container.LayoutChange -= OnLayoutChange;
    }
}

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

I listened to the LayoutChange event and called the SetSize method of the PopupExtensions class
on screen rotation so that the size of the Popup would be updated dynamically.

I have solved two issues by above.

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/pull/XYZ

Additional information

I used the following that I made myself to verify the operation.

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

Below are the verification results.

This is the verification result when the popup size does not exceed the screen size.
I specified the Size below.

Size = "100, 100"

Changed HorizontalOptions and VerticalOptions of Popup as below.

HorizontalOptions = "Start"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "Center"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "End"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "Fill"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "Start"
VerticalOptions = "Center"

[Android]	[iOS]

HorizontalOptions = "Center"
VerticalOptions = "Center"

[Android]	[iOS]

HorizontalOptions = "End"
VerticalOptions = "Center"

[Android]	[iOS]

HorizontalOptions = "Fill"
VerticalOptions = "Center"

[Android]	[iOS]

HorizontalOptions = "Start"
VerticalOptions = "End"

[Android]	[iOS]

HorizontalOptions = "Center"
VerticalOptions = "End"

[Android]	[iOS]

HorizontalOptions = "End"
VerticalOptions = "End"

[Android]	[iOS]

HorizontalOptions = "Fill"
VerticalOptions = "End"

[Android]	[iOS]

HorizontalOptions = "Start"
VerticalOptions = "Fill"

[Android]	[iOS]

HorizontalOptions = "Center"
VerticalOptions = "Fill"

[Android]	[iOS]

HorizontalOptions = "End"
VerticalOptions = "Fill"

[Android]	[iOS]

HorizontalOptions = "Fill"
VerticalOptions = "Fill"

[Android]	[iOS]

This is the verification result when the popup size does not exceed the screen size.
I specified the size below.

Size = "100, 1000"

Changed HorizontalOptions and VerticalOptions of Popup as below.

HorizontalOptions = "Start"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "Center"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "End"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "Fill"
VerticalOptions = "Start"

[Android]	[iOS]

Other patterns are the same, so they are omitted.

I specified the Size below.

Size = "1000, 100"

Changed HorizontalOptions and VerticalOptions of Popup as below.

HorizontalOptions = "Start"
VerticalOptions = "Start"

[Android]	[iOS]

HorizontalOptions = "Start"
VerticalOptions = "Center"

[Android]	[iOS]

HorizontalOptions = "Start"
VerticalOptions = "End"

[Android]	[iOS]

HorizontalOptions = "Start"
VerticalOptions = "Fill"

[Android]	[iOS]

Other patterns are the same, so they are omitted.

I specified the Size below.

Size = "1000, 1000"

Changed HorizontalOptions and VerticalOptions of Popup as below.

HorizontalOptions = "Start"
VerticalOptions = "Start"

[Android]	[iOS]

Other patterns are the same, so they are omitted.

The results are the same on both iOS and Android, confirming that the popup is displayed at the intended size and position.

Finally, it is a verification result when the terminal is rotated on the Android device.
I specified Size, HorizontalOptions, VerticalOptions as below.

Size = "100, 1000"
HorizontalOptions = "Center"
VerticalOptions = "Center"

[Before rotation]	[After rotation]

From the above, you can confirm that the Popup has been resized after rotation.

…onException`, Add message to `InvalidOperationException`

brminnick

Thanks @cat0363!

I confirmed that this code definitely fixes the Popup layout / sizing bugs.

I'd love to avoid using hard-coded strings on Android if it's possible. Hopefully it is! If it is truly the only way to retrieve this data, then we can merge it, but I'd prefer to avoid it if possible.

Just FYI - I updated the exceptions in PopupExtensions.android.cs. It isn't directly related to this bug fix. Making these exceptions for descriptive will better help devs in the future.

src/CommunityToolkit.Maui.Core/Views/Popup/PopupExtensions.android.cs

src/CommunityToolkit.Maui.Core/Views/Popup/PopupExtensions.macios.cs

…1320

cat0363 · 2023-08-08T00:09:55Z

@brminnick, I am glad that I was able to help you even a little.
Thanks for the refactoring. The code you wrote is beautiful and very instructive.

brminnick

Thanks so much @cat0363!!

This PR is going to make a lot of devs very happy 💯

cat0363 · 2023-08-09T00:40:49Z

@brminnick, Thank you, too.
Thank you to those who submitted issues.
I'm glad that many issues have been resolved.

P.S.
I added it to the review comment to confirm that there is no discrepancy in recognition.

brminnick · 2023-08-09T17:13:16Z

Ah - sorry - I finally understand the reason behind UIEdgeInserts now: iOS automatically assigns padding/margin to the Popup that is different from what the developer has defined in .NET MAUI, and specifying UIEdgeInserts eliminates this default behavior.

I agree - we should ensure that Popup is laid-out the same on Android + iOS.

If you're able to, feel free to open up a new PR for this and I'll review + approve it. I think we may have an existing Issue already opened too.

cat0363 · 2023-08-09T23:47:51Z

@brminnick , Thank you for releasing the latest version of CommunityToolkit.
I'm glad you understood what I wanted to say about PopoverLayoutMargins.
Search for related issues, and if there are no related issues, create a new issue.
I will create a new PR for this matter, so please wait for a while.

cat0363 and others added 4 commits August 4, 2023 13:51

Fix display position and size of Popup

37aa8a1

Consider the height of the status bar and navigation bar

e4681ea

Merge branch 'main' into Issue-1317

b90c1f7

Update Formatting, Change NullReferenceException -> `InvalidOperati…

74907ae

…onException`, Add message to `InvalidOperationException`

brminnick reviewed Aug 4, 2023

View reviewed changes

src/CommunityToolkit.Maui.Core/Views/Popup/PopupExtensions.android.cs Outdated Show resolved Hide resolved

src/CommunityToolkit.Maui.Core/Views/Popup/PopupExtensions.macios.cs Outdated Show resolved Hide resolved

cat0363 and others added 9 commits August 7, 2023 15:40

Changed how to get the height of Navigation Bar and Status Bar

eacce38

Fix conflict

bc236f6

Fix conflict

8a3d6b4

Add PopupLayoutPage

d1d98a7

Rename PopupLayout -> PopupLayoutAlignment

6d60d7c

Merge branch 'main' into Issue-1317

8f1e7f3

Update Formatting

15af44d

Throw exception for unsupported switch case

15a84c7

Merge branch 'Issue-1317' of https://github.com/cat0363/Maui into pr/…

57f45d1

…1320

brminnick added 3 commits August 8, 2023 08:06

Merge branch 'main' into Issue-1317

e411332

Remove UIEdgeInserts

1f6257d

Merge branch 'main' into Issue-1317

3950c3e

brminnick approved these changes Aug 8, 2023

View reviewed changes

brminnick merged commit bc38859 into CommunityToolkit:main Aug 8, 2023
8 checks passed

cat0363 mentioned this pull request Aug 10, 2023

[BUG] Popup has an outer margin on iOS. #1328

Closed

2 tasks

cat0363 mentioned this pull request Aug 16, 2023

[BUG] Can't apply style to popup #1345

Closed

2 tasks

cat0363 mentioned this pull request Nov 6, 2023

[BUG]? Popup does not wrap content (on Android) #1150

Closed

2 tasks

VNGames mentioned this pull request Dec 17, 2023

[BUG] Popup - Size is not respected in Landscape mode on iOS #1604

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix display position and size of Popup #1320

Fix display position and size of Popup #1320

cat0363 commented Aug 4, 2023 •

edited by brminnick

brminnick left a comment

cat0363 commented Aug 8, 2023

brminnick left a comment

cat0363 commented Aug 9, 2023

brminnick commented Aug 9, 2023 •

edited

cat0363 commented Aug 9, 2023

Fix display position and size of Popup #1320

Fix display position and size of Popup #1320

Conversation

cat0363 commented Aug 4, 2023 • edited by brminnick

Description of Change

Linked Issues

PR Checklist

Additional information

brminnick left a comment

Choose a reason for hiding this comment

cat0363 commented Aug 8, 2023

brminnick left a comment

Choose a reason for hiding this comment

cat0363 commented Aug 9, 2023

brminnick commented Aug 9, 2023 • edited

cat0363 commented Aug 9, 2023

cat0363 commented Aug 4, 2023 •

edited by brminnick

brminnick commented Aug 9, 2023 •

edited