React-native-navigation: refactor RNNSplashView and allow custom implementations

Created on 6 Mar 2017  ·  7Comments  ·  Source: wix/react-native-navigation

Currently RNNSplashView is a mess (one giant method) and needs some refactoring.

Also, we need tests to make sure custom splash screens are supported (it looks like they are but if something were to break there currently is no way to tell). Testing it won't be easy as we need some way to control it from outside. Maybe xctests are the solution here unfortunately.

iOS v2 🏚 stale

All 7 comments

Can you please provide more information on this issue? Probably it's something that community can help you with?

@Kureev updated

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

I looked into the RNNSpalashScreen. Below is what I can understand is happening.

  • show method tries different ways to get the LaunchImage or LaunchScreen and show it as rootViewController
  • First try to get the LaunchScreen storyboard and load it if it is available. In case if failed to load, take the root view from storyboard view controller and load it
  • Secondly, try to get the LaunchImage by checking Default or LaunchImage from assets catalog and loads it.

From initial look this is what I can propose, comments or suggestion welcome.

  • Rename RNNSplashScreen to RNNSplashViewController
  • Remove the static method show. Instead allocate and init in ReactNativeNavigation or where ever it needs to be accessed
  • Splash VC decides which way to use to get the launch screen or image
  • Keep the logic to get the launch screen from storyboard
  • Refactor the code for getting launch image from asset catelog to loop through the assets and try to find a image named LaunchImage. Once its found check whether image same scale and dimensions as our current device's screen. This way we can support all the devices and screens without hardcoding the dimensions and image names. Probably future proof with all new iDevices and screen sizes. I found this SO answer really helpful.
  • Add XCTest to init the splash view controller and check its root view for launch screen or image. Probably allow injecting the view controller with which way to search splash. (via storyboard or assets)

@DanielZlotin

Also, one more thing would like to add is bootstrap the RNN without the splash screen. Probably extra API method which only allocate and set the RNNBridgeManager. The reason behind this is I have a brownfield app and currently managed to make it work with some custom native code. But I would like to avoid splash when bootstrapping. May be take this as first step to support brownfield apps.

@rizwankce Good debugging! Can you make PR of those changes?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

The issue has been closed for inactivity.

Was this page helpful?
0 / 5 - 0 ratings