1025845 - [VsD Refresh] Lockscreen Visual Refresh > Media Player widget icons wrong

Status: RESOLVED FIXED

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

Description

[John Lu [:mnjul][Please NI or I'll miss]]

As in bug 1023500 c10 and in the screenshots in that bug, the media player widget icons are wrong (not to spec) when are on HiDPI devices.

Comment 1

[John Lu [:mnjul][Please NI or I'll miss]]

This can be fixed with CSS changes.

Comment 2

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: 2014-06-18-12-42-55.png

Hi Amy,

Please see if the fixed media player widget buttons are of correct sizes. Thanks! ("play" size not shown in screenshot; its size has similarly been fixed as the pause button)

Comment 3

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: Patch (PR @ Github)

Comment 4

[Amy Lee [:amylee] UX]

Comment on attachment 8441847 [details]
2014-06-18-12-42-55.png

Hi John, 

Here are is my feedback:

1. Move play icon 3px right, 2px down
2. Move back and forward icons 2px down
3. Change album name font to italic (2nd text line)
4. Fix highlight states for player controls so they are flushed to the left, right, and bottom edges (see music player app). Icons should be centered to the highlight states. 

I've also attached a screenshot for reference

Thanks!

Comment 5

[Amy Lee [:amylee] UX]

Attachment: lockscreen_music.png

lockscreen music player edits

Comment 6

[howie [:howie]]

Triage: no blocking

Comment 7

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: ls_muswidget_screenshots.png

Hi Amy,

How do the new screenshots look? Thanks!

Comment 8

[Amy Lee [:amylee] UX]

(In reply to John Lu [:mnjul] @MoCoTaipei from comment #7)
> Created attachment 8443403 [details]
> ls_muswidget_screenshots.png
> 
> Hi Amy,
> 
> How do the new screenshots look? Thanks!

Hi John, 

Do you have a patch I can check (I flashed the one in the bug and it seems to still be the old one)? Or can you send me 2 more screenshots of the highlight state for the back and forward controls. Thanks!

Comment 10

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: ls_muswidget_screenshots_2.png

Hi Amy,

Please check the additional screenshots.

Comment 11

[Amy Lee [:amylee] UX]

Attachment: ls_muswidget_screenshots_2.png

Hi John, 

Can you make the highlight state for the back and forward go to the left and right edge of the screen? See attached. Also can you please send me a link to the patch for the next review? Thanks!

Comment 12

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: ls_muswidget_screenshots_3.png

Hi Amy,

Your issues have been addressed; please see if the attached screenshots are correct. The patch in github is https://github.com/mnjul/gaia/tree/bug_1025845_lockscreen_mediaplayer_widget .

Comment 13

[Amy Lee [:amylee] UX]

Comment on attachment 8444995 [details]
ls_muswidget_screenshots_3.png

Hi John, 

From the screenshots I still see a 1px gap at the bottom of the highlight states. Once that is fixed I can + it. Thanks!

Comment 14

[John Lu [:mnjul][Please NI or I'll miss]]

Attachment: ls_muswidget_screenshots_4.png

Here the 1px gap gets eliminated ;)

Comment 15

[Amy Lee [:amylee] UX]

Comment on attachment 8445679 [details]
ls_muswidget_screenshots_4.png

Looks good. Thanks!

Comment 16

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8441848 [details] [review]
Patch (PR @ Github)

Tim, could you review this CSS polish-up patch for me? Thanks.

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Are we uplifting this to 2.0?

Comment 18

[John Lu [:mnjul][Please NI or I'll miss]]

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #17)
> Are we uplifting this to 2.0?
Yes I will request uplift approval

Comment 19

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8441848 [details] [review]
Patch (PR @ Github)

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 1025845 was because the implementation of bug 950884 did not conform to UX spec and was a follow-up bug

[User impact] if declined: Minimal; as this is just visual polish, things would function normally without this patch. However, without this patch, HiDPI devices would have unusually large buttons with media player widget in lockscreen.

[Testing completed]: Yes, both TBPL and Travis.

[Risk to taking this patch] (and alternatives if risky): Minimal -- this patch contains only CSS changes.

[String changes made]: None

Comment 20

[bhavana bajaj [:bajaj]]

Comment on attachment 8441848 [details] [review]
Patch (PR @ Github)

low risk, UI polish, approving for 2.0

Comment 21

[Ryan VanderMeulen [:RyanVM]]

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