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

Fix string constructor for ShellNavigationState #13478

Merged
merged 1 commit into from
Feb 5, 2021
Merged

Fix string constructor for ShellNavigationState #13478

merged 1 commit into from
Feb 5, 2021

Conversation

PureWeen
Copy link
Contributor

Description of Change

Fix string constructor for ShellNavigationState so it sets both Location and FullLocation

Issues Resolved

Platforms Affected

  • Core/XAML (all platforms)

Testing Procedure

  • unit tests 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 20, 2021
@PureWeen PureWeen added this to the 5.0.0 milestone Jan 20, 2021
@PureWeen PureWeen added this to To do in vNext+1 (5.0.0) via automation Jan 20, 2021
@PureWeen PureWeen moved this from To do to In Review in vNext+1 (5.0.0) Jan 20, 2021
@PureWeen PureWeen removed the request for review from StephaneDelcroix January 20, 2021 21:32
@@ -637,6 +643,296 @@ public async Task InitialNavigatingArgs()
null, "//item");
}


Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this is just moving code to the correct file. No changes happen here

public class ShellNavigationStateTests : ShellTestBase
{
[Test]
public void LocationInitializedWithUri()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These Tests are new

@@ -16,201 +16,6 @@ public override void TearDown()
Routing.Clear();
}

[Test]
public async Task RouteWithGlobalPageRoute()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this is just moving code to the correct file. No changes happen here

public ShellNavigationState(Uri location)
{
FullLocation = location;
Location = TrimDownImplicitAndDefaultPaths(FullLocation);
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 is the primary change from this PR

@@ -852,7 +852,7 @@ protected override bool OnBackButtonPressed()
return true;
}

var args = new ShellNavigatingEventArgs(this.CurrentState, "", ShellNavigationSource.Pop, true);
var args = new ShellNavigatingEventArgs(this.CurrentState, new ShellNavigationState(""), ShellNavigationSource.Pop, true);
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 change is just so we're avoiding using the implicit operator

@@ -36,7 +36,7 @@ public async Task GoToAsync(ShellNavigationParameters shellNavigationParameters)
if (shellNavigationParameters.PagePushing != null)
Routing.RegisterImplicitPageRoute(shellNavigationParameters.PagePushing);

ShellNavigationState state = shellNavigationParameters.TargetState ?? Routing.GetRoute(shellNavigationParameters.PagePushing);
var state = shellNavigationParameters.TargetState ?? new ShellNavigationState(Routing.GetRoute(shellNavigationParameters.PagePushing), false);
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 change is just so we're avoiding using the implicit operator

@@ -1063,7 +1063,7 @@ protected override async Task<Page> OnPopAsync(bool animated)
var navigationParameters = new ShellNavigationParameters()
{
Animated = animated,
TargetState = ".."
TargetState = new ShellNavigationState("..")
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 change is just so we're avoiding using the implicit operator

@PureWeen
Copy link
Contributor Author

Failing tests not related

@hartez hartez added this to To Do in 5.0.0 SR 3 via automation Feb 2, 2021
@hartez hartez moved this from To Do to Needs Review in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez removed this from In Review in vNext+1 (5.0.0) Feb 2, 2021
@rmarinho rmarinho merged commit 6b73a62 into 5.0.0 Feb 5, 2021
5.0.0 SR 3 automation moved this from PR Needs Review to Done Feb 5, 2021
@rmarinho rmarinho deleted the fix_13422 branch February 5, 2021 17:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/shell 🐚 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. i/regression t/bug 🐛
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Bug] Location property no longer set via ShellNavigationState's Uri ctor
4 participants