Rg.plugins.popup: iOS issue only, xamarin forms 5.0.0.1558-pre3 seems to break when showing just a simple popup.

[RonnyRos]

🐛 Bug Report

iOS only, android seems to work.
The popup does not seem to show running a simple popup in XF 5+.
If you run XF 4.8 it seems to appears correctly.

Expected behavior

Popup to work.

Reproduction steps

Upgrade to newest xamarin forms 5.0.0.1558-pre3 from 4.8.

Configuration

Windows VS 2019 16.7.4

Platform:

• [ ] :iphone: iOS
• [ ] :robot: Android
• [ ] :checkered_flag: WPF
• [ ] :earth_americas: UWP
• [ ] :apple: MacOS
• [ ] :tv: tvOS
• [x] :monkey: Xamarin.Forms

[ccsousa01]

I've spent the last 2 hours searching for a reason, thought it was maybe iOS v14 update, but must be Xamarin Forms 5. I am using 5.0.0.1539-pre2. The IPopupNavigation instance "_popup" is no longer working. Suspecting a threading issue as the page is frozen after calling await _popup.PushAsync(new MyModal()). Works just fine on Android but not on iOS, at least tested v13 & v14.

[Tommigun1980]

I'm in the same boat. Will have to downgrade to XF 4.8 for now, but I assume XF 5 is very close to release so would really appreciate a fix to ensure XF 5 compatibility.

Thanks a lot!

[Tommigun1980]

Hello! Are there any plans to support Xamarin.Forms 5? It is already in preview-3.

Thank you.

[LuckyDucko]

I can also confirm that it works on MacOS, it seems that iOS is the issue here.

@ccsousa01 is correct with the freezing, and with the threading that is used within Rg, it could be a problem area.

[LuckyDucko]

The issue seems to be that the UpdateSize function in PlatformExtension for the iOS project is continuously called, leading to an endless loop.

It seems that currentElement.BatchBegin and currentElement.BatchCommit within this function set off, directly or indirectly, ViewDidLayoutSubviews in PopupPageRenderer in the iOS Project, which contains UpdateSize, which then goes around in circles.

I am unsure what behaviour has changed to now make this happen, but i do have a bandaid solution, where I have wrapped
BatchBegin/BatchCommit and any of its containing functionality within an If statement to check if it's actually necessary.

        public static void UpdateSize(this PopupPageRenderer renderer)
        {
            var currentElement = renderer.CurrentElement;

            if (renderer.View?.Superview?.Frame == null || currentElement == null)
                return;

            var superviewFrame = renderer.View.Superview.Frame;
            var applactionFrame = UIScreen.MainScreen.ApplicationFrame;


            var systemPadding = new Thickness
            {
                Left = applactionFrame.Left,
                Top = applactionFrame.Top,
                Right = applactionFrame.Right - applactionFrame.Width - applactionFrame.Left,
                Bottom = applactionFrame.Bottom - applactionFrame.Height - applactionFrame.Top + renderer.KeyboardBounds.Height
            };

            //New check to see if batch begin necessary
            if (currentElement.SystemPadding != systemPadding && renderer.Element.Width != superviewFrame.Width && renderer.Element.Height != superviewFrame.Height)
            {
                currentElement.BatchBegin();

                currentElement.SystemPadding = systemPadding;

                renderer.SetElementSize(new Size(superviewFrame.Width, superviewFrame.Height));

                currentElement.BatchCommit();
            }
        }

If no-one figures out another reason for this functionality in a deep dive, this should do to fix the issue in a PR. however, I'll wait a bit to see if someone more knowledgeable can fix the issue at its core.

[Tommigun1980]

The issue seems to be that the UpdateSize function in PlatformExtension for the iOS project is continuously called, leading to an endless loop.

It seems that currentElement.BatchBegin and currentElement.BatchCommit within this function set off, directly or indirectly, ViewDidLayoutSubviews in PopupPageRenderer in the iOS Project, which contains UpdateSize, which then goes around in circles.

I am unsure what behaviour has changed to now make this happen, but i do have a bandaid solution, where I have wrapped
BatchBegin/BatchCommit and any of its containing functionality within an If statement to check if it's actually necessary.

        public static void UpdateSize(this PopupPageRenderer renderer)
        {
            var currentElement = renderer.CurrentElement;

            if (renderer.View?.Superview?.Frame == null || currentElement == null)
                return;

            var superviewFrame = renderer.View.Superview.Frame;
            var applactionFrame = UIScreen.MainScreen.ApplicationFrame;


            var systemPadding = new Thickness
            {
                Left = applactionFrame.Left,
                Top = applactionFrame.Top,
                Right = applactionFrame.Right - applactionFrame.Width - applactionFrame.Left,
                Bottom = applactionFrame.Bottom - applactionFrame.Height - applactionFrame.Top + renderer.KeyboardBounds.Height
            };

            //New check to see if batch begin necessary
            if (currentElement.SystemPadding != systemPadding && renderer.Element.Width != superviewFrame.Width && renderer.Element.Height != superviewFrame.Height)
            {
                currentElement.BatchBegin();

                currentElement.SystemPadding = systemPadding;

                renderer.SetElementSize(new Size(superviewFrame.Width, superviewFrame.Height));

                currentElement.BatchCommit();
            }
        }

If no-one figures out another reason for this functionality in a deep dive, this should do to fix the issue in a PR. however, I'll wait a bit to see if someone more knowledgeable can fix the issue at its core.

Hi @LuckyDucko and thanks so much for your research!

Question: Should this maybe be posted to https://github.com/xamarin/Xamarin.Forms/issues as a bug, as it may be an unintended change on Xamarin.Forms's side in XF 5? At least they could have a look at it and declare it a Xamarin.Forms regression, or alternatively if it is indeed the way to go then Rg.Plugins.Popup will need to be updated with your PR.

What do you think? Would you be up for posting in that case as you have all the technical details?

Thanks so much!

[LuckyDucko]

@Tommigun1980 no worries

I do believe its an Rg.Plugins.Popup issue, as we override default behaviour which then creates this endless loop so most likely I'll make a PR containing this soon

[stevewinn74]

+1 for merging this and pushing to nuget ASAP 👍🏻

[Tommigun1980]

@LuckyDucko saves the day!

[RonnyRos]

Good stuff @LuckyDucko ! :D thanks

[wstanczewski]

Hi, when can we expect updated nuget with that fix? It would be very helpful, thanks :)

[softlion]

+1 for the beta nuget!

[martijn00]

Done

[martijn00]

No reports of that yet. Any idea why?

[softlion]

works fine ok. xamarin issue.