-
Notifications
You must be signed in to change notification settings - Fork 87
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
Screen based animation overrides #1901
Conversation
work and reworking it as AnimatedNavigationTransform
… to `Circuit` - Think this makes sense as `DefaultDecoration` is an `AnimatedNavDecoration`
- Changes `AnimatedNavigationTransform` to Screen specific transition overrides with `AnimatedScreenTransform` - Adds navigation call site transition support
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.
Looking really good! A few meta-level things before you merge
- Let's stick circuit's experimental annotations on these new APIs
- Let's add some bits to the CHANGELOG and site docs
And one for a later PR - what do you think of adding a shared element tutorial to the inbox tutorial? I think having it in the STAR sample is great as a reference but I wonder if it'd also be good to have a tutorial that walks one through writing one in a simpler case like that.
import kotlinx.collections.immutable.ImmutableMap | ||
|
||
/** | ||
* Animated navigation decoration is an implementation of [NavDecoration] that provides the |
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.
before you merge, let's maybe add a simple sample snippet to this kdoc as a reference?
// todo DI this | ||
.addAnimatedScreenTransforms( |
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.
Do you wanna handle this in this PR?
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.
Planning on doing so separately, given the size of this PR
screen: KClass<out Screen>, | ||
animatedNavigationTransform: AnimatedScreenTransform, |
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 always kind of envisioned this as taking two keys (current and destination), do you think that's not necessary with this API? Example case: maybe photo viewer can do a hero element transition from a pet detail to the photo viewer but not if it's from a deeplink from home?
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.
Think having the single screen for a general screen override of any transition is valuable. Inside that you can check against the initial/target states to set specific screen to screen transitions.
The API right now is definitely more verbose, so I could see a need to add some sugar on top of this to make it easier to do simpler transitions.
About
Building out a surface where navigation transitions can be overridden at navigation time based on the type of navigation event and the source/target screens.
Due to how shared element transitions work with
AnimatedContent
, we need some ability to completely replace the the transition spec before the target shared elements are loaded into the composition. Can look atPetDetailAnimatedScreenTransform
for an example of this.AnimatedScreenTransform
usageEnterTransition
, z-index, andSizeTransform
for aScreen
as it becomes the top screen or is replaced by anotherScreen
in the navigation stack.ExitTransition
for a screen that is no longer the top screen.AnimatedNavEvent
that triggered the transition.Transition Selection
transitionSpec
on the decorator to obtain aContentTransform
based on theAnimatedNavEvent
.AnimatedScreenTransform
overrides defined for the target screen and the initial screen.ContentTransform
. Screen-specific overrides take precedence over the base transform.Example
The example checks if the user is navigating to the
PetDetailScreen
from theHomeScreen
, with the information needed to tie together a shared element transition. If that is the case no transitions are used, otherwise it returns null to fallback to the default nav decorator transition.Demo
Notice how on the forward navigation the grid view is sliding out, on the pop
AndroidPredictiveBackNavDecorator
already removes the animation.before.mp4
after.mp4