fix(Android): update status bar & orientation in screen stack fragment

[delphinebugner]

Description

I found this working on my app going in portrait mode on Android quite aleatory, when it was supposed to be locked on React Navigation - using the orientation: "portrait" on my screens.

(Note : orientation was not locked on the AndroidManifest file, I actually need the landscape mode elsewhere in the app.)

What is the fix?

Sometimes when the props orientation is set, the fragment and its activity associated are not yet available. So doing a setOrientation in the updateProps function does not work.
They can also be set directly on the onUpdate function of the ScreenFragment itself, using the trySetWindowTrait function and the main activity; but the "super" allowing this behavior to the ScreenStackFragment (=the ones that I use) were not here.

A similar fix is also done in the ScreenStackHeaderConfig onUpdate function. But in my case, I did not have a headerConfig: so the trySetWindowTrait was not called at all ... and my app could go landscape just after the opening.

After adding the super, bug was fixed and my app was well locked in portrait mode 👌 👌

Screenshots / GIFs

Here is the scheme that I made during my debugging session:

Here is just for fun my app going in portrait mode when it was not suppose to go:

Test code and steps to reproduce

Checklist

• Included code example that can be used to test this change
• Updated TS types -> no need I think
• Ensured that CI passes

[delphinebugner]

For apps having a headerConfig in their ScreenStackFragment, this results in the trySetWindowTrait being called twice - once in the super of ScreenStackFragment.onUpdate, once in the ScreenStackHeaderConfig.onUpdate (also called in the ScreenStackFragment.onUpdate) ; I am not sure it's a bad thing, but I do want to have your opinion on this!

Removing it from ScreenStackHeaderConfig.onUpdate can't be done I think, because this function is called elsewhere - probably in a situation where the fix was needed!

[tboba]

Hi @delphinebugner, thanks for submitting the PR! Really appreciate the time you've spent on debugging the lifecycle of the screens ❤️

I'll try to check the changes in upcoming days and will come back with the update 🤞

also,

For apps having a headerConfig in their ScreenStackFragment, this results in the trySetWindowTrait being called twice - once in the super of ScreenStackFragment.onUpdate, once in the ScreenStackHeaderConfig.onUpdate (also called in the ScreenStackFragment.onUpdate) ; I am not sure it's a bad thing, but I do want to have your opinion on this!

Yeah, I believe I understand what do you mean about that - when I was implementing #1874, I've spotted an issue where onCreateView was called twice. First, it was called inside ScreenStackFragment, then onContainerUpdate() was called which also led to call headerConfig.onUpdate(). After that another onCreateView() from ScreenStackFragment was called with another call to onContainerUpdate -> headerConfig.onUpdate() but in that case ScreenStackHeaderConfig was not initialized because it just returned on the !mIsAttachedToWindow || !isTop || mDestroyed condition.

This is quite problematic for us, because you can't just trust that top app bar will be created properly (at least in the mentioned above PR, where you need to provide additional native components, like AppBarLayout, CollapsingToolbarLayout and MaterialToolbar).

I think this may be related to your case of double calling onUpdate method, but I think this is a React Native bug, rather than react-native-screens issue, as I've also observed same thing on the iOS (maybe on the debug mode the views are being created twice?).
Nonetheless, this is still worth to investigate 🤔

[tboba]

At first glance it looks good to me, haven't spotted any bugs there! But I may just have wrong repro 😄 @delphinebugner could you attach your test application to this PR (or any snack / link to repo) so I could check if this change fixes your problem?

[delphinebugner]

The change fixes my problem, I patched it locally already! But my app is on a private (enterprise) repository, I can't share the code with you 😥
I can try to reproduce the setup in a snack, even if I'm not sure to be able to reproduce the bug - it's kind of a race condition between the addView and the updateProps , don't know why we had it especially!

[tboba]

@delphinebugner Sure! Then I would say I trust you, as I've tested by myself how our example apps behave 😄 I also believe that someone has just forgot about that super. Let's get this merged 👍

[tboba]

Hi @delphinebugner! I'm happy to say that we've released new version of react-native-screens (3.29.0) that contains changes from this PR 🥳

Check it out! If you find something wrong related to the newest version (this change is still buggy or doesn't work for you) let us know 🎉