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

Animated.View::onPanResponderMove Stops Updating Animated.Value When Child Element Is Deleted #45126

Closed
kasperski95 opened this issue Jun 24, 2024 · 2 comments
Labels
API: Animated API: PanResponder Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Resolution: Fixed A PR that fixes this issue has been merged. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@kasperski95
Copy link

kasperski95 commented Jun 24, 2024

Description

When a child element inside an Animated.View is deleted, the onPanResponderMove stops updating an Animated.Value, which stops the dragging animation. This issue doesn't exist on old architecture.

Steps to reproduce

  1. Clone the included reproducer.
  2. Install project dependencies with yarn.
  3. Run the application on Android.
  4. Drag element on the screen.
  5. Notice that dragging is interrupted on new architecture when a child element is removed.

React Native Version

0.74.2

Affected Platforms

Runtime - Android, Other (please specify)

Areas

Fabric - The New Renderer

Output of npx react-native info

System:
  OS: macOS 14.4.1
  CPU: (8) arm64 Apple M2
  Memory: 161.36 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.19.0
    path: ~/.asdf/installs/nodejs/18.19.0/bin/node
  Yarn:
    version: 3.6.4
    path: ~/.asdf/installs/nodejs/18.19.0/.npm/bin/yarn
  npm:
    version: 10.2.5
    path: ~/.asdf/plugins/nodejs/shims/npm
  Watchman:
    version: 2023.08.28.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.13.0
    path: /usr/local/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK:
    API Levels:
      - "33"
      - "34"
    Build Tools:
      - 30.0.3
      - 33.0.0
      - 34.0.0
    System Images:
      - android-33 | Android TV ARM 64 v8a
      - android-33 | Android TV Intel x86 Atom
      - android-33 | Google TV ARM 64 v8a
      - android-33 | Google TV Intel x86 Atom
      - android-33 | Google APIs ARM 64 v8a
      - android-33 | Google APIs Intel x86_64 Atom
      - android-33 | Google Play ARM 64 v8a
      - android-33 | Google Play Intel x86_64 Atom
    Android NDK: Not Found
IDEs:
  Android Studio: 2022.3 AI-223.8836.35.2231.10671973
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /usr/bin/javac
  Ruby:
    version: 2.6.10
    path: /usr/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.2
    wanted: 0.74.2
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: Not found
  newArchEnabled: false

Stacktrace or Logs

This issue isn't a crash or failure.

Reproducer

https://github.com/kasperski95/repro-rn-na-pan-responder-issue

Screenshots and Videos

New Architecture

NEW_ARCHITECTURE.mov

Old Architecture (expected behavior)

OLD_ARCHITECTURE.mov
@kasperski95 kasperski95 added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Jun 24, 2024
@cortinico cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Triage 🔍 labels Jun 24, 2024
@cortinico
Copy link
Contributor

Thanks for reporting this @kasperski95

Just out of curiosity: have you experienced this isssue from using a library (if so, which one) or directly inside your app?

@cortinico
Copy link
Contributor

@kasperski95 out of curiosity, is this issue happening also on iOS?

cortinico added a commit to cortinico/react-native that referenced this issue Aug 1, 2024
Summary:
This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
…ure (facebook#45865)

Summary:
Pull Request resolved: facebook#45865

This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610
Closes facebook#45675

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
…ure (facebook#45865)

Summary:
Pull Request resolved: facebook#45865

This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610
Closes facebook#45675

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
Summary:
This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
…ate the Pressable fix

Summary:
This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the
fix for bug facebook#45126 and facebook#44610.

Changelog:
[Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix

Differential Revision: D60908117
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
Summary:
This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
…ate the Pressable fix (facebook#45930)

Summary:
Pull Request resolved: facebook#45930

This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the
fix for bug facebook#45126 and facebook#44610.

Changelog:
[Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix

Reviewed By: mdvacca

Differential Revision: D60908117
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
Summary:
This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 7, 2024
…ate the Pressable fix (facebook#45930)

Summary:
Pull Request resolved: facebook#45930

This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the
fix for bug facebook#45126 and facebook#44610.

Changelog:
[Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix

Differential Revision: D60908117
cortinico added a commit to cortinico/react-native that referenced this issue Aug 8, 2024
…ure (facebook#45865)

Summary:
Pull Request resolved: facebook#45865

This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610
Closes facebook#45675

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Reviewed By: mdvacca

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 8, 2024
…ure (facebook#45865)

Summary:
Pull Request resolved: facebook#45865

This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610
Closes facebook#45675

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Reviewed By: mdvacca

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 8, 2024
Summary:
This diff introduces the logic to defer the destruction of ViewState (and EventEmitter) for views that are currently touched on by the user. The idea is to let the UIManager know which view is currently active from the `JSTouchDispatcher` and eventually defer the view deletion till the view is not interacted anymore.

The JSTouchDispatcher already retains the information on which tag was the touch originally fired.
We'll pass over that information to the UIManager/SurfaceMountingManager so that it can be accounted for when the view has to be deleted.

This is causing a couple of bad bugs on Android:

Fixes facebook#45126
Fixes facebook#44610

Changelog:
[Android] [Fixed] - Do not destroy views when there is a touch going on for New Architecture

Differential Revision: D60594878
cortinico added a commit to cortinico/react-native that referenced this issue Aug 8, 2024
…ate the Pressable fix (facebook#45930)

Summary:
Pull Request resolved: facebook#45930

This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the
fix for bug facebook#45126 and facebook#44610.

Changelog:
[Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix

Reviewed By: mdvacca

Differential Revision: D60908117
facebook-github-bot pushed a commit that referenced this issue Aug 8, 2024
…ate the Pressable fix (#45930)

Summary:
Pull Request resolved: #45930

This introduces the `enableEventEmitterRetentionDuringGesturesOnAndroid` that allows us to gate the
fix for bug #45126 and #44610.

Changelog:
[Internal] [Changed] - Introduce the enableEventEmitterRetentionDuringGesturesOnAndroid to gate the Pressable fix

Reviewed By: mdvacca

Differential Revision: D60908117

fbshipit-source-id: 885917832718d9b90d043b2d7e2cdb47e0f01ea7
@cortinico cortinico added the Resolution: Fixed A PR that fixes this issue has been merged. label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Animated API: PanResponder Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Resolution: Fixed A PR that fixes this issue has been merged. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants