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.

Sign up for GitHub

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

Jump to bottom

[Android][sdk39] Make SplashScreen methods work in SDK39 #10294

Merged
merged 8 commits into from Sep 24, 2020
Merged

[Android][sdk39] Make SplashScreen methods work in SDK39 #10294

merged 8 commits into from Sep 24, 2020

Conversation

bbarthec
Copy link
Contributor

@bbarthec bbarthec commented Sep 22, 2020 •
edited

Redirect native code flow from verisioned splash screen module realm storing no information to unversioned splash screen module realm that stores all information about current activity's splash.

Why

Resolves #10263

How

  • in SDK 39 codebase whenever you make a call into native methods you end up in versioned native codebase
  • each ExperienceActivity is being registered in unversioned expo-splash-screen
  • unfortunately versioned expo-splash-screen is not using SingletonModules (yet?) and therefore it tries to operate on unregistered activity (because the activity is registered in unversioned module)
  • I've redirected the flow from versioned to unversioned realm by simply importing in the versioned module the kotlin object from unversioned module that handles the whole splash screen logic

Other solution would be:

  • create package with the SplashScreenSingletonModule interface and publish as a separate package (that would add complexity into expo-splash-screen package itself)
  • register unversioned SplashScreen object as a singleton module in ModuleRegistry
  • use ModuleRegistry.getSingletonModule("SplashScreenSingletonModuleName", SplashScreenInterface.class); (that would add complexity into expo-splash-screen package itself)

Test Plan

  • open Expo Client
  • expo init & select tabs managed template
  • see that no warning is thrown and everything works (both SplashScreen.preventAutoHideAsync and SplashScreen.hideAsync)

@bbarthec
[Android][sdk39] Make SplashScreen methods work
e231ffb 
Redirect native code flow from verisioned splash screen module realm storing no information to unversioned splash screen module realm that stores all information about current activity's splash.
sjchmiela
sjchmiela reviewed Sep 22, 2020
View reviewed changes
Copy link
Contributor

@sjchmiela sjchmiela left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd advocate for making these other changes in this PR. 👍

...s/expoview-abi39_0_0/src/main/java/abi39_0_0/expo/modules/splashscreen/SplashScreenModule.kt Outdated Show resolved Hide resolved
...s/expoview-abi39_0_0/src/main/java/abi39_0_0/expo/modules/splashscreen/SplashScreenModule.kt Outdated Show resolved Hide resolved
...s/expoview-abi39_0_0/src/main/java/abi39_0_0/expo/modules/splashscreen/SplashScreenModule.kt Show resolved Hide resolved
@bbarthec @sjchmiela
Update android/versioned-abis/expoview-abi39_0_0/src/main/java/abi39_…
a9b817a 
…0_0/expo/modules/splashscreen/SplashScreenModule.kt

Co-authored-by: Stanisław Chmiela <sjchmiela@users.noreply.github.com>
@bbarthec
Update android/versioned-abis/expoview-abi39_0_0/src/main/java/abi39_…
4bbdac5 
…0_0/expo/modules/splashscreen/SplashScreenModule.kt
@github-actions
Copy link
Contributor

Native Component List for this branch is ready

sjchmiela
sjchmiela approved these changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@sjchmiela sjchmiela left a comment

Choose a reason for hiding this comment

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

Superb! Thanks for taking care of everything! 👍

@pdandradeb pdandradeb mentioned this pull request Sep 23, 2020
Closed
2 tasks
@bbarthec bbarthec merged commit 1b2b00b into master Sep 24, 2020
@bbarthec bbarthec deleted the @bbarthec/sdk-39/splash-screen/fix-no-native-splash-screen-registered branch September 24, 2020 08:52
bbarthec added a commit that referenced this pull request Sep 24, 2020
@bbarthec @sjchmiela
[Android][sdk39] Make SplashScreen methods work (#10294)
bce9476 
Co-authored-by: Stanisław Chmiela <sjchmiela@users.noreply.github.com>
bbarthec added a commit to expo/expo-cli that referenced this pull request Sep 24, 2020
@bbarthec
[configure-splash-screen] <feat>: Accomodate Android API change
8f9cf1b 
expo/expo#10294 introduces scoping to SplashScreen object used in project's MainActivity
@bbarthec bbarthec mentioned this pull request Sep 24, 2020
Merged
3 tasks
bbarthec added a commit to expo/expo-cli that referenced this pull request Sep 24, 2020
@bbarthec
[configure-splash-screen] <feat>: Accomodate Android API change
cbbca5d 
expo/expo#10294 introduces scoping to SplashScreen object used in project's MainActivity
bbarthec added a commit to expo/expo-cli that referenced this pull request Sep 28, 2020
@bbarthec
[configure-splash-screen] <feat>: Accomodate Android API change
4bbc065 
expo/expo#10294 introduces scoping to SplashScreen object used in project's MainActivity
bbarthec added a commit to expo/expo-cli that referenced this pull request Sep 28, 2020
@bbarthec
[configure-splash-screen]<feat>: Accommodate Android singletons.Splas…
ff0014c 
…hScreen import from the subpackage (#2699)

expo/expo#10294 introduces scoping to SplashScreen object used in project's MainActivity
@bbarthec bbarthec changed the title [Android][sdk39] Make SplashScreen methods work [Android][sdk39] Make SplashScreen methods work in SDK39 Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjchmiela sjchmiela sjchmiela approved these changes

@esamelson esamelson Awaiting requested review from esamelson

@tsapeta tsapeta Awaiting requested review from tsapeta tsapeta is a code owner

Assignees

@sjchmiela sjchmiela

@bbarthec bbarthec

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ios][Android] Warning about native splash screen in latest Expo development clients
3 participants
@bbarthec @sjchmiela @esamelson