CB-13685 android: Adaptive Icon Support

[erisu]

Platforms affected

Android

What does this PR do?

This PR adds support for Adaptive Icon.

Changes to Default Template Project

• Updated AndroidManifest to use adaptive icons configuration by default.
• Added adaptive icons and ic_launcher.xml for Android platform 26 and higher.

Added JavaScript Unit Testing.

config.xml for Adaptive Icons with Images

<platform name="android">
  <icon background="res/icon/android/ldpi-background.png" density="ldpi" foreground="res/icon/android/ldpi-foreground.png" />
  <icon background="res/icon/android/mdpi-background.png" density="mdpi" foreground="res/icon/android/mdpi-foreground.png" />
  <icon background="res/icon/android/hdpi-background.png" density="hdpi" foreground="res/icon/android/hdpi-foreground.png" />
  <icon background="res/icon/android/xhdpi-background.png" density="xhdpi" foreground="res/icon/android/xhdpi-foreground.png" />
  <icon background="res/icon/android/xxhdpi-background.png" density="xxhdpi" foreground="res/icon/android/xxhdpi-foreground.png" />
  <icon background="res/icon/android/xxxhdpi-background.png" density="xxxhdpi" foreground="res/icon/android/xxxhdpi-foreground.png" />
</platform>

In this case, the foreground image will automatically be applied to the src attribute value. This is to support non-adaptive icon supported Android devices. If the user supplies their own src value, foreground will not be copied over.

config.xml for Adaptive Icons with Vectors

<platform name="android">
  <icon background="res/icon/android/ldpi-background.xml” density="ldpi" foreground="res/icon/android/ldpi-foreground.xml” src="res/icon/android/ldpi-icon.png" />
  <icon background="res/icon/android/mdpi-background.xml” density="mdpi" foreground="res/icon/android/mdpi-foreground.xml” src="res/icon/android/mdpi-icon.png" />
  <icon background="res/icon/android/hdpi-background.xml” density="hdpi" foreground="res/icon/android/hdpi-foreground.xml” src="res/icon/android/hdpi-icon.png" />
  <icon background="res/icon/android/xhdpi-background.xml” density="xhdpi" foreground="res/icon/android/xhdpi-foreground.xml” src="res/icon/android/xdpi-icon.png" />
  <icon background="res/icon/android/xxhdpi-background.xml” density="xxhdpi" foreground="res/icon/android/xxhdpi-foreground.xml” src="res/icon/android/xxhdpi-icon.png" />
  <icon background="res/icon/android/xxxhdpi-background.xml” density="xxxhdpi" foreground="res/icon/android/xxxhdpi-foreground.xml” src="res/icon/android/xxxhdpi-icon.png" />
</platform>

In this example, the adaptive icon supports vectors and colors for the foreground and background. If the foreground is a vector or color, the src attribute must also be defined with a PNG for Android devices that does not support adaptive icon.

config.xml for Adaptive Icons with Colors for Background First: create colors.xml file with content

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <color name="background">#FF0000</color>
</resources>

Second: update config.xml with…

<platform name="android">
  <resource-file src="/res/values/colors.xml" target="/app/src/main/res/values/colors.xml" />

  <icon background=“@color/background” density="ldpi" foreground="res/icon/android/ldpi-foreground.png" />
  <icon background="@color/background" density="mdpi" foreground="res/icon/android/mdpi-foreground.png" />
  <icon background="@color/background" density="hdpi" foreground="res/icon/android/hdpi-foreground.png" />
  <icon background="@color/background" density="xhdpi" foreground="res/icon/android/xhdpi-foreground.png" />
  <icon background="@color/background" density="xxhdpi" foreground="res/icon/android/xxhdpi-foreground.png" />
  <icon background="@color/background" density="xxxhdpi" foreground="res/icon/android/xxxhdpi-foreground.png" />
</platform>

The naming convention for the two new attributes, background and foreground, was decided to match the element names that is used in the ic_launcher.xml for consistency. https://developer.android.com/guide/practices/ui_guidelines/icon_design_adaptive#creating_adaptive_icons_in_xml


Error Example: Foreground contains vector and missing src attribute.

<platform name="android">
  <icon background="res/icon/android/mdpi-background.png" foreground="res/icon/android/mdpi-foreground.xml” density=“mdpi" />
</platform>

What testing has been done on this change?

• Built default app with no modifications to config.xml
• Built default app with legacy icon set defined
• Built default app with adaptive icon set with images.
• Built default app with adaptive icon set with vectors.
• Built default app with adaptive icon set with colors.
• Converted from adaptive to legacy icon set and rebuilt.
• Converted from legacy to adaptive icon set and rebuild.

Checklist

• [X] Reported an issue in the JIRA database
• [X] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• [X] Added automated test coverage as appropriate for this change.

[erisu]

I also want to point out that this PR will be required.

https://github.com/apache/cordova-common/pull/26

This PR contains the updates to the ConfigParser to include foreground and background when getting static resources.

[dpogue]

You can ignore the node 4 Appveyor failure due to let/const, we're in the process of dropping support for node 4.

[erisu]

@dpogue I have also noticed this.

There is the CI improvement PR, https://github.com/apache/cordova-android/pull/442, which deprecates node 4, adds node 10 support, and other changes.

When I tested my PR with the CI improvement PR, other issues appeared. https://ci.appveyor.com/project/erisu/cordova-android/build/1.0.14

If we just remove node 4 and add node 10, with no additional changes, then my PR passed. https://ci.appveyor.com/project/erisu/cordova-android/build/1.0.17

[raphinesse]

@erisu I rebased #442 onto latest master and your changes on top. CI tests pass on Travis as well as AppVeyor. I guess the failures were fixed by #450.

Note that in your "passing" CI tests with only Node 4 removed, the Java tests actually never ran, due to a bug that is fixed by #442.

[codecov-io]

Codecov Report

Merging #448 into master will decrease coverage by 19.06%. The diff coverage is 66.92%.

@@             Coverage Diff             @@
##           master     #448       +/-   ##
===========================================
- Coverage    62.9%   43.84%   -19.07%     
===========================================
  Files          15       18        +3     
  Lines        1650     2023      +373     
  Branches      308      383       +75     
===========================================
- Hits         1038      887      -151     
- Misses        612     1136      +524

Impacted Files	Coverage Δ
bin/templates/cordova/lib/AndroidManifest.js	35.44% <40%> (-64.56%)	:arrow_down:
bin/templates/cordova/lib/prepare.js	41.66% <68%> (ø)
bin/templates/cordova/lib/retry.js	15.38% <0%> (-84.62%)	:arrow_down:
bin/templates/cordova/lib/device.js	22.44% <0%> (-77.56%)	:arrow_down:
bin/templates/cordova/lib/run.js	26.98% <0%> (-73.02%)	:arrow_down:
bin/templates/cordova/lib/Adb.js	34.14% <0%> (-65.86%)	:arrow_down:
bin/templates/cordova/lib/builders/builders.js	37.5% <0%> (-62.5%)	:arrow_down:
bin/templates/cordova/lib/emulator.js	48.64% <0%> (-40.97%)	:arrow_down:
...n/templates/cordova/lib/builders/ProjectBuilder.js
... and 6 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 ebbd91f...821038e. Read the comment docs.

[erisu]

@raphinesse I rebased with the latest master now that the #442 PR has been merged. Do you think this is ready to merge?

[raphinesse]

Sorry, I know very little about the Android build, so I can't really review this. Maybe @dpogue or @infiloop.

[erisu]

@raphinesse Thank you for pointing out improvements to my test spec. I have updated the test and reduced the line count. I also removed the done() calls as you pointed out because they were not needed.

[Menardi]

This PR looks good to me. I need adaptive icons for one of my own apps, so I'm eager to see this get merged soon. Without support for this, icons for apps using cordova-android@7 (targeting API 26+) look a bit rubbish on newer devices.

I noticed the main cordova-android maintainer, Joe, has announced on the mailing list that he is stepping down. So, I expect that getting this thoroughly reviewed is now more difficult. If there is anything I can do to get this over the line, please let me know.

[dpogue]

I will make sure this gets merged into the next major

[janpio]

@raphinesse Could you have a look again and redo your review please?

[raphinesse]

@janpio Unfortunately I am on a very tight schedule right now. The only thing I had reviewed were the specs. I glanced over them again and they do look much better. I still saw a few things that could be improved, but I don't have time to tackle that right now.

Since it's "only the specs", I would not want to block this PR because of this. However, I can neither give my approval for the rest of this PR since I know too little of the subject at hand.

[janpio]

Thanks @raphinesse! Same for me... hope someone with actual Android experience turns up.

[erisu]

@janpio I rebased with the latest master. I also did the same for cordova-common's PR that is associated with this PR.

The downside, both cordova-common and cordova-android have jasmine@^3.1.0 as the dep. Both will update to 3.2.0, and It seems the cordova-common tests already failed for what I believe is the same side-effect of the latest Jasmine's release.

https://github.com/apache/cordova-common/pull/26

I am expecting these tests may show similar side-effects. We might need to update both repo's with the temp fix...

[dpogue]

The changes in this PR look fine to me. I had held off on merging due to some of the questions on the cordova-common side about where the Android-specific handling code should live, especially with @erisu's proposed refactoring.

[wisespira]

Do I need to install anything on top of cordova to get this to work? I'm currently getting a Unquoted attribute value error

[janpio]

Are you using cordova-android master in your app? This hasn't really been released yet.

[wisespira]

Oops thanks I'll again with the master branch

[erisu]

Though the PR is merged into master, the package.json still references cordova-common@^2.2.0. There is a cordova-common PR that is also necessary for this change to work. The cordova-common PR has also been merged into master which will be released on cordova-common@3.0.0.

Using cordova-android@master alone will not work unless you link cordova-common@master. Or wait for release.

[LarryBJohnson]

How would one go about switching from standered npm cordova to this master branch?

[janpio]

What does slandered npm cordova mean?

[LarryBJohnson]

If I where to go in my command line and download cordova using node js I would type npm install -g cordova would this give me a version of cordova that would let me use adaptive icons ?

[janpio]

No.

[raphinesse]

No. npm i -g cordova@nightly and npm i cordova-android@nightly in your project

[LarryBJohnson]

so does adding @nightly switch it to this branch ? allowing me to create adaptive icons with cordova ?

[raphinesse]

No. It uses the latest nightly build. Should be easier to setup than using master. Try an earlier nightly build if npm throws errors at you during installation. A full list of available versions can be found via npm.

[nprail]

Has this been released yet? If not, when will it be?

[dpogue]

It is not released yet, it will be included in the next major version. We can't/won't commit to any particular timeline for that next major, but we are working on it.