Skip to content
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

Trigger rerender on animation complete #37836

Closed
wants to merge 1 commit into from

Conversation

genkikondo
Copy link
Contributor

Summary:
When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which triggers a rerender (via setState) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Differential Revision: D46655246

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Jun 12, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Differential Revision: D46655246

fbshipit-source-id: 7e4cfc1be6b54efd24a51a68f2a3b95ba444ee66
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: 7d67f4f9eef3c2f37019c1ba7d70fac2b9905e5d
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: 1d4e7006d5e02fa730da22e784c9bb80a49809a9
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: 41e1301fe5e7709388d08655bc158c57d54e38d9
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: cf60e55859ce67750ad156e90003c952e4f73953
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: 64ee82b027eb4cccacb993e0f3aec7bc9e939e1a
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: 44462a5f868fdba1d9b09202a46ecefc23bc2082
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

genkikondo added a commit to genkikondo/react-native that referenced this pull request Jun 13, 2023
Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: 06a679e1e17177168da70e3852d4b3484010efee
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

Summary:
Pull Request resolved: facebook#37836

When using the native driver for animations that involve layout changes (ie. translateY and other transforms, but not styles such as opacity), because it bypasses Fabric, the new coordinates are not updated so the Pressability responder region/tap target is incorrect.

Prior diffs ensure that upon completion of natively driven animations, the final values are synced to the JS side AnimatedValue nodes. In this diff, on completion of a natively driven animation, AnimatedProps.update() is called, which in turn calls the value update callback on AnimatedProps, which [triggers a rerender (via setState)](https://www.internalfb.com/code/fbsource/[566daad5db45807260a8af1f85385ca86aebf894]/xplat/js/react-native-github/packages/react-native/Libraries/Animated/useAnimatedProps.js?lines=80) which has the effect of pushing the latest animated values to Fabric.

Alternative considered was using setNativeProps, but that approach was dropped as setNativeProps was only introduced to make migration easier and should not be used for new code, as per sammy-SC.

Changelog:
[General][Fixed] - When animating using native driver, trigger rerender on animation completion in order to update Pressability responder regions

Reviewed By: javache

Differential Revision: D46655246

fbshipit-source-id: b19054ef10f4e0f3c8ca9c50ec6652a3de555bc3
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D46655246

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 14, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in c870a52.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants