Closed
Bug 1062334
Opened 10 years ago
Closed 10 years ago
[Notifications] Update count indicator to #25dbff
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)
People
(Reporter: epang, Assigned: apastor)
References
Details
(Keywords: polish, Whiteboard: [systemsfe])
Attachments
(2 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
text/x-github-pull-request
|
mikehenrty
:
review+
epang
:
ui-review+
fabrice
:
approval-gaia-v2.1+
|
Details |
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!
Reporter | ||
Comment 1•10 years ago
|
||
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!
Flags: needinfo?(apastor)
Reporter | ||
Updated•10 years ago
|
Attachment #8487216 -
Attachment description: [Visual Spec] AmbientI ndicator Notification → [Visual Spec] Ambient Indicator Notification
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8487615 -
Flags: ui-review?(epang)
Flags: needinfo?(apastor)
Assignee | ||
Comment 3•10 years ago
|
||
[Blocking Requested - why for this release]: The current color of the ambient indicator is not visible enough, depending on the brightness of the screen
blocking-b2g: --- → 2.1?
Updated•10 years ago
|
Target Milestone: --- → 2.1 S4 (12sep)
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8487615 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23927
Looks good to me! Thanks for updating :).
Attachment #8487615 -
Flags: ui-review?(epang) → ui-review+
Assignee | ||
Updated•10 years ago
|
Attachment #8487615 -
Flags: review?(mhenretty)
Assignee | ||
Updated•10 years ago
|
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Assignee | ||
Updated•10 years ago
|
Comment 6•10 years ago
|
||
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?
Flags: needinfo?(apastor)
Assignee | ||
Comment 7•10 years ago
|
||
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.
Flags: needinfo?(apastor)
Comment 8•10 years ago
|
||
Comment on attachment 8487615 [details]
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.
Attachment #8487615 -
Flags: review?(mhenretty) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8487615 [details]
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]: -
Attachment #8487615 -
Flags: approval-gaia-v2.1?(fabrice)
Comment 11•10 years ago
|
||
(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...
Assignee | ||
Comment 12•10 years ago
|
||
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.
Updated•10 years ago
|
Attachment #8487615 -
Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Assignee | ||
Comment 15•10 years ago
|
||
I don't think we need to cover this color change with integration tests.
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•