You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2022/09/11 00:16:16 UTC
[GitHub] [cordova-android] jcesarmobile opened a new pull request, #1487: fix: Add android prefix to windowSplashScreenBrandingImage
jcesarmobile opened a new pull request, #1487:
URL: https://github.com/apache/cordova-android/pull/1487
closes https://github.com/apache/cordova-android/issues/1475
`windowSplashScreenBrandingImage` is not valid, it should be `android:windowSplashScreenBrandingImage`.
All other values should also include the `android:`, but the support library allows to use them without the `android:` part, but since `windowSplashScreenBrandingImage` is not supported by the support library apps fail to compile.
This PR adds the `android:` prefix to the value so it gets added to the styles file with the prefix.
Needed to patch how the cdvConfigPrefKey is generated to strip the android: part of the themeKey
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a diff in pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
erisu commented on code in PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#discussion_r968388910
##########
lib/prepare.js:
##########
@@ -378,14 +378,18 @@ function updateProjectSplashScreen (platformConfig, locations) {
'windowSplashScreenAnimatedIcon',
'windowSplashScreenAnimationDuration',
'windowSplashScreenBackground',
- 'windowSplashScreenBrandingImage',
+ 'android:windowSplashScreenBrandingImage',
'windowSplashScreenIconBackgroundColor',
'postSplashScreenTheme'
].forEach(themeKey => {
- const cdvConfigPrefKey = 'Android' + themeKey.charAt(0).toUpperCase() + themeKey.slice(1);
+ let index = 0;
+ if (themeKey.indexOf(':') !== -1) {
+ index = themeKey.indexOf(':') + 1;
+ }
+ const cdvConfigPrefKey = 'Android' + themeKey.charAt(index).toUpperCase() + themeKey.slice(index + 1);
Review Comment:
What you have is good. Dont need to use replace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] leMaik commented on pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
leMaik commented on PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#issuecomment-1312843126
It's been a while; is there any chance we can get this released? :pray:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] dpogue commented on a diff in pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
dpogue commented on code in PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#discussion_r967718653
##########
lib/prepare.js:
##########
@@ -378,14 +378,18 @@ function updateProjectSplashScreen (platformConfig, locations) {
'windowSplashScreenAnimatedIcon',
'windowSplashScreenAnimationDuration',
'windowSplashScreenBackground',
- 'windowSplashScreenBrandingImage',
+ 'android:windowSplashScreenBrandingImage',
'windowSplashScreenIconBackgroundColor',
'postSplashScreenTheme'
].forEach(themeKey => {
- const cdvConfigPrefKey = 'Android' + themeKey.charAt(0).toUpperCase() + themeKey.slice(1);
+ let index = 0;
+ if (themeKey.indexOf(':') !== -1) {
+ index = themeKey.indexOf(':') + 1;
+ }
+ const cdvConfigPrefKey = 'Android' + themeKey.charAt(index).toUpperCase() + themeKey.slice(index + 1);
const cdvConfigPrefValue = platformConfig.getPreference(cdvConfigPrefKey, this.platform);
let themeTargetNode = splashScreenTheme.find(`item[@name="${themeKey}"]`);
-
+ console.log('hola', cdvConfigPrefKey);
Review Comment:
stray console.log
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] erisu commented on a diff in pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
erisu commented on code in PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#discussion_r967759668
##########
lib/prepare.js:
##########
@@ -378,14 +378,18 @@ function updateProjectSplashScreen (platformConfig, locations) {
'windowSplashScreenAnimatedIcon',
'windowSplashScreenAnimationDuration',
'windowSplashScreenBackground',
- 'windowSplashScreenBrandingImage',
+ 'android:windowSplashScreenBrandingImage',
'windowSplashScreenIconBackgroundColor',
'postSplashScreenTheme'
].forEach(themeKey => {
- const cdvConfigPrefKey = 'Android' + themeKey.charAt(0).toUpperCase() + themeKey.slice(1);
+ let index = 0;
+ if (themeKey.indexOf(':') !== -1) {
+ index = themeKey.indexOf(':') + 1;
+ }
+ const cdvConfigPrefKey = 'Android' + themeKey.charAt(index).toUpperCase() + themeKey.slice(index + 1);
Review Comment:
Not sure if this is better performance or eaiser to read, but what about this?
```js
const themeKeyDescoped = themeKey.replace('android:', '');
const cdvConfigPrefKey = 'Android' + themeKeyDescoped.charAt(0).toUpperCase() + themeKeyDescoped.slice(1);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] jcesarmobile merged pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
jcesarmobile merged PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] codecov-commenter commented on pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#issuecomment-1242824738
# [Codecov](https://codecov.io/gh/apache/cordova-android/pull/1487?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1487](https://codecov.io/gh/apache/cordova-android/pull/1487?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2c7566a) into [master](https://codecov.io/gh/apache/cordova-android/commit/60e3803c67734087b7a6ff8ee8bd3a659cb050f8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (60e3803) will **decrease** coverage by `0.16%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #1487 +/- ##
==========================================
- Coverage 72.25% 72.08% -0.17%
==========================================
Files 21 21
Lines 1748 1752 +4
==========================================
Hits 1263 1263
- Misses 485 489 +4
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-android/pull/1487?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [lib/prepare.js](https://codecov.io/gh/apache/cordova-android/pull/1487/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliL3ByZXBhcmUuanM=) | `46.17% <0.00%> (-0.47%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] jcesarmobile commented on a diff in pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
jcesarmobile commented on code in PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#discussion_r967838038
##########
lib/prepare.js:
##########
@@ -378,14 +378,18 @@ function updateProjectSplashScreen (platformConfig, locations) {
'windowSplashScreenAnimatedIcon',
'windowSplashScreenAnimationDuration',
'windowSplashScreenBackground',
- 'windowSplashScreenBrandingImage',
+ 'android:windowSplashScreenBrandingImage',
'windowSplashScreenIconBackgroundColor',
'postSplashScreenTheme'
].forEach(themeKey => {
- const cdvConfigPrefKey = 'Android' + themeKey.charAt(0).toUpperCase() + themeKey.slice(1);
+ let index = 0;
+ if (themeKey.indexOf(':') !== -1) {
+ index = themeKey.indexOf(':') + 1;
+ }
+ const cdvConfigPrefKey = 'Android' + themeKey.charAt(index).toUpperCase() + themeKey.slice(index + 1);
const cdvConfigPrefValue = platformConfig.getPreference(cdvConfigPrefKey, this.platform);
let themeTargetNode = splashScreenTheme.find(`item[@name="${themeKey}"]`);
-
+ console.log('hola', cdvConfigPrefKey);
Review Comment:
wops, sorry, shouldn't commit so late, I was pretty sure I deleted all the console messages
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] jcesarmobile commented on a diff in pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
jcesarmobile commented on code in PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#discussion_r967837939
##########
lib/prepare.js:
##########
@@ -378,14 +378,18 @@ function updateProjectSplashScreen (platformConfig, locations) {
'windowSplashScreenAnimatedIcon',
'windowSplashScreenAnimationDuration',
'windowSplashScreenBackground',
- 'windowSplashScreenBrandingImage',
+ 'android:windowSplashScreenBrandingImage',
'windowSplashScreenIconBackgroundColor',
'postSplashScreenTheme'
].forEach(themeKey => {
- const cdvConfigPrefKey = 'Android' + themeKey.charAt(0).toUpperCase() + themeKey.slice(1);
+ let index = 0;
+ if (themeKey.indexOf(':') !== -1) {
+ index = themeKey.indexOf(':') + 1;
+ }
+ const cdvConfigPrefKey = 'Android' + themeKey.charAt(index).toUpperCase() + themeKey.slice(index + 1);
Review Comment:
I have removed the if as it's not really necessary, so the code is simpler now, let me know if you still prefer to use the replace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-android] jcesarmobile commented on pull request #1487: fix: Add android prefix to windowSplashScreenBrandingImage
Posted by GitBox <gi...@apache.org>.
jcesarmobile commented on PR #1487:
URL: https://github.com/apache/cordova-android/pull/1487#issuecomment-1242978971
Android Studio underlines the `android:windowSplashScreenBrandingImage` in red warning about it being a target SDK 31+ feature, but still compiles, so not sure if we need to fix it.
While at the moment the `windowSplashScreenBrandingImage` is red and fails to compile.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org