fix(iOS): incorrect safe area on transparent modals using landscape orientation

[tboba]

Description

Normally, while opening a modal, each screen should preserve its safe area, depending on the screen orientation. Unfortunately, when user dismisses transparent modals, safe area of the origin screen breaks, resulting in the vertical safe area on landscape orientation.
Because of that, I investigated the logic behind the final decision of screen orientations and it looks like the problem was lying on supportedScreenOrientations method in RNSScreen.mm file.

1. First, the modal was being asked for its supported device orientation. Once it reached the supportedScreenOrientations method, it was asking for config of childVC. Because the childVC was nil and the orientation wasn't set in screen options, it was returning nil, resulting in returning all orientations without upside down.
2. After that, there was a time for looking onto child VCs of a screen behind the modal. Since it was presenting a modal, it was first checking for its last child - since it didn't provide any modal and it haven't got any children, the last child was also nil, resulting in returning the modal as an orientation of the screen.
3. Returning a modal is (probably) a bad idea here, since it does not have any screen orientation specified, resulting in returning nil as a screen orientation. This was probably resulting a bug with wrong safe area.

This PR changes this bad behavior by not returning lastViewController (which is a modal that is being presented from a screen) and going further for looking a config in child view controllers.
However, this behavior may lead to the further bugs we haven't discovered yet.

Changes

• Changed return statement in findChildVCForConfigAndTrait method.

Screenshots / GIFs

Before

8mb.video-sIp-hTxqFeUR.mp4

After

8mb.video-A3U-bQKlTe4h.mp4

Test code and steps to reproduce

You can check Test2008 in TestsExample and FabricTestExample in order to tests how transparent modals behave.

Checklist

• Included code example that can be used to test this change
• Ensured that CI passes

[tboba]

To be honest I'm still thinking if this !includingModals statement is still necessary. Let's look from the perspective of the logic - previously, when screen was providing a modal, this method was returning lastViewController when the modal could not find any further child VCs on its way. When the screen was providing a modal and includingModals was set to NO, the nil was being returned straight-forward.
Now, since we're going further when the modal can't find any child VCs, shouldn't we remove this if (we want to include modals in logic, not stop on them)? Or maybe the name includingModals is misleading and we should change it? What do you think? I tested already the app without this if and it looks like it just works (however, updating lastViewController to return last child VC may be sufficient), but maybe you've got some objections? Please let me know!

My suggestion:

• remove if with !includingModals
• replace further if with (modalOrChild != nil && includingModals)

[kkafar]

To be honest I'm still thinking if this !includingModals statement is still necessary.

I guess the comment below this if statement explains why do we return nil in such case.

I don't really understand the problem here, must admit, but if tests turned out positive we can go with current solution. I'm not decisive here, however.

[amadeus]

We've patched this fix into next week's iOS beta. Initial signs are positive that it's working, will see if any hidden issues come up during the week beta process.

Thanks for fixing this so quickly! 🙏

[kkafar]

Seems alright, thanks!

[kkafar]

To be honest I'm still thinking if this !includingModals statement is still necessary.

I guess the comment below this if statement explains why do we return nil in such case.

I don't really understand the problem here, must admit, but if tests turned out positive we can go with current solution. I'm not decisive here, however.