The needsCustomLayoutForChildren check is missing from the new React Native architecture #34120
Labels
Needs: Triage 🔍
Type: New Architecture
Issues and PRs related to new architecture (Fabric/Turbo Modules)
Comments
grahammendick
added
Needs: Triage 🔍
Type: New Architecture
roryabraham
pushed a commit
to Expensify/react-native
that referenced
this issue
Aug 17, 2022
…acebook#34254) Summary: Fixes facebook#34120 The new React Native architecture doesn't check `needsCustomLayoutForChildren` so it wrongly positions native views on Android. In facebook#34120 there are videos comparing the positioning of a native action view in the old and the new architecture. This PR passes the parent tag to the `updateLayout` method of the `SurfaceMountingManager`. The `SurfaceMountingManager` calls `needsCustomLayoutForChildren` on the parent view manager (copied the code from the `NativeViewHierarchyManager` in the old architecture). **NOTE** - I wasn't sure where to get the parent shadow view from so I've put in my best guesses where I could and left it as `{}` otherwise. ## Changelog [Android] [Fixed] - Migrate `needsCustomLayoutForChildren` check to the new architecture Pull Request resolved: facebook#34254 Test Plan: I checked the fix in the repro from facebook#34165. Here is a video of the action view closing using the native button that is now visible in the new architecture. https://user-images.githubusercontent.com/1761227/180607896-35bf477f-4552-4b8a-8e09-9e8c49122c0c.mov Reviewed By: cipolleschi Differential Revision: D38153924 Pulled By: javache fbshipit-source-id: e2c77fa70d725a33ce73fe4a615f6d884312580c
roryabraham
pushed a commit
to Expensify/react-native
that referenced
this issue
Aug 17, 2022
…acebook#34254) Summary: Fixes facebook#34120 The new React Native architecture doesn't check `needsCustomLayoutForChildren` so it wrongly positions native views on Android. In facebook#34120 there are videos comparing the positioning of a native action view in the old and the new architecture. This PR passes the parent tag to the `updateLayout` method of the `SurfaceMountingManager`. The `SurfaceMountingManager` calls `needsCustomLayoutForChildren` on the parent view manager (copied the code from the `NativeViewHierarchyManager` in the old architecture). **NOTE** - I wasn't sure where to get the parent shadow view from so I've put in my best guesses where I could and left it as `{}` otherwise. ## Changelog [Android] [Fixed] - Migrate `needsCustomLayoutForChildren` check to the new architecture Pull Request resolved: facebook#34254 Test Plan: I checked the fix in the repro from facebook#34165. Here is a video of the action view closing using the native button that is now visible in the new architecture. https://user-images.githubusercontent.com/1761227/180607896-35bf477f-4552-4b8a-8e09-9e8c49122c0c.mov Reviewed By: cipolleschi Differential Revision: D38153924 Pulled By: javache fbshipit-source-id: e2c77fa70d725a33ce73fe4a615f6d884312580c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
I've started updating my navigation library to work with the new architecture and I've run into a problem on Android. The new React Native architecture doesn't check
needsCustomLayoutForChildrenso it wrongly positions native views. You can see what I mean in the videos below. In the first video you can see the native close button is visible on the old architecture. In the second video the close button is missing because the new React Native architecture has positioned the action view in the wrong place.Version
0.68.0
Output of
npx react-native infoSystem:
OS: macOS 12.4
CPU: (8) arm64 Apple M1 Pro
Memory: 524.81 MB / 16.00 GB
Shell: 5.8.1 - /bin/zsh
Binaries:
Node: 17.7.1 - /opt/homebrew/bin/node
Yarn: 1.22.18 - /opt/homebrew/bin/yarn
npm: 8.5.2 - /opt/homebrew/bin/npm
Watchman: 2022.03.14.00 - /opt/homebrew/bin/watchman
Managers:
CocoaPods: 1.11.3 - /usr/local/bin/pod
SDKs:
iOS SDK:
Platforms: DriverKit 21.4, iOS 15.5, macOS 12.3, tvOS 15.4, watchOS 8.5
Android SDK:
API Levels: 29, 30, 31, 32
Build Tools: 29.0.2, 30.0.2, 31.0.0, 32.0.0, 32.1.0
System Images: android-30 | Intel x86 Atom_64, android-30 | Google Play ARM 64 v8a, android-32 | Google APIs ARM 64 v8a
Android NDK: Not Found
IDEs:
Android Studio: 2021.1 AI-211.7628.21.2111.8193401
Xcode: 13.4.1/13F100 - /usr/bin/xcodebuild
Languages:
Java: 11.0.11 - /usr/bin/javac
npmPackages:
@react-native-community/cli: Not Found
react: 17.0.2 => 17.0.2
react-native: 0.68.0-rc.4 => 0.68.0-rc.4
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found
Steps to reproduce
I've created a repository with a reproduction of the bug. The README in the repo contains the steps needed to recreate the problem. There are two apps in the repo, one built with the old architecture and the other built with the new architecture. You can see that the bug only happens on the new architecture. The only dependencies in the apps are the 3 packages that make up my navigation library.
You can see the close button appears on the old architecture
Screen.Recording.2022-06-25.at.21.29.40.mov
But the close button is missing on the new architecture because React Native has positioned the action view in the wrong place
Screen.Recording.2022-06-25.at.21.32.50.mov
Snack, code example, screenshot, or link to a repository
https://github.com/grahammendick/needs-custom-layout-for-children-bug
The text was updated successfully, but these errors were encountered: