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

feat!: android 12 splash screen #1441

Merged
merged 31 commits into from
Jun 30, 2022
Merged

Conversation

erisu
Copy link
Member

@erisu erisu commented Jun 14, 2022

Motivation and Context

Closes: #1313

Support Android 12 Splash Screen API and backwards compatibility

Description

Old Splash Screen

  • Drop old copy resources for previous previous splash screen plugin.
  • Remove old resource files

New Splash Screen

  • Created new template resources
  • Added compat library to support older devices
  • Added new config.xml preference
    • AndroidPostSplashScreenTheme
      • Considered as advance feature and use at own risk. Unsupported feature.
    • AndroidWindowSplashScreenAnimationDuration
      • Manages the duration of the animation, not the delay of the splash screen.
    • AndroidWindowSplashScreenAnimatedIcon
    • AndroidWindowSplashScreenBackground
      • Background color of the splash screen. Default is white (#FFFFFF)
    • AndroidWindowSplashScreenBrandingImage
      • Adds an image to the bottom of the splash screen.
    • AndroidWindowSplashScreenIconBackgroundColor
  • Continue support for config.xml preference with follow behavioural changes.
    • AutoHideSplashScreen
      • Default=onPageFinished, onPageFinished does not mean JavaScript executed or finished loading. Only covers HTML
    • SplashScreenDelay
      • AutoHideSplashScreen must be true to use.
      • Default=-1 which represents onPageFinished.
      • Can override with custom millisecond value. E.g. 3000 = 3 second delay.
    • FadeSplashScreen
      • Default = true
      • Only controls the fade out.
    • FadeSplashScreenDuration
      • Default=500ms (0.5 seconds)

Testing

  • npm t
  • cordova build android
  • cordova run android
API 31 API 23
Screenshot_20220614_175352 Screenshot_1655196639

Note:

  • Some simulators do not display the splash screen when launch from Android Studio. This appears to be a bug from AS. Launching the app from within the simulator will show the splash screen.
  • The default splash screen is an XML Android Drawable. It has a resolution of 512x512. It is padded and the inner icon size is 384x384. It is also scalled by 50% and pivotX/Y of 256.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@erisu erisu force-pushed the feat/android-12-splashscreen branch 4 times, most recently from facb0e7 to 95737bd Compare June 21, 2022 06:46
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2022

Codecov Report

Merging #1441 (254e5d5) into master (8d6e41f) will decrease coverage by 3.44%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##           master    #1441      +/-   ##
==========================================
- Coverage   75.96%   72.52%   -3.45%     
==========================================
  Files          21       21              
  Lines        1677     1740      +63     
==========================================
- Hits         1274     1262      -12     
- Misses        403      478      +75     
Impacted Files Coverage Δ
lib/Api.js 50.61% <ø> (ø)
lib/prepare.js 47.32% <14.28%> (-12.68%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d6e41f...254e5d5. Read the comment docs.

@erisu erisu added this to the 11.0.0 milestone Jun 24, 2022
Copy link
Member

@NiklasMerz NiklasMerz left a comment

Choose a reason for hiding this comment

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

I have some trouble with the branding image and think the documentation PR will be very important for this.

But the features work in tests with the emulator and the code LGTM at quick glance.

Thank you @erisu for the hard work!

@erisu
Copy link
Member Author

erisu commented Jun 24, 2022

I have some trouble with the branding image and think the documentation PR will be very important for this.

It seems that branding is actually not supported below API 31 and there is an issue ticket open about it. https://issuetracker.google.com/issues/194301890

Maybe I can remove the branding code since it is not working and dont know if it will be supported.

@NiklasMerz
Copy link
Member

It seems that branding is actually not supported below API 31 and there is an issue ticket open about it. https://issuetracker.google.com/issues/194301890

Maybe I can remove the branding code since it is not working and dont know if it will be supported.

Or we just leave it in for future versions and put a hint in the docs?

@erisu erisu force-pushed the feat/android-12-splashscreen branch 2 times, most recently from c446cd1 to f632d4e Compare June 27, 2022 13:35
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

Looks good, but if I open the themes.xml in Android Studio it shows "errors" because it doesn't understand the theme properties.
The templates's app/build.gradle should include implementation "androidx.core:core-splashscreen:${cordovaConfig.ANDROIDX_CORE_SPLASHSCREEN_VERSION}" so it understands the properties.

Also, current default delay is 3000 and it's being changed to -1, we have to remember to document the "breaking" change

@erisu erisu requested a review from jcesarmobile June 28, 2022 01:31
@erisu
Copy link
Member Author

erisu commented Jun 28, 2022

Looks good, but if I open the themes.xml in Android Studio it shows "errors" because it doesn't understand the theme properties. The templates's app/build.gradle should include implementation "androidx.core:core-splashscreen:${cordovaConfig.ANDROIDX_CORE_SPLASHSCREEN_VERSION}" so it understands the properties.

I added the dependency back the app as well.

Also, current default delay is 3000 and it's being changed to -1, we have to remember to document the "breaking" change

I am writting the splash screen docs as well. Since the plugin does not support iOS or Android, the repo should not keep the docs anyways cause it will lead to confusion on what support the plugin has. Those docs are being removed from the plugin repo.

@erisu erisu changed the title feat: android 12 splash screen feat!: android 12 splash screen Jun 28, 2022
@erisu erisu force-pushed the feat/android-12-splashscreen branch from d6b975d to 0ac37d2 Compare June 28, 2022 03:25
@erisu erisu requested a review from timbru31 June 28, 2022 08:54
@erisu erisu force-pushed the feat/android-12-splashscreen branch from 254e5d5 to 3c172be Compare June 29, 2022 02:37
@erisu erisu merged commit 606e9c4 into apache:master Jun 30, 2022
@erisu erisu deleted the feat/android-12-splashscreen branch June 30, 2022 01:49
@NelsonChad
Copy link

Hi ,when i run cordova platform add android@11.0.0 I'm getting this error in prepare.js file

TypeError: Cannot read properties of null (reading 'find')
at E:\DEV\IONIC\donateApp_cord\node_modules\cordova-android\lib\prepare.js:387:49
at Array.forEach (<anonymous>)
at updateProjectSplashScreen (E:\DEV\IONIC\donateApp_cord\node_modules\cordova-android\lib\prepare.js:384:7)
at updateProjectAccordingTo (E:\DEV\IONIC\donateApp_cord\node_modules\cordova-android\lib\prepare.js:269:5)
at E:\DEV\IONIC\donateApp_cord\node_modules\cordova-android\lib\prepare.js:67:21
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async Promise.all (index 0)
[ERROR] An error occurred while running subprocess cordova
```.

I've configurated de config.xml as recommended, can you help me please?

@ATaysikuu
Copy link

Hey, same issue here trying to upgrade a cordova 10 based project to cordova 11.

Cannot read property 'find' of null
TypeError: Cannot read property 'find' of null
    at C:\Jenkins\workspace\REDACTEDPROJECTNAME\node_modules\cordova-android\lib\prepare.js:387:49
    at Array.forEach (<anonymous>)
    at updateProjectSplashScreen (C:\Jenkins\workspace\REDACTEDPROJECTNAME\node_modules\cordova-android\lib\prepare.js:384:7)
    at updateProjectAccordingTo (C:\Jenkins\workspace\REDACTEDPROJECTNAME\node_modules\cordova-android\lib\prepare.js:269:5)
    at C:\Jenkins\workspace\REDACTEDPROJECTNAME\node_modules\cordova-android\lib\prepare.js:67:21
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async Promise.all (index 0)
[ERROR] An error occurred while running subprocess cordova.
        
        cordova platform add android@11 --verbose --save exited with exit code 1.
        
        Re-running this command with the --verbose flag may provide more information.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android 12 splash screen issue
7 participants