Skip to content

Fix natively driven animations not getting reset properly#29585

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

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

Conversation

@phamfoo

@phamfoo phamfoo commented Aug 7, 2020

Copy link
Copy Markdown
Contributor

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

analysis-bot commented Aug 7, 2020

Copy link
Copy Markdown
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

analysis-bot commented Aug 7, 2020

Copy link
Copy Markdown
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

phamfoo commented May 13, 2021

Copy link
Copy Markdown
Contributor Author

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