Features/webp support for splashscreen

[PieterVanPoyer]

Platforms affected

Android

Motivation and Context

• Fixes Build breaks using webp splash images instead of PNG cordova-plugin-splashscreen#257

A splashscreen image on Android can be in a webp format. ( https://developer.android.com/studio/write/convert-webp )
When a webp image is included the build breaks.

Description

If the inputted splash screen has a .webp extension, it is saved to the correct density folder as screen.webp.
An possible old screen.png of the same folder is deleted.

If the inputted splash screen has another extension (png or jpg), it is saved to the correct density folder as screen.png.
An possible old screen.webp of the same folder is deleted.

If android encouters a webp and a png with the same name and density a compilation error is thrown by Android

Testing

• added Unittests

Todo

• Maybe something must be added in the (SplashScreen plugin) documentation.

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

[timbru31]

Could you also test if .jpg splashscreen do work? It's stated in the docs, but it seems like that there is a hardcoded check to screen.png.

[codecov-io]

Codecov Report

Merging #1113 (30bcaa5) into master (335b0f2) will increase coverage by 2.09%.
The diff coverage is 38.46%.

@@            Coverage Diff             @@
##           master    #1113      +/-   ##
==========================================
+ Coverage   69.71%   71.80%   +2.09%     
==========================================
  Files          20       21       +1     
  Lines        1806     1745      -61     
==========================================
- Hits         1259     1253       -6     
+ Misses        547      492      -55     

Impacted Files	Coverage Δ
bin/templates/cordova/lib/prepare.js	47.79% <38.46%> (+3.88%)	⬆️
bin/templates/cordova/lib/utils.js	92.85% <0.00%> (-7.15%)	⬇️
bin/templates/cordova/lib/Adb.js	100.00% <0.00%> (ø)
bin/templates/cordova/lib/run.js	100.00% <0.00%> (ø)
bin/templates/cordova/lib/device.js	100.00% <0.00%> (ø)
bin/templates/cordova/lib/target.js	96.77% <0.00%> (ø)
bin/templates/cordova/lib/emulator.js	98.84% <0.00%> (+0.71%)	⬆️
...n/templates/cordova/lib/builders/ProjectBuilder.js	74.83% <0.00%> (+1.62%)	⬆️
bin/templates/cordova/lib/build.js	33.33% <0.00%> (+2.22%)	⬆️
... and 1 more

---

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 335b0f2...30bcaa5. Read the comment docs.

[PieterVanPoyer]

Could you also test if .jpg splashscreen do work? It's stated in the docs, but it seems like that there is a hardcoded check to screen.png.

@timbru31 Hey I did test the .jpg flow. Really odd, but it works. So what happens, the .jpg is saved as a screen.png in the android project. But android is just displaying it nice.

A testrepo is available at: https://github.com/PieterVanPoyer/cordova-plugin-splashscreen-testing-app/tree/testing/jpg-splash-screen

[PieterVanPoyer]

@erisu and @timbru31 I hate to disturb you guys. But is anyone able to review this PR. Kind regards.

[breautek]

Looks good to me.

Tested it on one of my apps 👍

[raphinesse]

I only had the time to take a quick look at this. It is great that we have test coverage for this feature and the code looks clean 👍

However, I can't help to think that it is not optimal that we have to hard-code the handling of a specific set of extensions into the core platform to implement a plugin feature.

So can someone shed some light on why we have the splash-screen logic here in the first place?

And could we at least implement this in a more generic way? Something more like

delete $dest/screen.*
copy $splashImg $dest/$splashImg

[PieterVanPoyer]

Hi @raphinesse thanks for your time.

I was also happy with the testcoverage. There were some examples and it was easy to just add my stuff and improve the coverage. And like you said: 'the code is clean'.

I must admit, when I was looking for fixing the issue, I was surprised to find the handling of the splashscreens in the cordova-android lib and not in the plugin code. Unfortunately I cannot shed any light on that matter.

I think making this stuff more generic wouldn't be a real improvement, and I don't exactly know how you'd like that to be.
On the other hand, taking the time to make an issue in the splashscreen plugin to move the splashscreen code to the right place would be a good first step.
When the handling (copying, deleting) of splashscreens is in the plugin, it can be less generic, because that's what the plugin should do. Handling splashscreens.

Kind regards
Pieter

[raphinesse]

taking the time to make an issue in the splashscreen plugin to move the splashscreen code to the right place would be a good first step.

Actually, it seems that we have plans to move things in the opposite direction. Sorry, I did not remember that. I don't know why this functionality should be moved from plugin to platform, but there is probably a good reason. So the platform seems indeed like the right location for these changes.

I'll get back to you regarding a more generic way to implement this.

[raphinesse]

@PieterVanPoyer FYI: I made a pull request (PieterVanPoyer#1) against this branch

[raphinesse]

LGTM now. Thanks a lot for testing and fixing my changes @PieterVanPoyer !

[breautek]

Thank you for the review @raphinesse

Our current dev version is targeting a minor release so I'll go ahead and merge this. Thank you @PieterVanPoyer for your time and effort in preparing this PR!

[PieterVanPoyer]

@breautek thanks for the effort. Maybe I should make an issue in the Cordova Docs repo, to add some info about the webp - support to the docs?