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

[Android] [Shell] Add a virtual method to allow custom transitions between fragments #7197

Closed
GiampaoloGabba opened this issue Aug 17, 2019 · 9 comments

Comments

@GiampaoloGabba
Copy link
Contributor

Summary

Xamarin.Forms Shell doesnt appear to have a virtual method to modify the transaction, like SetupPageTransition in NavigationPage, in order to create a custom transition animation.

I'm developing a plugin to make shared transitions between pages. For now is working well with the "standard" NavigationPage and i would like to port it to Shell.

Inspecting the renderer code, i noticed that the fragment transaction is created and then committed, without any chance to modify it before the commit.
It would be very useful if we can have an overridable method SetupFragmentsTransition where we can modify the transaction before it gets committed.

API Changes

Instead of this:

if (animate)
	transaction.SetTransition((int)global::Android.App.FragmentTransit.EnterMask);

We should have something like:

if (animated)
	SetupFragmentsTransition(transaction, (int)global::Android.App.FragmentTransit.EnterMask);

And then:

protected virtual void SetupFragmentsTransition(FragmentTransaction transaction, int enterMask)
{
	transaction.SetTransition(enterMask);
}

Intended Use Case

Provide a way to setup custom fragment transitions with Shell.

@GiampaoloGabba
Copy link
Contributor Author

In case this proposal get accepted, i already have a pull request ready.

@jsuarezruiz
Copy link
Contributor

Could you check Shell on iOS to see if require changes too?

@GiampaoloGabba
Copy link
Contributor Author

No changes required for iOS, we have this virtual method:
protected virtual IShellItemTransition CreateShellItemTransition() where i can return my custom IShellItemTransition this should be enough

@PureWeen
Copy link
Contributor

@GiampaoloGabba Is just having the Fragment Transaction enough? I need to look a bit deeper into fragment transitions but it seems like having a reference to the PreviousFragment, Incoming Fragment, and the Transaction would be helpful

Maybe using an args parameter would be helpful so we could expand on it?

I don't think having the entermask as a parameter is useful is it? That'll just be something someone sets or doesn't set themselves as part of that method.

@GiampaoloGabba
Copy link
Contributor Author

GiampaoloGabba commented Aug 19, 2019

@PureWeen Yep, the entermask is not useful at all in the override. We coul just apply the default and then call the virtual method, so a user can override the transaction.

Regarding the transaction, we need for sure pass the operation (maybe a boolean indicating if we are going back). That is enough for "base" transitions to apply with: transaction.SetCustomAnimations or transaction.SetTransition

For more advanced transitions, would be very useful to have both fragments (Previous and Incoming) as you suggested, this would avoid to write boilerplate code to get them in the renderer (as i do now in my plugin).

I dont think we will ever need other parameters but... never say never :)

@jsuarezruiz
Copy link
Contributor

Looks nice. Could you add the changes in the Spec to cover also future cases? I think having both Fragments (previous and incoming one) as well as another parameter with information about the state of navigation (back, etc.) is enough.

@GiampaoloGabba
Copy link
Contributor Author

I'm still looking at this before finalize the spec.
I took a deeper look in the shell renderer system and found that there is this virtual method protected virtual void SetupAnimation(ShellNavigationSource navSource, FragmentTransaction t, Page page) in ShellItemRendererBase.cs

Maybe i didnt fully grasp how shell navigation works between ShellSections,ShellItems ecc... in Android, so i'll do some doublechecks before wasting your time!

@PureWeen
Copy link
Contributor

@GiampaoloGabba so those are tied to two different transition behaviors

there is this virtual method protected virtual void SetupAnimation(ShellNavigationSource navSource, FragmentTransaction t, Page page) in ShellItemRendererBase.cs

This is tied to Push/Pop navigation. So when you're on a ShellSection and you push a page onto the stack then this is the method that's called. Which sounds like what you want?

transaction.SetTransition((int)global::Android.App.FragmentTransit.EnterMask);

The code you are referencing originally is when you are switching ShellSections. So when the user clicks on a Flyout Item to go to a new ShellSection that's the navigation that's called.

That code also seems like it should be inside a virtual method though :-)

But this all circles back to my previous point as well
#7197 (comment)

Does FragmentTransaction t, Page page provide enough information to perform the full amount of animating that you would want to be able to do?

I'm still curious if those should take in an Args instead of just be a list of parameters.

@samhouts
Copy link
Member

Duplicate of #6033

@samhouts samhouts marked this as a duplicate of #6033 Feb 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants