Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Shell] Routing: fix infinite loops and remove public apis #5954

Merged
merged 11 commits into from Apr 24, 2019

Conversation

@PureWeen
Copy link
Contributor

commented Apr 18, 2019

Description of Change

  • removed GoToPart from the IShellController code parts because there isn't any platform code that uses them so it's not useful at this point
  • added an exception if someone tries to do a relative route to a shell section as this behavior hasn't been implemented yet so I don't want the behavior to change once we make it so relative routes push to a stack. I also added an internal overload to turn this on for unit tests so unit tests can continue unhindered with validating behavior
  • I fixed the logic when you use an absolute route to navigation via / . It was causing an infinite loop
  • removed getroutekeys and a few additional classes added in pre9 that will become exposed later down the road

API Changes

Removed:

  • Routing.GetRouteKeys
  • class NavigationRequest
  • GotoPart from the IShellController classes as the platforms don't use these methods so they don't need to be apart of the controllers

Platforms Affected

  • Core/XAML (all platforms)

Testing Procedure

  • there are unit tests

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@PureWeen PureWeen requested a review from StephaneDelcroix as a code owner Apr 18, 2019

@PureWeen PureWeen requested review from rmarinho and removed request for StephaneDelcroix Apr 18, 2019

@PureWeen PureWeen requested a review from paymicro Apr 18, 2019

@samhouts samhouts added this to In Review in vCurrent (4.0.0) Apr 22, 2019

@PureWeen PureWeen added the blocker label Apr 23, 2019

@PureWeen PureWeen removed the request for review from rmarinho Apr 23, 2019

@PureWeen

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2019

@samhouts samhouts self-requested a review Apr 24, 2019

@samhouts samhouts self-assigned this Apr 24, 2019

@paymicro
Copy link
Collaborator

left a comment

Works good. I added small comments.

Show resolved Hide resolved Xamarin.Forms.Core/Routing.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Routing.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellUriHandler.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellUriHandler.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellUriHandler.cs Outdated

@PureWeen PureWeen requested a review from paymicro Apr 24, 2019

PureWeen added some commits Apr 24, 2019

@PureWeen PureWeen requested a review from paymicro Apr 24, 2019

Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellNavigationState.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellUriHandler.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellUriHandler.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Shell/ShellUriHandler.cs Outdated

samhouts and others added some commits Apr 24, 2019

Update Xamarin.Forms.Core/Shell/ShellNavigationState.cs
Co-Authored-By: PureWeen <shane94@hotmail.com>
Update Xamarin.Forms.Core/Shell/ShellUriHandler.cs
Co-Authored-By: PureWeen <shane94@hotmail.com>

samhouts and others added some commits Apr 24, 2019

Update Xamarin.Forms.Core/Shell/ShellUriHandler.cs
Co-Authored-By: PureWeen <shane94@hotmail.com>
Update Xamarin.Forms.Core/Shell/ShellUriHandler.cs
Co-Authored-By: PureWeen <shane94@hotmail.com>

@PureWeen PureWeen merged commit 48606a5 into 4.0.0 Apr 24, 2019

12 of 15 checks passed

Xamarin Forms Beta Build #PR-5954 - (2618606) failed
Details
Xamarin Forms Beta (Build Windows Phase Release) Build Windows Phase Release failed
Details
Xamarin Forms Beta (Build Windows Phase debug) Build Windows Phase debug failed
Details
Xamarin Forms Build #PR-5954 - (2618607) succeeded
Details
Xamarin Forms (Build Windows Phase Debug,any cpu) Build Windows Phase Debug,any cpu succeeded
Details
Xamarin Forms (Nuget Phase) Nuget Phase succeeded
Details
Xamarin Forms (OSX Phase) OSX Phase succeeded
Details
Xamarin Forms (Prepare Build Phase) Prepare Build Phase succeeded
Details
Xamarin Forms (Test Phase debug) Test Phase debug succeeded
Details
Xamarin Forms Beta (Build Android [Fast Renderers]) Build Android [Fast Renderers] succeeded
Details
Xamarin Forms Beta (Build Android [Legacy Renderers]) Build Android [Legacy Renderers] succeeded
Details
Xamarin Forms Beta (Build Android [Pre-AppCompat]) Build Android [Pre-AppCompat] succeeded
Details
Xamarin Forms Beta (OSX Phase) OSX Phase succeeded
Details
Xamarin Forms Beta (Prepare Build Phase) Prepare Build Phase succeeded
Details
license/cla All CLA requirements met.
Details

vCurrent (4.0.0) automation moved this from In Review to Done Apr 24, 2019

@PureWeen PureWeen deleted the fix_navigation_infinite branch Apr 24, 2019

@samhouts samhouts added this to the 4.0.0 milestone Apr 29, 2019

@samhouts samhouts changed the title fix infinite loops and remove public apis [Shell] Routing: fix infinite loops and remove public apis Apr 30, 2019

@samhouts samhouts changed the title [Shell] Routing: fix infinite loops and remove public apis fix infinite loops and remove public apis Apr 30, 2019

@samhouts samhouts changed the title fix infinite loops and remove public apis [Shell] Routing: fix infinite loops and remove public apis Apr 30, 2019

AxelUser added a commit to AxelUser/Xamarin.Forms that referenced this pull request Jun 15, 2019

fix infinite loops and remove public apis (xamarin#5954)
* fix infinite loops and remove public apis

* - remove comments, fix location to not have implicit

* force uri to be relative on ios when no scheme specified

* address PR comments

* add error message

* reformat absolute uris

* additional suggestions

* Update Xamarin.Forms.Core/Shell/ShellNavigationState.cs

Co-Authored-By: PureWeen <shane94@hotmail.com>

* Update Xamarin.Forms.Core/Shell/ShellUriHandler.cs

Co-Authored-By: PureWeen <shane94@hotmail.com>

* Update Xamarin.Forms.Core/Shell/ShellUriHandler.cs

Co-Authored-By: PureWeen <shane94@hotmail.com>

* Update Xamarin.Forms.Core/Shell/ShellUriHandler.cs

Co-Authored-By: PureWeen <shane94@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.