Supported Web Animations Api for navigation animations (Mobile) #808
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't have to track progress anymore. Only want to know what state the scenes are in so can run the appropriate keyframe animation - pushEnter, pushExit, popEnter, popExit. Took these names from android - when pushing a new scene then one scene enters another exits and same when popping. Plan is to then play/reverse the web animation api on the scenes and see what happens
They are showing up - but none of them are animating or even being removed at the moment
Only want to include those that are no longer there for this part
Check the flags and run the animations - either play or reverse depending on whether enter or exit. Set fill mode to forwards so that animation runs from where it left off. For example, if use arrows to flick between history back and forward the animation to play then reverse smoothly
The four are pushEnter, pushExit, popEnter and popExit
Same idea but needed index to tell if it's pushExit or popEnter. If the scene was at the top and now isn't then it's pushExit. If it wasn't at the top but now is then it's popEnter
Testing in the people/person master/details sample. Tried back/forward arrows when details came in and it didn't work. It wasn't correctly setting the nav state. It didn't think the details was pushEnter because it there was a scene before - it was mid popExit so the scene was there. It was only setting pushEnter when the scene wasn't there before. Also tracked the current navigation state on the scene element because can't afford to play the animation twice. When removed the popExit scene it updated scenes and reevaluated animation - this replayed the same animation so it ran twice. So tracking to avoid this.
The animation starts from either unmounted or crumb for consistency. Don't really understand why need to change fill to backwards here?
Not the same as before - not rendering on animation anymore so can't calculate rest from the scenes. Instead put it back to what it used to be and set it to true when the mounted scene rests. This matches native as well - don't need to wait for all scenes to rest. Only freeze crumb scenes anyway and if they're still animating then they're hidden anyway so should be fine. I guess if it's not a stack - some other custom navigation on mobile - then maybe still can see the crumbs? Had to prevent onRest being called in infinite loop - so used the navState for that. Cleared it when they come to rest
So load new url pointing at details. Then the master scene starts in crumb. Need pushExit defined when popEnter runs first before pushExit
In twitter example only the 1st back worked. After that nothing happened. Needed to persist the animation otherwise the original pushEnter animation was removed when it was replaced by the pushExit (wrongly named - it should be popEnter). Also only cleared navState if the animation ran - the finished promise always resolves if no animation so navState was being cleared when actually different animation was running
It was pushExit when it was going to the crumb. But the animation is now coming from the crumb (to match the unmount) so it should be popEnter
Finished the push enter immediately if there's no old state
A bit early to be fixing unit tests but wanted to check the approach is sound. Added id={key} to scenes to get the tests passing - will think what to really do later. Excluded the exiting scenes immediately because not animating in unit tests. Had to fire onRest otherwise tests failed where was expecting the crumb scene to be suspended (no state update).
Tweaked a couple of tests so that the useLayoutEffect fired - separated into separate acts. Think the old way of running requestAnimationFrame in a row meant it wasn't necessary. Deleted all the requestAnimationFrame because that's not there anymore.
Might end up keeping old NavigationMotion and creating NavigationStack for backward compatibility - so might need to bring back the old test approach
More consistent with the real code so prefer it
This is continuous overlapping navigations. For example, A -> B -> C -> D then keeping finger on CMD + back arrow and scene B needs to do popEnter and popExit - so want the popEnter animation to complete and then the pushEnter reverse to run. So moved pushEnter logic inside the popExit finished promise - and vice versa. So pushEnter waits on popEnter and vice versa. This was something couldn't get working properly with the old approach - because B would go from crumb to exit in a single duration. The old Motion would just move between target and destination so had no notion of intermediate states
Just implemented and tested on twitter sample
Navigate A -> B -> C then back twice and forward once while the back is still in flight. This would go wrong - it would run the popEnter animation in reverse for B even though the popExit was still running for B. The popEnter only needs to run if the pushExit was the previous state - so only if it's opposite had run. If it hasn't then there's nothing to reverse so just finish it so it shows immediately. And vice versa - doing a double forwards in history from A then history back while the forward is still in flight. Made the two paths symmetrical now - if there's a previous state then only reverse if the previous state is the opposite. It will be if not running multiple animations at once. If wait for them to finish then it will be it's opposite. For example, popEnter follows pushExit - that's navigate forward followed by back. But if it's not then finish the animation so it shows immediately
The obj value is only when want separate duration per scene
Navigate from A to A -> B -> C then go back and it went wrong. Think B showed but without animation and then back again didn't do anything. Marked all new scenes as pushEnter because when fluently navigating both B and C are pushEnter. Before B was getting any animation props at all - it had mounted false, wasMounted false and no scene so it was fallling through and getting no animation props. Marking it as pushEnter fixed this. Also hid a Scene if it doesn't have navigationEvent - this is fluently navigated scene. Without this it was showing blank - only shows if it has different animation to the actually mounted scene. So gave timeline slide up and tweet slide in and fluently navigated over timeline to tweet. Then could see blank timeline scene sliding up
Copied it over incorrectly. The key is already on the Scene in NavigationMotion
Navigate from A -> B to A -> C then during the animation go browser back and forward. This showed a new scene animating in instead of reversing C back out. That's because it was given a different key - it had key 1+ because of replace but then going back and forward gave C key of 1 - because not replacing this time. So changed the way the keys are built - used the state key names instead - a scene's key is all the crumb state's and current state's keys. So key looks like A-C for scene C both times. Then it works. Have to smarten up the keys though because there could be state called A and A- and C and -C. Then could get same key names A--C for 2 different paths
Because the stack does all the rendering now, unlike motion, need a prop to set className (also need prop for style, will do later)
Gonna duplicate all the static and dynamic tests for the stack
Duplicating static and dynamic in each test
These are different tests for static and dynamic so created 2 new tests for static and dynamic stack. Renamed existing 2 to NavigationMotion for clarity
A -> B -> C then go back 2 (used browser history drop down but can to navigateBack(2)) and while that's animating go forward once in history (back to B). This used to go blank, just black background. That's because B didn't go through the popEnter - it went straight from pushExit to popExit so couldn't play pushEnter because it hadn't been reversed yet. So changed to take it through popEnter first. It will then go to popExit when the first popExit completes because then it rerenders
Previous change means A -> B -> C and then navigatingBack 2 needs multiple renders to get rid of B. Now B first goes to popEnter and then popExit. So 2 renders when before it went straight to popExit. Had to change because going straight from pushExit to popExit skipped animation so went blank when forward in history during the animation. So fixed the test mock animation to call onRest for popEnter to fire that extra render
Must've copied by accident when duplicating for stack tests
Either remove the animation props or set the duration to 0. At the scene level can't remove the animation props because they'll use the ones on the stack then. So either set duration to 0 or set the props to {} to turn off animation for an individual scene
The unmountStyle can be an array of object with keyframes array on it
The scene wasn't scrolling. Plus this matches [the documentation](https://grahammendick.github.io/navigation/documentation/mobile/setup.html)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the
NavigationStackcomponent that uses the Web Animations Api for navigation animations. TheunmountStyleandcrumbStyleprops accept the keyframes array. Here's what the props look like for the Twitter example.Using the Web Animations Api is better than the
NavigationMotionapproach because it doesn’t require React state updates to run the animation. It should play nicely with the dev tools Animations tab. Also it doesn't need arenderMotionprop anymore so native and web components are more aligned.Considered View Transitions Api but it's not up to the job. Only the entering scene is a live view so the user couldn't run a custom animation in the exiting scene. Also they don't all play smoothly together if the user presses multiple browser backs at once. For example, A → B → C then cmd + ← twice should play B and C exiting together. If C takes 0.5s to exit then it must still take 0.5s even if 'interrupted' with that second browser back. View transitions don't do that. C would disappear as soon as the second browser back is pressed.