Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix Shell Navigation for Hierarchally registered Global Routes #13330

Merged
merged 14 commits into from
Jan 15, 2021
Merged

Conversation

PureWeen
Copy link
Contributor

@PureWeen PureWeen commented Jan 7, 2021

Description of Change

Fixed Hierarchical Global Route Registration so that it would correctly apply the route registered to the current state of shell

If a user is registering full route path Route.Register("path1/path2/path3") and Shell is currently at "path1/path2"

Then the user should be able to navigate to that route via

  • GotoAsync("path3") or Route.Register("path1/path2/path3")

Issues Resolved

Platforms Affected

  • Core/XAML (all platforms)

Testing Procedure

  • unit test included

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Jan 7, 2021
@PureWeen PureWeen added this to the 5.0.0 milestone Jan 7, 2021
@PureWeen PureWeen added this to To do in vNext+1 (5.0.0) via automation Jan 7, 2021
@PureWeen PureWeen moved this from To do to In Review in vNext+1 (5.0.0) Jan 7, 2021
namespace Xamarin.Forms
{
[DebuggerDisplay("RequestDefinition = {Request}, StackRequest = {StackRequest}")]
internal class NavigationRequest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No changes here just moving this class to its own file

[DebuggerDisplay("Full = {FullUri}, Short = {ShortUri}")]
internal class RequestDefinition
{
public RequestDefinition(RouteRequestBuilder theWinningRoute, Shell shell)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No changes here just moving this class to its own file

/// <summary>
/// This attempts to locate the intended route trying to be navigated to
/// </summary>
internal class RouteRequestBuilder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No changes here just moving this class to its own file

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jan 9, 2021
@PureWeen PureWeen removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jan 9, 2021
@PureWeen PureWeen requested review from jsuarezruiz and hartez and removed request for rmarinho January 14, 2021 17:00
@PureWeen
Copy link
Contributor Author

Failing Tests are unrelated


List<string> CollapsePath(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This good just got moved to ShellUriHandler.CollapsePath

@@ -475,24 +475,6 @@ void RemoveExcessPathsWithinTheRoute()
}
}

List<Page> BuildFlattenedNavigationStack(List<Page> startingList, IReadOnlyList<Page> modalStack)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code just got moved to ShellNavigationManager.BuildFlattenedNavigationStack

@rmarinho rmarinho merged commit de4d0bc into 5.0.0 Jan 15, 2021
vNext+1 (5.0.0) automation moved this from In Review to Done Jan 15, 2021
@rmarinho rmarinho deleted the fix_13328 branch January 15, 2021 18:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/navigation a/shell 🐚 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. Core t/bug 🐛
Projects
No open projects
4 participants