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

Fix app restart on split screen (Android) #5073

Closed
wants to merge 6 commits into from

Conversation

Somena1
Copy link
Contributor

@Somena1 Somena1 commented Oct 24, 2021

This commit fixes app restart on split screen and screen resize
issue #5068

Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Somena1 for tracking this down! Will be great to have this fix in.

One tiny comment below. Other than that, the commit history needs some cleaning-up to meet our style. I described this with more details in a chat thread; happy to answer questions there or in the #git help stream.


I tried this out empirically, and it worked great.

The main question was whether the app would successfully handle those changes. Basically this line of the Android manifest is making an assurance to the system that the app is prepared to handle these changes; docs here:
https://developer.android.com/guide/topics/manifest/activity-element#config
When the system doesn't get that assurance, it just restarts the app from scratch to be sure.

And empirically the app indeed does just fine when we add those assurances and then go ahead and switch to split-screen mode, or resize the app's portion of the split screen. That layer of things is basically all RN's job, so RN is handling this just fine -- we just haven't been letting the system know that we're ready to do so.

So that means this change almost certainly should also go into React Native upstream, in their template app. Here's an upstream report of the issue: facebook/react-native#25040
The author of that issue closed it after learning they could make this change in their own app, but it really should go in the template app so that all future RN apps benefit. If you'd like to send a version of this PR to RN upstream too, that would also be a nice contribution.

android/app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
@gnprice gnprice added the upstream: RN Issues related to an issue in React Native label Oct 25, 2021
@Somena1
Copy link
Contributor Author

Somena1 commented Oct 30, 2021

@gnprice I've been been able to fix the author, I still can't resolve the merge commit. I don't know if using the script from tools folder will break the branch.

@gnprice
Copy link
Member

gnprice commented Nov 1, 2021

Thanks for the revision!

I still can't resolve the merge commit. I don't know if using the script from tools folder will break the branch.

Hmm, I don't know what script you're referring to. Please post in the #git help stream, and describe what you've tried and what results you've gotten. Then we can help you there.

@Somena1
Copy link
Contributor Author

Somena1 commented Nov 2, 2021

Okay then, I'll head to the #git help stream

@Somena1 Somena1 force-pushed the Fix-split-screen branch 3 times, most recently from 03cd864 to 4610a00 Compare November 4, 2021 12:37
@Somena1 Somena1 closed this Nov 4, 2021
@Somena1 Somena1 deleted the Fix-split-screen branch November 4, 2021 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-Android a-layout upstream: RN Issues related to an issue in React Native
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants