-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[UWP] [Shell] Make Navigation and Transition overridable for Shell #12442
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
/azp run |
No pipelines are associated with this pull request. |
@@ -191,7 +175,49 @@ internal void NavigateToContent(ShellNavigationSource source, ShellContent shell | |||
} | |||
} | |||
|
|||
NavigationTransitionInfo GetTransitionInfo(ShellNavigationSource navSource) | |||
protected virtual void OnPopRequested(ShellNavigationSource source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would these be more helpful if they included the full arg like iOS?
protected virtual async void OnPopRequested(NavigationRequestedEventArgs e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yepp, is a good idea.
Do you think we should use the NavigationRequestedEventArgs
in Xamarin.Forms.Core? Or just pass the Page and isAnimated properties in the virtual method?
I'm asking because the NavigationRequestedEventArgs
has this property:
public Task<bool> Task { get; set; }
but in the shell renderer we dont have a navigation task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think NavigationRequestedEventArgs makes sense
We use that property on the iOS renderers, it might be useful on UWP at some point
Sorry for the delay, i was really busy lately and didnt had time to finish the PR with the changes requested by @PureWeen I had a conversation with pureween the other day and he is going to finish this, but.... Let me know if you are too busy, i can try to finish it! |
7bf22d7
to
e3d6b5b
Compare
@PureWeen definetely i love the new changes you made to the renderers and the |
/azp run |
No pipelines are associated with this pull request. |
@GiampaoloGabba Awesome!! I kicked off the UI Tests. Once those pass I'll merge Thank you for all your work and focus on bringing better transitions to Forms. I'm really excited about what you are bringing to the ecosystem here!!! |
Description of Change
The
ShellSectionRenderer
for UWP doesn't expose any virtual method to override, closing a lot of possibilities expecially for plugin authors.This PR comes after a conversation on Twitter with @PureWeen and adds a bunch of
protected virtual
methods to the navigation process (splitting up theNavigateToContent
method in submethods).Also i also made the
GetTransitionInfo
method virtual so we can change the animation transition between pages.Please note that i also made virtual the
NavigateToContent
method, so we have the ability to to jump in the navigation process before the destination page has been created (or after with the new virtual methods).API Changes
Added:
protected void OnPopRequested(ShellNavigationSource source)
protected void OnPopToRootRequested(ShellNavigationSource source)
protected void OnPushRequested(ShellNavigationSource source)
protected void OnInsertRequested(ShellNavigationSource source, Page page)
protected void OnRemoveRequested(ShellNavigationSource source, Page page)
protected void OnShellSectionChanged(ShellNavigationSource source)
Changed:
protected virtual void NavigateToContent(ShellNavigationSource source, ShellContent shellContent, Page page, bool animate = true)
protected virtual NavigationTransitionInfo GetTransitionInfo(ShellNavigationSource navSource)
Platforms Affected
Behavioral/Visual Changes
None
Before/After Screenshots
Not applicable
Testing Procedure
I think the current tests are enough, i have just moved some code inside virtual methods, no need to special test cases.
PR Checklist