Is Maui's ProcessNavigation using the correct logic for Modals?

[BurkusCat]

Description

This is the current (20th June 2023) logic in the ProcessNavigation method in Prism Maui for determining if modal navigation should be used:

        useModalNavigation = pageParameters.ContainsKey(KnownNavigationParameters.UseModalNavigation) ? pageParameters.GetValue<bool>(KnownNavigationParameters.UseModalNavigation) : false;
        if (!useModalNavigation.Value && !MvvmHelpers.HasNavigationPageParent(currentPage) && (currentPage is not FlyoutPage || (currentPage.Parent is FlyoutPage parentFlyout && parentFlyout.Flyout == currentPage)))
            useModalNavigation = true;

I read this logic as:

• if there is a page parameter for modal navigation, use its value
• otherwise, modal navigation defaults to false
• however, if it is set to false initially, there is a chance for it to be auto-set to use modal navigation under certain conditions about its parent being a flyout page etc.

I'm not familiar enough with this code to determine if this is correct or not. So I wanted to ask these questions:

• should this logic also be checking parameters like the animated bool below is set? Currently, it is only checking pageParameters which makes me think it would miss out in certain cases where a parameter is passed (like in the "Steps to Reproduce")
• should useModalNavigation be "auto-set" or should it be controlled by the caller? It seems strange that false could be specified by the caller but this could be automatically overridden & set to true based on where the page is in the navigation stack. Xamarin.Forms Prism doesn't have this logic:

            if (pageParameters.ContainsKey(KnownNavigationParameters.UseModalNavigation))
                useModalNavigation = pageParameters.GetValue<bool>(KnownNavigationParameters.UseModalNavigation);

Apologies if this is the way the logic should be. No specific bug really to report here, just wanted flag it up in case it ever led to strange/unexpected behaviour! :) I don't have the expertise to know what it should be.

Steps to Reproduce

            var param = new NavigationParameters
            {
                { KnownNavigationParameters.UseModalNavigation, true }
            };

            var result = await navigationService.CreateBuilder()
                .AddSegment(nameof(ExamplePage))
                .WithParameters(param)
                .NavigateAsync();

Platform with bug

.NET MAUI

Affected platforms

iOS, Android, Windows

Did you find any workaround?

N/A

Relevant log output

No response

[dansiegel]

@BurkusCat The best I can tell you is that we've tried to cover every scenario we can think of. If you have a scenario that you think is wrong and you can produce a Unit Test that fails with the current logic... and even better if you want to help fix it we do take PRs and love when members of the community get involved to help identify bugs and submit fixes.