Skip to content

Fix natively driven animations not getting reset properly#29585

Closed
phamfoo wants to merge 1 commit intofacebook:masterfrom
phamfoo:fix-reset-animation
Closed

Fix natively driven animations not getting reset properly#29585
phamfoo wants to merge 1 commit intofacebook:masterfrom
phamfoo:fix-reset-animation

Conversation

@phamfoo
Copy link
Copy Markdown
Contributor

@phamfoo phamfoo commented Aug 7, 2020

Summary

Fixes #28517

Animated.loop needs to reset the animation after each iteration but currently, natively driven animations are not getting reset properly.

Changelog

[General] [Fixed] - Fix natively driven animations not getting reset properly

Test Plan

Tested within RNTester

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 7, 2020
@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Aug 7, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,209,941 56
android hermes armeabi-v7a 6,859,109 52
android hermes x86 7,644,577 60
android hermes x86_64 7,535,513 48
android jsc arm64-v8a 9,368,718 8
android jsc armeabi-v7a 9,010,018 -4
android jsc x86 9,231,446 0
android jsc x86_64 9,808,590 12

Base commit: fe79abb

@analysis-bot
Copy link
Copy Markdown

analysis-bot commented Aug 7, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: fe79abb

@yungsters
Copy link
Copy Markdown
Contributor

Looks good. Thanks for the contribution, @tienphaw.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@yungsters has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@yungsters merged this pull request in 129180c.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 13, 2021
@phamfoo
Copy link
Copy Markdown
Contributor Author

phamfoo commented May 13, 2021

Glad to see the PR finally getting merged, thanks @yungsters. BTW, is there anything I can do in my future PRs to get them reviewed faster?

@yungsters
Copy link
Copy Markdown
Contributor

I apologize on behalf of the team for the delays. We are currently trying out some new internal tooling to help review PRs more promptly.

For this specific PR, one thing that would have helped is creating an RNTester example demonstrating the fix. (I already have a pending internal PR to create an example for this specific one.)

facebook-github-bot pushed a commit that referenced this pull request May 13, 2021
Summary:
Creates a new `Animated.loop` example in RNTester that uses the native driver.

This example shows the precise problem fixed by #29585 (D28383538 (129180c)).

Changelog:
[Internal]

Reviewed By: kacieb

Differential Revision: D28406914

fbshipit-source-id: 63ea7799d4b8bef8b0c1caaa3daf514ca04b7ab1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Needs: React Native Team Attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Animation does not loop when native drivers are used

6 participants