-
Notifications
You must be signed in to change notification settings - Fork 1
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
[VO-428] fix: Prevent the app to stuck on SplashScreen after a restart notification #1181
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ldoppea
changed the title
fix: Prevent the app to stuck on SplashScreen after a restart notification
[VO-428] fix: Prevent the app to stuck on SplashScreen after a restart notification
Feb 28, 2024
acezard
reviewed
Feb 29, 2024
return splashScreenLogger.info(`Splash screen shown "${bootsplashName}"`) | ||
const result = await RNBootSplash.show({ fade: true, bootsplashName }) | ||
splashScreenLogger.info( | ||
`Splash screen shown "${bootsplashName}" (${result.toString()})` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like there are typing issues detected by github ci here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because cozy/react-native-bootsplash#4 is not merged yet
zatteo
approved these changes
Feb 29, 2024
Ldoppea
force-pushed
the
fix/iap_refresh
branch
2 times, most recently
from
February 29, 2024 15:36
3dfd6bf
to
74b8129
Compare
`react-native-bootsplash` has been upgraded to retrieve a fix that ensures the `show` and `hide` methods always resolve their promises Related PR: cozy/react-native-bootsplash#4
On Android, when the app request a restart, the current Activity is not directly destroyed and the app react as if it was sent to `background` This triggers the `useGlobalAppState` hooks which shows the `LOCK_SCREEN` splashScreen The after the app effectively restarted, it fails to hide all splashScreens because the `LOCK_SCREEN` one is not expected to be displayed (but it is) To prevent this kind of side effects, we want to fully unmount the components tree, so all hooks would unregister themselves This commit introduces the RestartProvider that is responsible to unmount the entire components tree when `unmountAppForRestart()` is called
In previous commit we created the RestartProvider in order to unmount the app and unregister all hooks To make this work, we need hooks to correctly unregister event listener when unmount
With previous implementation we blindly hid the LOCK_SCREEN's splash screen when the app would resume This was made in #1139 to ensure we don't forget any path But we found that this approach is too naive as it would fail when the app requests a restart This is because, on Android, when the app requests a restart, the current Activity is not directly destroyed and the app react as if it was sent to `background`. In that scenario the hook would show the LOCK_SCREEN's splash screen and then would fail to hide it, as it would consider the app is starting from scratch This side effect has been fixed in previous commit as the entire component's tree is now unmount before restart and so the LOCK_SCREEN is not shown anymore. But we still want to revert to specificly placed call instead of global one, so we have better control on what is happening Related PR: #1139
We initially had to stop the HttpServer when restarting the app otherwise it would survive the react-native restart (because it is implemented on native side) But since we now unmount the entire component's tree, then the HttpServerProvider already stop the server when it's hooks are unmount So calling `stop()` here would crash the iOS app that is not able to handle calling `stop()` when server is already stopped
In previous commit we removed the `httpServerProvider.restart()` call, so we can now move all the restart related code inside of the RestartProvider (HttpServerProvider is under RestartProvider)
Ldoppea
force-pushed
the
fix/iap_refresh
branch
from
February 29, 2024 15:52
74b8129
to
c933080
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On Android, when the app request a restart, the current Activity is not
directly destroyed and the app react as if it was sent to
background
This triggers the
useGlobalAppState
hooks which shows theLOCK_SCREEN
splashScreenThe after the app effectively restarted, it fails to hide all
splashScreens because the
LOCK_SCREEN
one is not expected to bedisplayed (but it is)
To prevent this kind of side effects, we want to fully unmount the
components tree, so all hooks would unregister themselves
This PR also fix some hideSplashScreen scenario and improve logs
TODO: