-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat!: android 12 splash screen #1441
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
Conversation
facb0e7 to
95737bd
Compare
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
NiklasMerz
left a comment
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.
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!
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? |
c446cd1 to
f632d4e
Compare
jcesarmobile
left a comment
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.
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
I added the dependency back the app as well.
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. |
d6b975d to
0ac37d2
Compare
…ndingImage case and added branding warning
254e5d5 to
3c172be
Compare
|
Hi ,when i run cordova platform add [email protected] I'm getting this error in prepare.js file |
|
Hey, same issue here trying to upgrade a cordova 10 based project to cordova 11. |
Motivation and Context
Closes: #1313
Support Android 12 Splash Screen API and backwards compatibility
Description
Old Splash Screen
New Splash Screen
config.xmlpreferenceAndroidPostSplashScreenThemeAndroidWindowSplashScreenAnimationDurationAndroidWindowSplashScreenAnimatedIconxmlAndroid Drawable, orpng.AndroidWindowSplashScreenBackground#FFFFFF)AndroidWindowSplashScreenBrandingImageAndroidWindowSplashScreenIconBackgroundColorconfig.xmlpreferencewith follow behavioural changes.AutoHideSplashScreenSplashScreenDelayAutoHideSplashScreenmust betrueto use.onPageFinished.FadeSplashScreenFadeSplashScreenDurationTesting
npm tcordova build androidcordova run androidNote:
Checklist
(platform)if this change only applies to one platform (e.g.(android))