Skip to content

Conversation

@gavrix
Copy link

@gavrix gavrix commented Jan 2, 2019

This is an implementation of proposal #17475.
I haven't been using ReactNative since original evaluation about a year ago, upon revisiting I see this issue is still around.

Issue:

When deleting a view which UI state (like transform) has been connected to AnimatedProps, said state is reset to defaults before running deleting animation (fade to transparent using opacity, by default). This makes it impossible to implement nice swipe-to-delete experience (when swipe is driven by native driver)

Proposal

To iterate what I described in #17475: instead of disconnected animated component from native view when component is unmounted, keep that connection at native side around until native view is fully purged from the hierarchy.

Concerns:

I'm not 100% sure this is a correct approach. May there be legitimate use cases when AnimatedProps has to be disconnected from native view while latter has to remain in the hierarchy? If anyone has broader context around this mechanism I'd like to discuss.

Changelog:

[iOS] [Changed] - AnimatedProps isn't disconnected from native view (by calling __disconnectAnimatedView) when component is unmounted on js side. Instead, runtime on native side is keeping track of such connections and disconnects when native view is fully removed from hierarchy.

Test Plan:

As a proof-of-concept please see gif provided in #17475.
I wanted to provide a proper integration test but couldn't wrap my head around how to best do it. Any help or hint on direction would much appreciated.

@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 Jan 2, 2019
@matthargett
Copy link
Contributor

@hramos it looks like this is dependent on the iOS snapshot test fix PR by @grabbou #22861, if we want CI to be green before merge.

@gavrix does Android not have this problem as well?

@hramos hramos removed the 🔶APIs label Jan 24, 2019
@gavrix
Copy link
Author

gavrix commented Feb 8, 2019

I came to my attention that Reanimated implements the same functionality as Animated and somehow doesn't have this issue. I am going to close this issue and consider Reanimated preferred animation framework.

@gavrix gavrix closed this Feb 8, 2019
@matthargett
Copy link
Contributor

That solution works for you, but should this PR still be merged to fix the underlying issue in RN? It seems so to me :)

@gavrix
Copy link
Author

gavrix commented Feb 14, 2019

It was an experiment and only supported on iOS and I was mainly asking for feedback and I found that issue is not reproducible with Reanimated (which is basically an Animated replacement)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API: Animated CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: iOS iOS applications.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants