[Enhancement] GoBack fails when NavigationPage is pushed as a Modal

[dansiegel]

Description

When trying to navigate back from the root of a NavigationPage which has been pushed modally the navigation fails with the last ditch error message "Unknown error occured."

Steps to Reproduce

1. From an existing navigation stack say NavigationPage/ViewA, modally push a new NavigationPage with a new root view like:

await NavigationService.NavigateAsync("NavigationPage?useModalNavigation=true/ViewB");

2. Now from the ViewModel in ViewB call GoBack(). The NavigationResult will contain an exception from line 120 in the PageNavigationService:

result.Exception = new Exception("Unknown error occured.");
return result;

This occurs because we check to see if the popped page is not null but we never handle the scenario where the popped page is null. Since ViewB is the Root Page of the NavigationPage it cannot be popped and it's parent NavigationPage must instead be popped.

Expected Behavior

The navigation succeeds and returns you to the View behind your Modal.

Actual Behavior

The navigation fails

Basic Information

• Version with issue: 7.1
• Last known good version: n/a
• Xamarin.Forms version: 3.6
• IDE: 16.1

Screenshots

n/a

Reproduction Link

n/a

[brianlagunas]

The guidance in this case is to force a modal go back.

[dansiegel]

this is true... however this would be a use case the NavigationService should be able to figure out... we aren't bothering to handle when the popped page is null... that should be our clue.

I would propose that we do some checking when we encounter this scenario. For this exact case ViewB would have a parent NavigationPage… the Parent element of the NavigationPage is our Application but the Application.MainPage is not our NavigationPage… this should tell us that we should use Modal Navigation.

[dansiegel]

looking more at this... we currently share some logic for Push/Pop to determine whether we want Modal navigation or not. It seems to me that if we change line 95 to point to a new method as shown below it would fix the issue and let us better determine if we should use Modal or non-Modal navigation. My gut tells me there are probably a couple of edge additional cases we may want to check for. This would however improve the intelligence around whether we automatically determine we should use modal navigation or not.

internal bool UseModalNavigationForPop(Page currentPage, bool? useModalNavigationDefault)
{
    bool useModalNavigation = true;

    if (useModalNavigationDefault.HasValue)
    {
        useModalNavigation = useModalNavigationDefault.Value;
    }
    else if (currentPage is NavigationPage navPage)
    {
        useModalNavigation = navPage.CurrentPage == navPage.RootPage &&
            _applicationProvider.MainPage != currentPage;
    }
    else if(currentPage.Parent is NavigationPage navParent)
    {
        useModalNavigation = navParent.CurrentPage == navParent.RootPage &&
            _applicationProvider.MainPage != navParent;
    }

    return useModalNavigation;
}

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.