Peter La Reporter
	Description • 10 years ago

According to the visual spec attached to the original lockscreen visual refresh bug, the icons are white.  The icons in the build currently look grey or semitransparent.  They aren't as bright as they should be.

The original spec:
https://bug950884.bugzilla.mozilla.org/attachment.cgi?id=8427036

	John Lu [:mnjul][Please NI or I'll miss] Assignee
	Comment 1 • 10 years ago

Hi Greg, could you review this for me, and see if you have comments/concerns regarding my approach:

Note that I have only removed the .dark class in class and not darkIcons and lightIcons codes ( https://github.com/mnjul/gaia/blob/bug_1021860_lockscreen_icon_wrong_color/shared/js/lockscreen_slide.js#L1105-L1109 and https://github.com/mnjul/gaia/blob/bug_1021860_lockscreen_icon_wrong_color/shared/js/lockscreen_slide.js#L727-L731 ); the codes are in the shared lockScreenSlider javascript, and in order to "remove" the logics at the moment we can probably only add flags and nothing else.

I will sync with Germán (responsible for callscreen visual refresh) and maybe open a new bug to see how we're going to tidy the codes up.

	bhavana bajaj [:bajaj]
	Comment 2 • 10 years ago

Feel free to seek uplift when ready but this is not a blocker for release.

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 3 • 10 years ago

Comment on attachment 8436687 [details]
Patch

The style change is OK, and we unfortunately need to keep doing workarounds like this until we can finally get rid of such mess (a mess began from a 2K lockscreen.js, and become worse when people want to add more functions on it while the refactoring is on going, not to mention that our unstable testing environment blocked it hurtfully).

	John Lu [:mnjul][Please NI or I'll miss] Assignee
	Comment 4 • 10 years ago

Master: https://github.com/mozilla-b2g/gaia/commit/a9f19ecf715ab7dced311022b31bc974a7a45436

	John Lu [:mnjul][Please NI or I'll miss] Assignee
	Comment 5 • 10 years ago

Comment on attachment 8436687 [details]
Patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]

[Bug caused by] (feature/regressing bug #): bug 1021860 was because of not-to-spec implementation of bug 950884

[User impact] if declined: minimal, as this is only cosmetic change

[Testing completed]: Yes, both Travis and TBPL, with rebase against master just 1.5 hours ago

[Risk to taking this patch] (and alternatives if risky): minimal, as this is only cosmetic change

[String changes made]: None

	bhavana bajaj [:bajaj]
	Comment 6 • 10 years ago

Comment on attachment 8436687 [details]
Patch

low risk, polish change.

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 10 years ago

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