Skip to content

React to onUserDrivenAnimationEnded event in JS#45414

Closed
cipolleschi wants to merge 4 commits into
facebook:mainfrom
cipolleschi:export-D59681428
Closed

React to onUserDrivenAnimationEnded event in JS#45414
cipolleschi wants to merge 4 commits into
facebook:mainfrom
cipolleschi:export-D59681428

Conversation

@cipolleschi
Copy link
Copy Markdown
Contributor

Summary:
This change completes the fix for broken pressable when animations were applied to components with native driven animations.

When creating the AnimatedProps, if they are natively drive animation, we look for the AnimatedValue involved and we register a listener. This is needed to make sure that the NativeModule will send te updated value upon calling the update function.

Then, when observing the props lifecycle, it register a listener to the new OnUserAnimationEnded event, fired by the NativeAnimation module.

When the OnUserAnimationEnded event is fired, the AnimatedProps will update the props that depends on the user driven animation.

Changelog

[General][Fixed] - reallign the shadow tree and the native tree when the user finishes interacting with the app.

Differential Revision: D59681428

@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 labels Jul 12, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

…#45413)

Summary:
Pull Request resolved: facebook#45413

## Changes
Add an example in RNTester to test that pressability with NativeDrivers works properly.

## Context
The pressability handling is a bit peculiar.
We have to handle 3 main behaviors:
* `PressIn` -> `PressOut` => triggers the `onPress`
* `PressIn` -> move inside the rectangle -> `PressOut` => triggers the `onPress`
* `PressIn` -> move outside the rectangle -> `PressOut` => cancel `onPress`.

For the first case, we detect whether the press happens inside a component in the Native layer only. And everything works.

When a move is involved, we:
1. Detect the initial press in the Native layer
2. We move the coursor and we delegate the detection of whether we are inside of a rect or not to the JS layer
3. The JS layer asks the C++ layer about the layout and decide whether we are in case 2 (move but still inside the rect) or in case 3 (move but outside the rect).

The problem is that with `nativeDriver` and animations, the C++ layer doesn't know about where the receiver view actually is.
This results in issues like the one shown by [facebook#36504](facebook#36504), where the onMove is not handled correctly.

## Solution
The solution is to keep detecting whether we are in the receiver view or not in the Native layer and pass the receiver view position and size back to JS so that the JS layer don't have to jump to C++ to make this decision.

We decided to pass the frame information because the JS layer is adding some padding and configurations to the final rectangle and we don't want to lose those configurations.

## Changelog
[General][Added] - Add example in RNTester to show that pressability works properly with NativeDrivers

Differential Revision: D58182480

Reviewed By: sammy-SC
Summary:
Pull Request resolved: facebook#45382

This change is the first step to tr and solve the pressability's `onTouchMove` issue with animation driven natively in the New Architecture.

 The idea is to trigger a special event from native to let JS know that a scroll event has ended (`scrollViewDidEndDragging` or `scrollViewdidEndDecelerating`).

When this happens, we need to send an event to JS to let him know that it has to sync the Native Tree with the Shadow Tree.

Step 2 is to connect Native with JS

## Changelog:
[iOS][Added] - Send onScrollEnded event to NativeTurboAnimatedModule

Differential Revision: D59459989

Reviewed By: sammy-SC
Summary:
Pull Request resolved: facebook#45383

This is the second step required to fix the onTouchMove event in the new architecture.

In this change, we are retrieving the list of nodes that are connected by the animation, and we are sending an event to the nodes so that we can trigger the commit.

## Changelog
[iOS][Added] - retrieve the tags of the nodes connected by the animation and send them to JS

Differential Revision: D59524617
@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

Summary:
Pull Request resolved: facebook#45414

This change completes the fix for broken pressable when animations were applied to components with native driven animations.

When creating the AnimatedProps, if they are natively drive animation, we look for the AnimatedValue involved and we register a listener. This is needed to make sure that the NativeModule will send te updated value upon calling the `update` function.

Then, when observing the props lifecycle, it register a listener to the new `OnUserAnimationEnded` event, fired by the NativeAnimation module.

When the `OnUserAnimationEnded` event is fired, the AnimatedProps will update the props that depends on the user driven animation.

## Changelog
[General][Fixed] - reallign the shadow tree and the native tree when the user finishes interacting with the app.

Differential Revision: D59681428
@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@analysis-bot
Copy link
Copy Markdown

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 21,352,271 +1,175
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 24,548,410 +577
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: c7fab53
Branch: main

cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Jul 26, 2024
Summary:
Pull Request resolved: facebook#45414

This change completes the fix for broken pressable when animations were applied to components with native driven animations.

When creating the AnimatedProps, if they are natively drive animation, we look for the AnimatedValue involved and we register a listener. This is needed to make sure that the NativeModule will send te updated value upon calling the `update` function.

Then, when observing the props lifecycle, it register a listener to the new `OnUserAnimationEnded` event, fired by the NativeAnimation module.

When the `OnUserAnimationEnded` event is fired, the AnimatedProps will update the props that depends on the user driven animation.

## Changelog
[General][Fixed] - reallign the shadow tree and the native tree when the user finishes interacting with the app.

Differential Revision: D59681428
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jul 30, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in afa887b.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @cipolleschi in afa887b

When will my fix make it into a release? | How to file a pick request?

cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Jul 31, 2024
Summary:
Pull Request resolved: facebook#45838

Some Internal tests in the old architecture were failing after landing [facebook#45414](facebook#45414) because the `RCTNativeAnimatedModule` in the old architecture was not declaring the event.

This change fixes it by declaring the event that is never fired in the Old Architecture as it is not needed.

## Changelog
[iOS][Added] - Declare the `onUserDrivenAnimationEnded` in the old Architecture

Reviewed By: sammy-SC

Differential Revision: D60507812
facebook-github-bot pushed a commit that referenced this pull request Jul 31, 2024
Summary:
Pull Request resolved: #45838

Some Internal tests in the old architecture were failing after landing [#45414](#45414) because the `RCTNativeAnimatedModule` in the old architecture was not declaring the event.

This change fixes it by declaring the event that is never fired in the Old Architecture as it is not needed.

## Changelog
[iOS][Added] - Declare the `onUserDrivenAnimationEnded` in the old Architecture

Reviewed By: sammy-SC

Differential Revision: D60507812

fbshipit-source-id: eb12563c6551204bcf98f3a2001e1efcf84ef05e
cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Aug 12, 2024
Summary:
Pull Request resolved: facebook#45839

Pull Request resolved: facebook#45414

This change completes the fix for broken pressable when animations were applied to components with native driven animations.

When creating the AnimatedProps, if they are natively drive animation, we look for the AnimatedValue involved and we register a listener. This is needed to make sure that the NativeModule will send te updated value upon calling the `update` function.

Then, when observing the props lifecycle, it register a listener to the new `OnUserAnimationEnded` event, fired by the NativeAnimation module.

When the `OnUserAnimationEnded` event is fired, the AnimatedProps will update the props that depends on the user driven animation.

## Changelog
[General][Fixed] - reallign the shadow tree and the native tree when the user finishes interacting with the app.

Reviewed By: sammy-SC

Differential Revision: D60499583
cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Aug 12, 2024
Summary:
Pull Request resolved: facebook#45414

This change completes the fix for broken pressable when animations were applied to components with native driven animations.

When creating the AnimatedProps, if they are natively drive animation, we look for the AnimatedValue involved and we register a listener. This is needed to make sure that the NativeModule will send te updated value upon calling the `update` function.

Then, when observing the props lifecycle, it register a listener to the new `OnUserAnimationEnded` event, fired by the NativeAnimation module.

When the `OnUserAnimationEnded` event is fired, the AnimatedProps will update the props that depends on the user driven animation.

## Changelog
[General][Fixed] - reallign the shadow tree and the native tree when the user finishes interacting with the app.

Reviewed By: sammy-SC

Differential Revision: D60499583
cipolleschi added a commit to cipolleschi/react-native that referenced this pull request Aug 12, 2024
Summary:
Pull Request resolved: facebook#45837

Some Internal tests in the old architecture were failing after landing [facebook#45414](facebook#45414) because the `RCTNativeAnimatedModule` in the old architecture was not declaring the event.

This change fixes it by declaring the event that is never fired in the Old Architecture as it is not needed.

## Changelog
[iOS][Added] - Declare the `onUserDrivenAnimationEnded` in the old Architecture

Reviewed By: sammy-SC

Differential Revision: D60499584
facebook-github-bot pushed a commit that referenced this pull request Aug 12, 2024
Summary:
Pull Request resolved: #45839

Pull Request resolved: #45414

This change completes the fix for broken pressable when animations were applied to components with native driven animations.

When creating the AnimatedProps, if they are natively drive animation, we look for the AnimatedValue involved and we register a listener. This is needed to make sure that the NativeModule will send te updated value upon calling the `update` function.

Then, when observing the props lifecycle, it register a listener to the new `OnUserAnimationEnded` event, fired by the NativeAnimation module.

When the `OnUserAnimationEnded` event is fired, the AnimatedProps will update the props that depends on the user driven animation.

## Changelog
[General][Fixed] - reallign the shadow tree and the native tree when the user finishes interacting with the app.

Reviewed By: sammy-SC

Differential Revision: D60499583

fbshipit-source-id: 02d25e7ca31b91f4d6e4ec1654350e2d84117eda
facebook-github-bot pushed a commit that referenced this pull request Aug 12, 2024
Summary:
Pull Request resolved: #45837

Some Internal tests in the old architecture were failing after landing [#45414](#45414) because the `RCTNativeAnimatedModule` in the old architecture was not declaring the event.

This change fixes it by declaring the event that is never fired in the Old Architecture as it is not needed.

## Changelog
[iOS][Added] - Declare the `onUserDrivenAnimationEnded` in the old Architecture

Reviewed By: sammy-SC

Differential Revision: D60499584

fbshipit-source-id: 581a30a88dbd6d8d67078a11699157c55ed19e58
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.

4 participants