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

Layout Animations for the New Architecture #6055

Merged
merged 85 commits into from
Jun 18, 2024

Conversation

bartlomiejbloniarz
Copy link
Contributor

@bartlomiejbloniarz bartlomiejbloniarz commented May 27, 2024

Summary

This PR brings layout animations to the New Architecture.

In the Old Architecture layout animations were implemented separately for iOS and Android. The new implementation leverages the MountingOverrideDelegate to intercept and animate layout changes right before they are sent to the platform. This way we can have a unified implementation across platforms (it should also work beyond Android and iOS).

MountingOverrideDelegate

The override delegate is called every time a new transaction is about to be mounted. It gives us full access to the mutations list and allows us to change them. This mechanism is used by RN, also in Layout Animations. It is important to note that this way we never commit a new ShadowTree - we only change the mutations that are sent to the platform. This means that those changes don't influence the layout of other views, which is consistent with the Old Arch implementation.

Limitations

  • configuration of entering animations is done using nativeID since I was unable to obtain the tag of a view before the animation should start
  • skipExiting does not work properly on android (I'm not sure why the componentWillUnmount method is called there earlier than on iOS)
  • globalOriginX and globalOriginY currently return the values of originX and originY - this part will probably require a native convertPoint function to be used
  • due to view flattening some exiting animations behave differently. The issue is explained in this PR.

Test plan

Go through the Layout Animations examples in the FabricExample app.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jun 14, 2024
Summary:
PR changing the single mountingOverrideDelegate to a vector of those, so other listeners can operate on the transaction. Used by `react-native-screens` in software-mansion/react-native-screens#2134 and `react-native-reanimated` in software-mansion/react-native-reanimated#6055.

Till now, only one listener could be added there, meaning that e.g. `Layout Animations` from `react-native`,  `Layout Animations` from `react-native-reanimated` and listening for `Screen` removal in `react-native-screens` could not operate at the same time.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[GENERAL] [FIXED] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[GENERAL] [FIXED] - Add option for multiple `mountingOverrideDelegates`

Pull Request resolved: #44927

Test Plan: The code of `LayoutAnimations` inside `react-native` should work the same since it will add just one listener then. For other cases, different libraries can read/mutate transactions.

Reviewed By: javache

Differential Revision: D58530278

Pulled By: sammy-SC

fbshipit-source-id: d6305963621000be11d51a50cffff64526cca934
Copy link
Member

@piaskowyk piaskowyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work 👏👏👏

For me, this is ready to merge. Let users test it in real-life scenarios, and we can gather feedback accordingly.

@bartlomiejbloniarz bartlomiejbloniarz added this pull request to the merge queue Jun 18, 2024
Merged via the queue into main with commit 993491e Jun 18, 2024
14 checks passed
@bartlomiejbloniarz bartlomiejbloniarz deleted the @bartlomiejbloniarz/fabric-layout-animations branch June 18, 2024 08:38
@mrousavy
Copy link
Contributor

Nice work guys 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants