1201627 - Unify alert_* icons across API levels

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

My use case: adding a new animated "sync" icon for use by a notification, and replacing the default Firefox notification icon with a "sync problem" icon for an existing notification.

I observed that most of our alert_ icons are split by API level.  We have, e.g.,

https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/drawable-xhdpi/alert_app.png

https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/drawable-xhdpi-v11/alert_app.png

which are 36x36, grey and 48x48, white, respectively.  It's not clear why we need different color schemes and sizes.

Comment 1

[Nick Alexander :nalexander [he/him]]

mcomella suggests that these can be consolidated, simplifying our lives just that little bit.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Also worth noting that we only have one set of animating icons, which were grey (I changed download to white in bug 1186020 and bug 1199015 is open to change it for the web app animation). This is also inconsistent with our static icons.

Anthony, do you have a problem with the notification icons being white across all versions?

Sebastian, do you know of any Android history here? Is there a strong reason for notification icons being grey? If so and we break convention, how bad is that?

Comment 3

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I'd like to see if we can do a comparison of these affects across different versions. Can the we do this through an emulator easily? In particular, how do we look compared to others that (i suspect) will be grey in older Android versions.

But, personally I don't see a problem with changing over to a solid white. Especially if we start moving towards a "1 UI to rule them all" approach.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

NI self to get those comparison shots.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I can confirm we need different sizes (I used the larger size on GB and it displayed at the larger size rather than being scaled), however, these assets can be offset with inset/scale drawables.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: 5+ notifications

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: GB notifications

I think there was a different color scheme on ICS which may have changed in JB or KK, but I can't remember, and we don't really support that config.

Anthony, what do you think?

To summarize the trade-offs:
  * One set of icons: we save APK size but at the cost of maintaining an extra set of inset/scale drawables
  * Two sets of icons: we can match platform convention without developer effort. But this is expensive in places (e.g. animations)

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Unadjusted GB notification

If you wanted to see what happens.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

At Anthony's prompting, I discovered we can tint the notification icons – Notification.Builder.setSmallIcon takes an Icon, which has Icon.setTint.

NI self to see how much work this would be (e.g. notifications are all called from Java [1]).

[1]: https://mxr.mozilla.org/mozilla-central/search?string=alert_download&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #2)
> Sebastian, do you know of any Android history here? Is there a strong reason
> for notification icons being grey? If so and we break convention, how bad is
> that?

Apart from the icons looking "wrong" on a platform I do not see any strong reason. I guess it comes down whether we want to start abandoning GB until we do not support it anymore or if we still want to have the best possible experience there.
However try to also compare the icons when the notification shade is expanded (Do we show them there?) because this has changed a lot in the past too (bright/dark backgrounds).

Comment 11

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I'm more concerned about ICS and KitKat's grey icons than GB's if that helps

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

If we tint all the notifications (which I think all of them should be tinted), it's pretty easy to ship one set of icons. All of our notifications go through NotificationHelper [1] which uses NotificationCompat.Builder to put together the notification (where we can use the strategy from comment 9).

We would use resource aliases and inset drawables to pad the notifications on lower API levels (so they scale down) – this would be the hardest part and require a bit of testing to get right.

This could also go under a fatfennec initiative (set blocking).

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/NotificationHelper.java?rev=8234563d01fc#99

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Note that we are currently inconsistent where the download animations are white and the final download state is grey or white, depending on API level.

Comment 14

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.