-
Notifications
You must be signed in to change notification settings - Fork 41
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
Replaced React Motion with React Move #126
Merged
Merged
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
React move doesn’t work on React Native. Had to: - Remove babel section from package json of react-move - Removed timestamp || from https://github.com/tannerlinsley/react-move/blob/master/src/Transition.j s#L220
When shared elements came in, it brought in context. Turns out ReactReconciler always re-renders when context is there. Before context it the reconciler short circuited the update because the scenes where === identical. So, cached the url in constructor and added a shouldComponentUpdate to avoid re-rendering crumb scenes
React Motion calls it per item, React Move calls it for all together. Prefer React Move, but means have to clear out possible multiple scenes in one go
Tricky bit was setting immutable to false on Animate component. If don’t set this then every time the SharedElementMotion sets State the Animate updates which resets the timer progress. So a 300ms animation takes longer because it resets 300 multiple times. Setting immutable makes it compare the data props on values instead of ===. Because the shared elements are going to the same place each update, the data’s the same so it doesn’t update and so doesn’t reset the timer
Added missing onRest prop
React motion didn’t animate the first screen but react move does, so changed home unmounted style to match mounted
This reverts commit a4f2b09.
Moved the shouldComponentUpdate checks to the top level components (the ones from renderScene), because then it’s safe for the descendant components to get the url in the render (not the constructor).
Lowered it to 0 but it didn’t work when duration went back to 300. Changing 0 to 5 didn’t work, but 50 did. Not sure why, but might investigate react move later
React motion used to take care of this, but react move doesn’t
Missed out during refactor
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.
React Motion animations aren't symmetric. Leave animates differently to enter, for example, because it uses a spring. Switched to React Move with a linear easing.
Doing away with springs makes the api cleaner. React Move’s interpolation is implicit so can return plain data in style props.
Can also set an animation duration with React Move. Important for animations to end at a fixed time. Especially with shared element motion because the Modal stops screen interaction until it's removed.