Motionate exit of screens in Navigator
#35312
Closed
Conversation
This file contains hidden or 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
7 tasks
|
Size Change: +97 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
15 tasks
ciampo
reviewed
Oct 4, 2021
Contributor
There was a problem hiding this comment.
Thank you for taking the initiative and working on this improvement for the Navigator component, @stokesman !
The code in this PR introduces quite a few changes:
- It introduces a wrapper
divin theNavigationProviderwith a CSS Grid layout (which ensures that all children fill thatdivs entirely) - It changes the initial value for the
pathinternal state ofNavigatorProvider - It changes the path of the
motionimport - It refactors the logic around the initial focus on mount in
NavigatorScreen - It refactors how the motion's animation configuration is computed by extracting some of that logic outside of the
NavigationScreens render function - It wraps the contents of
NavigatorScreeninto an `AnimatedPresence component - It switches from an early return to a ternary operator in the
NavigatorScreenfor returningnull
Some of these changes may be slightly out of scope for this PR (points 2, 3, 4, 5, 7).
For the time being, I would like to prioritise landing #35214 and #35317 (or any alternative) before focusing on this PR.
Finally, there is also a chance that we may prefer to keep the components simpler and decide not to introduce exit animations.
ciampo
added
[Feature] UI Components
Contributor
Author
|
@ciampo, Thanks for having look at this. You nicely summarized the changes, which I certainly would have done before taking it out of draft or requesting a review.
Me too. I'd never intended for this to get focus before any related PR. Putting it up now instead of later was partly to bolster my suggestion #35214 (comment) that a wrapper could be of use.
I'm aware that could be the case. I think it's nice to have something to show to go along with a suggested feature. Even if it goes nowhere it's good practice for me and can help judging if the feature worthwhile. |
Contributor
Author
|
Even if we want to add the exit animation at this point in time it probably be without framer motion so I'm closing this out. |
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.
Alternative to #35311. Here the animation is driven by framer-motion instead of CSS.
How has this been tested?
Manually.
Screenshots
navigator-exit-motionate.mp4
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).