Skip to content

Conversation

bijington
Copy link
Contributor

@bijington bijington commented Jun 25, 2025

Description of Change

This introduces the ability for developers to determine whether the previous page displayed in their app was a PopupPage as below

public class MyPage : Page
{
    protected override void OnNavigatedTo(NavigatedToEventArgs args)
    {
        base.OnNavigatedTo(args);

        if (args.WasPreviousPageAToolkitPopup() is true)
        {
            return;
        }

        // do other stuff
    }
}

The reason for the extension method over allowing a developer to just check the PreviousPage property is twofold:

  1. PreviousPage is currently internal
  2. PopupPage is also internal

I understand that internal access to MAUI will be removed from MAUI for .NET 10.0 but it also appears that this will be introduced dotnet/maui#28384. If that is the case the code in this PR will continue to function when .NET 10.0 ships.

Linked Issues

  • Fixes #

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) - the constructor for NavigatedToEventArgs is internal so it would require some reflection to construct something to test.
  • 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/pulls

Additional information

@Copilot Copilot AI review requested due to automatic review settings June 25, 2025 05:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds an extension method to NavigatedToEventArgs that allows developers to detect if the previous page was a Toolkit popup.

  • Introduces WasPreviousPageAToolkitPopup extension.
  • Checks args.PreviousPage is PopupPage.
  • Adds new file under Extensions namespace.
Comments suppressed due to low confidence (2)

src/CommunityToolkit.Maui/Extensions/NavigatedToEventArgsExtensions.shared.cs:11

  • The XML comment references but the method checks for PopupPage; update the cref to for consistency.
	/// Determines whether the previous page was a Community Toolkit <see cref="Popup"/>.

src/CommunityToolkit.Maui/Extensions/NavigatedToEventArgsExtensions.shared.cs:15

  • Consider adding unit tests covering the cases where the previous page is a PopupPage and when it's not to ensure the extension behaves correctly.
	public static bool WasPreviousPageAToolkitPopup(this NavigatedToEventArgs args) => args.PreviousPage is PopupPage;

@bijington
Copy link
Contributor Author

Thanks for the approval @pictos

@jfversluis do you have any understanding on whether dotnet/maui#28384 will make it into .NET 10.0?

@jfversluis
Copy link
Member

Asked @PureWeen or maybe @jsuarezruiz knows

@TheCodeTraveler TheCodeTraveler added pending documentation This feature requires documentation approved This Proposal has been approved and is ready to be added to the Toolkit needs discussion Discuss it on the next Monthly standup labels Jul 1, 2025
@david-maw
Copy link

While this seems useful, I'd need the same extension on ShellNavigatingEventArgs to address my use case where what I want to do is block navigation to anything except a popup by setting Shell.Current.Navigating += PreventPrematureNavigation; and defining this nasty looking function:

    private static void PreventPrematureNavigation(object sender, ShellNavigatingEventArgs e)
    {
        if (e.CanCancel)
        {// If we can cancel the navigation, do so if it is not for a popup
            if (e.Source == ShellNavigationSource.Push && e.Target.Location.OriginalString.Contains("Popup"))
            {
                // Don't cancel if the navigation is to a popup
                RecordMsg("Splash Page: Allowed Navigation to " + e.Target.Location.OriginalString);
            }
            else if (e.Source == ShellNavigationSource.PopToRoot && e.Current.Location.OriginalString.Contains("Popup"))
            {
                // Don't cancel if the navigation is from a popup
                RecordMsg("Splash Page: Allowed Navigation from " + e.Current.Location.OriginalString);
            }
            else
            {
                e.Cancel();
                RecordMsg("Splash Page: Navigation canceled because it was not for a popup");
            }
        }
        else
        {
            // If we can't cancel the navigation, just log it
            RecordMsg("Splash Page: Navigation not canceled because CanCancel was false");
        }
    }

@bijington
Copy link
Contributor Author

bijington commented Jul 4, 2025

@david-maw what's your use case for preventing navigation? I know we've discussed it somewhere but I don't recall the exact reason.

@david-maw
Copy link

Annoyingly, Play Store testing seems to be able to cause navigation even when there's no UI to do it (it gets to various pages before initialization has completed, which is a Bad Thing). I suspect it may be a timing window that I have not been able to duplicate, and I originally solved it by prohibiting all navigation while initialization was in process. That's no longer sufficient because popups now use navigation, so I need an explicit exception for them.

@bijington
Copy link
Contributor Author

Hmm yeah I'd try to get to the bottom of why it's happening rather than try to cancel things in this way. One way might be to collect analytics if it only happens in a released version. Can you reproduce in a released version without going through the play store?

@david-maw
Copy link

I have not been able to reproduce it outside Play Store testing which is why I suspect some sort of timing window.

@david-maw
Copy link

Of course once I had a workaround I went on to more pressing issues. Still, the need to create an exception for popup is reason enough to take another run at it. Who knows, in the intervening couple of years the problem, whatever it is, could have gone away and then there would be no need for the workaround. Also, pigs might fly.

@david-maw
Copy link

I should add that current turnaround time for Play Store submissions varies between an hour and several days and this only ever failed one time in ten or so, meaning it might be a while before I learn anything.

@david-maw
Copy link

I think I have a clue as to what is happening that creates the requirement to selectively inhibit navigation. My app uses the Shell implementation and its AppShell.xaml looks like this:

    <TabBar>
        <!--This is the initial page, used to decide what to show the user first, it is normally never visible -->
        <ShellContent ContentTemplate="{DataTemplate view:GettingStartedPage}" Route="{x:Static local:Routes.GettingStartedPage}"/>
        <!--This is the initialization page the user normally sees, ShellContent items with routes are navigated to in code-->
        <ShellContent ContentTemplate="{DataTemplate view:SplashPage}" Route="{x:Static local:Routes.SplashPage}"/>
    </TabBar>
    <ShellContent Title="Items" ContentTemplate="{DataTemplate view:LineItemsPage}" Route="{x:Static local:Routes.LineItemsPage}"/>
    <ShellContent Title="Totals" ContentTemplate="{DataTemplate view:TotalsPage}" Route="{x:Static local:Routes.TotalsPage}"/>
    <MenuItem      Text="Camera" Clicked="GoToImagePageWithCamera"/>
    <ShellContent Title="Image" ContentTemplate="{DataTemplate view:ImagePage}" Route="{x:Static local:Routes.ImagePage}"/>
    <ShellContent Title="Properties" ContentTemplate="{DataTemplate view:PropertiesPage}" Route="{x:Static local:Routes.PropertiesPage}"/>
    ...

By setting Shell.TabBarIsVisible="False" and Shell.FlyoutBehavior="Disabled" on GettingStartedPage the user is presented with a page that has no user navigation capabilities. What the page actually does is push a help page onto the stack (conditionally, but that's a detail). When the help page is done it invokes Shell.Current.Navigation.PopAsync to return to GettingStartedPage. This time round GettingStartedPage navigates to SplashPage and is never used again.

Based on a recent failure (so a sample size of one), I think what happens is that when the help page invokes PopAsync it occasionally goes to LineItemsPage (and I see a failure there because SplashPage hasn't been invoked yet). Usually, this attempt is preempted by GettingStartedPage navigating to SplashPage, which is the intended flow, but rarely it goes toLineItemsPage instead and faults.

I think the fix is just to inhibit any navigation to places I don't approve of until the app is done initializing, but hopefully I've got enough tracing logic that I can catch the next attempt at a failure and confirm my suspicions.

@david-maw
Copy link

I think I have a clue as to what is happening that creates the requirement to selectively inhibit navigation. My app uses the Shell implementation and its AppShell.xaml looks like this:

    <TabBar>
        <!--This is the initial page, used to decide what to show the user first, it is normally never visible -->
        <ShellContent ContentTemplate="{DataTemplate view:GettingStartedPage}" Route="{x:Static local:Routes.GettingStartedPage}"/>
        <!--This is the initialization page the user normally sees, ShellContent items with routes are navigated to in code-->
        <ShellContent ContentTemplate="{DataTemplate view:SplashPage}" Route="{x:Static local:Routes.SplashPage}"/>
    </TabBar>
    <ShellContent Title="Items" ContentTemplate="{DataTemplate view:LineItemsPage}" Route="{x:Static local:Routes.LineItemsPage}"/>
    <ShellContent Title="Totals" ContentTemplate="{DataTemplate view:TotalsPage}" Route="{x:Static local:Routes.TotalsPage}"/>
    <MenuItem      Text="Camera" Clicked="GoToImagePageWithCamera"/>
    <ShellContent Title="Image" ContentTemplate="{DataTemplate view:ImagePage}" Route="{x:Static local:Routes.ImagePage}"/>
    <ShellContent Title="Properties" ContentTemplate="{DataTemplate view:PropertiesPage}" Route="{x:Static local:Routes.PropertiesPage}"/>
    ...

By setting Shell.TabBarIsVisible="False" and Shell.FlyoutBehavior="Disabled" on GettingStartedPage the user is presented with a page that has no user navigation capabilities. What the page actually does is push a help page onto the stack (conditionally, but that's a detail). When the help page is done it invokes Shell.Current.Navigation.PopAsync to return to GettingStartedPage. This time round GettingStartedPage navigates to SplashPage and is never used again.

Based on a recent failure (so a sample size of one), I think what happens is that when the help page invokes PopAsync it occasionally goes to LineItemsPage (and I see a failure there because SplashPage hasn't been invoked yet). Usually, this attempt is preempted by GettingStartedPage navigating to SplashPage, which is the intended flow, but rarely it hits LineItemsPage instead and faults.

I think the fix is just to inhibit any navigation to places I don't approve of until the app is done initializing, but hopefully I've got enough tracing logic that I can catch the next attempt at a failure and confirm my suspicions.

@VladislavAntonyuk VladislavAntonyuk removed the needs discussion Discuss it on the next Monthly standup label Aug 7, 2025
@DavidV1603
Copy link

Would it be possible to add this to ShellNavigatingEventArgs also, to handle the navigation to a PopupPage different than other pages when using the Shell?

@dotnet-policy-service dotnet-policy-service bot added stale The author has not responded in over 30 days help wanted This proposal has been approved and is ready to be implemented labels Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved This Proposal has been approved and is ready to be added to the Toolkit help wanted This proposal has been approved and is ready to be implemented pending documentation This feature requires documentation stale The author has not responded in over 30 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants