-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Shared Element Transition] Animation restart #4043
Conversation
android/src/main/java/com/swmansion/reanimated/layoutReanimation/Snapshot.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/reanimated/layoutReanimation/SharedTransitionManager.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/reanimated/layoutReanimation/SharedTransitionManager.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/reanimated/layoutReanimation/SharedTransitionManager.java
Outdated
Show resolved
Hide resolved
@@ -53,7 +53,8 @@ export class SharedTransition implements ILayoutAnimationBuilder { | |||
const keyToTargetValue = | |||
'target' + propName.charAt(0).toUpperCase() + propName.slice(1); | |||
animations[propName] = withTiming(values[keyToTargetValue], { | |||
duration: 1000, | |||
// native screen transition takes around 500ms | |||
duration: 500, |
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 think it's 350 ms to be exact (source) but 500 ms is okay as well
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 set 500 ms because, in my subjective opinion, the transition with a 500 ms duration looks much better than animation with a 350 ms duration, but like I said. This is my subjective opinion. I think we should allow for users to determine the default transition duration. We should do it in another 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.
Looks good to me, haven't tested it in the Example app though.
In #4043 I changed the restarting animation logic in `animationManager.ts`. Accidentally I removed restarting animation for the rest of the Layout Animations. Fortunately, I found a better way to handle restart animation for both LA and SET animations types.
## Summary This change allows restarting of animation in two cases: - when someone changes the current screen during transition animation - when an update of layout comes for shared view during the transition animation **Known limitation**: If the parent of shared view updates owns the layout during the shared transition it doesn't affect for shared view layout. This issue will be fixed in the future.
In software-mansion#4043 I changed the restarting animation logic in `animationManager.ts`. Accidentally I removed restarting animation for the rest of the Layout Animations. Fortunately, I found a better way to handle restart animation for both LA and SET animations types.
Summary
This change allows restarting of animation in two cases:
Known limitation: If the parent of shared view updates owns the layout during the shared transition it doesn't affect for shared view layout. This issue will be fixed in the future.