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 regression when hosting react-native in a bottom sheet dialog in 0.71 #38473

Closed
rasaha91 opened this issue Jul 17, 2023 · 2 comments
Closed
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Needs: Triage 🔍 p: Microsoft Partner: Microsoft Partner

Comments

@rasaha91
Copy link

Description

We have a brownfield app that hosts react-native in a bottom sheet dialog. Prior to 0.71, the bottom sheet dialog would be sized appropriately, matching the size of the contents of the react-native instance. After 0.71, we are seeing that the bottom sheet dialog is sized much larger. To reproduce the issue, you can clone https://github.com/rasaha91/rn-bottomsheet and follow the instructions in the readme (make sure you switch to an appropriate branch first). Follow the instructions for 0.70 and then for 0.71 to see the regression.

This repo basically follows the steps located here https://reactnative.dev/docs/0.71/integration-with-existing-apps, following the respective instructions for each major version. I've also made sure to use the versions of gradle and AGP that RN uses for each major version as well.

Here are the results if you follow the instructions:

0.70:

image

0.71:

image

The layout issue is also present in 0.72, which you can verify by switching to the 0.72 branch in the repo.

One observation that I was able to make is that the native layout tree is different between 0.70 and 0.71. There appears to be an extra ReactViewGroup

0.70:

image

0.71:

image

React Native Version

0.71.12

Output of npx react-native info

System:
OS: Windows 10 10.0.22621
CPU: (32) x64 AMD Ryzen Threadripper PRO 3955WX 16-Cores
Memory: 92.13 GB / 127.86 GB
Binaries:
Node: 16.14.2 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.15 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
npm: 8.5.0 - C:\Program Files\nodejs\npm.CMD
Watchman: Not Found
SDKs:
Android SDK: Not Found
Windows SDK:
AllowDevelopmentWithoutDevLicense: Enabled
AllowAllTrustedApps: Enabled
Versions: 10.0.19041.0, 10.0.22000.0, 10.0.22621.0
IDEs:
Android Studio: Not Found
Visual Studio: 17.6.33815.320 (Visual Studio Enterprise 2022), 16.11.33801.447 (Visual Studio Enterprise 2019)
Languages:
Java: 11.0.16.1 - C:\Program Files\Microsoft\jdk-11.0.16.101-hotspot\bin\javac.EXE
npmPackages:
@react-native-community/cli: Not Found
react: 18.2.0 => 18.2.0
react-native: 0.71.12 => 0.71.12
react-native-windows: Not Found
npmGlobalPackages:
react-native: Not Found

Steps to reproduce

https://github.com/rasaha91/rn-bottomsheet

Follow the instructions on the readme, trying it on the 0.70 branch first, then trying it on the 0.71 branch to see the regression.

Snack, code example, screenshot, or link to a repository

https://github.com/rasaha91/rn-bottomsheet

Follow the instructions on the readme, trying it on the 0.70 branch first, then trying it on the 0.71 branch to see the regression.

@github-actions github-actions bot added the Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. label Jul 17, 2023
@rasaha91
Copy link
Author

rasaha91 commented Jul 17, 2023

This appears to be the same issue seen in #38024 @cortinico as FYI since you might have taken a look at it based on the comments in that issue. 0.71.11 had a regression with extra ReactViewGroups, and a fix was cherry-picked with 2b06a75 which I believe made it into 0.71.12. Comparing 0.71.11 and 0.71.12, I saw a simplified native layout tree (one less level of ReactViewGroup). You can switch to the 0.71.11 branch in the repo linked above to test it out. Switch to the 0.71 branch to test out 0.71.12.

0.71.11:

image

0.71.12:

image

When it was working in 0.70:

image

There is still one extra ReactViewGroup that is showing up in 0.71.12 compared to 0.70.6, that is causing the layout to be different. So it seems that the cherry-pick provided a partial fix to this problem by reducing the number of extraneous ReactViewGroups, but didn't fix the issue entirely.

This issue of extra ReactViewGroups is also seen in the duplicate issue linked above:

0.70.6:

image

0.71.11:

image

There are several top level ReactViewGroups in the tree, under the ReactRootView. My guess is if that UI was recreated with 0.71.12, you would see almost the same tree as 0.70.6, but with just a single ReactViewGroup wrapping the children.

@rasaha91
Copy link
Author

The majority of the discussion surrounding this issue is taking place in #38024 where we have concluded this is dev only and is due to a regression due to devtools overlay. Closing this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Needs: Triage 🔍 p: Microsoft Partner: Microsoft Partner
Projects
None yet
Development

No branches or pull requests

2 participants