1062334 - [Notifications] Update count indicator to #25dbff

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Eric Pang [:epang] UX]

Hi Alberto,

Can you help update the notification counter to #25dbff.
The reason I think this update is needed is because it disappears on light screens such as settings.

Please let me know if you have any questions.

Thanks!

Comment 1

[Eric Pang [:epang] UX]

Attachment: [Visual Spec] Ambient Indicator Notification

Hey Alberto, 

I've updated the spec, this time the colours update for both the counter and bar.

When the notification counter fades in the bar also fades to #006e86.
The new colour for the counter is #00d3ff.

The details are in the spec.  
Also, here on box
https://mozilla.box.com/s/kkhu56gz8tzr1z7y1sj0

Let me know if you have any questions!

Comment 2

[Alberto Pastor [:albertopq]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23927

Comment 3

[Alberto Pastor [:albertopq]]

[Blocking Requested - why for this release]: The current color of the ambient indicator is not visible enough, depending on the brightness of the screen

Comment 4

[Gregor Wagner [:gwagner]]

Broken feature.

Comment 5

[Eric Pang [:epang] UX]

Comment on attachment 8487615 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23927

Looks good to me!  Thanks for updating :).

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

I'm confused, this bug seems to be about the color of the ambient indicator. Why are there changes to the logic of when we update the indicator?

Comment 7

[Alberto Pastor [:albertopq]]

Yeah, the thing is that now we need to animate changing the color at one specific moment, so we need to updateIndicator when the toast timesout (or when the user dismisses). I needed to add the logic for handling that.

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8487615 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23927

Ah, makes sense. Let's add a comment explaining why we don't update ambient indicator in addNotification, and also let's add a unit test for that.

Comment 9

[Alberto Pastor [:albertopq]]

master: https://github.com/mozilla-b2g/gaia/commit/82edbbebc4ee9e2b5bee4f93f988d7fd3bd8d673

Comment 10

[Alberto Pastor [:albertopq]]

Comment on attachment 8487615 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23927

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 996044
[User impact] if declined: The counter is not visible enough depending on the brightness
[Testing completed]: Manual testing. Ui review
[Risk to taking this patch] (and alternatives if risky): Low risk, just changing color
[String changes made]: -

Comment 11

[[:fabrice] Fabrice Desré]

(In reply to Alberto Pastor [:albertopq] from comment #10)

> [Testing completed]: Manual testing. Ui review

The test you added doesn't matter here?

> [Risk to taking this patch] (and alternatives if risky): Low risk, just
> changing color

Nope, there is some logic change too...

Comment 12

[Alberto Pastor [:albertopq]]

Yeah sorry, I mixed bugs...

[Testing completed]: Manual testing and Ui review. Added unit tests for the logic on skipping the indicator update when adding a new unread notification.

[Risk to taking this patch] (and alternatives if risky): Given the new design, we need to animate the change of color exactly when the toast timesout (or the user dismiss it). The logic for doing that is covered by unit tests, and the previous UI tests for adding/dimissing notifications are still covering this, so I would say the patch is save.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/ea64187cf8116caf6f383b2bb6aa2e331ddd708a

Comment 14

[[:RJ]]

this issue can be verified on Flame 2.2 Master KK (319mb) (Full Flash) and 2.1 KK (319mb) (Full Flash)

this issue of the ambient notifications bar has been changed from the old color #00d3ff to the new updated color of #25dbff

Flame 2.2 Master KK (319mb) (Full Flash)

Device: Flame 2.2 Master
BuildID: 20141011040204
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

2.1 KK (319mb) (Full Flash)

Environmental Variables:
Device: Flame 2.1 KK (319mb) (Full Flash)
Build ID: 20141010000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Version: 34.0a2 Flame 2.1 KK (319mb)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 15

[Alberto Pastor [:albertopq]]

I don't think we need to cover this color change with integration tests.