Closed Bug 1201627 Opened 9 years ago Closed 4 years ago

Unify alert_* icons across API levels

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox43 affected)

RESOLVED INCOMPLETE
Tracking Status
firefox43 --- affected

People

(Reporter: nalexander, Unassigned, Mentored)

References

Details

(Whiteboard: [lang=java])

Attachments

(3 files)

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.
mcomella suggests that these can be consolidated, simplifying our lives just that little bit.
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?
Flags: needinfo?(s.kaspari)
Flags: needinfo?(alam)
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.
Flags: needinfo?(alam)
NI self to get those comparison shots.
Flags: needinfo?(michael.l.comella)
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.
Attached image 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)
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
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
Flags: needinfo?(michael.l.comella)
(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).
Flags: needinfo?(s.kaspari)
I'm more concerned about ICS and KitKat's grey icons than GB's if that helps
Flags: needinfo?(alam)
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
Blocks: fatfennec
Mentor: michael.l.comella, s.kaspari
Flags: needinfo?(michael.l.comella)
Whiteboard: [lang=java]
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.
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.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: