The needsCustomLayoutForChildren check is missing from the new React Native architecture

[grahammendick]

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 needsCustomLayoutForChildren so 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.

Reproduction of the problem

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

[grahammendick]

Hi @sammy-SC and @cipolleschi I’ve been working on migrating my navigation library to the new architecture for about 6 months now and this bug is the final blocker! So I’ve freed up some time to work on the PR. But I’m going to need some guidance.

There’s a TODO in the SurfaceMountingManager that says

TODO Check if the parent of the view has to layout the view, or the child has to lay itself out. see NativeViewHierarchyManager.updateLayout

So the call to needsCustomLayoutForChildren is missing from the SurfaceMountingManager. I've checked that adding this call would fix the bug shown in the videos above! But I don’t know how to add this call in for real because I can’t access the parent ViewManager without the parentTag. The parentTag isn't available in the new architecture like it is in the old architecture

[grahammendick]

Hi @sammy-SC and @cipolleschi I've created a PR that is my first attempt at migrating needsCustomLayoutForChildren to the new architecture . It's not working but I'm hoping you can help me fix it.

The PR passes the parent tag to the updateLayout method of the SurfaceMountingManager. This works the first time the screen loads but the parent tag is 0 after I click the 'Hello' button (I'm using the repro fron this discussion to test it).

[grahammendick]

Hi @sammy-SC and @cipolleschi I've made a bit more progress with the PR. The PR now passes the parent tag both first time and after I click the 'Hello' button. Here's the video of the bug fix running in the new architecture. You can see the action view is now positioned correctly so that the close button is visible.

The PR still needs work because I couldn't work out where to get the parent shadow view from in most cases. Please can you help me to get the PR into a mergeable state?

Screen.Recording.2022-07-23.at.14.39.10.mov

[cipolleschi]

Hi @grahammendick, thanks for working on this.
Unfortunately, I don't have a lot of context and knowledge about how these things work on Android.
@cortinico, could you help here, even by just tagging the proper person?

[grahammendick]

It's Ok it's already been merged in! Thanks anyway though 👍