808250 - In Display settings, the icons for brightness slider seem flipped to people

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect, P4)

Description

[Larissa Co [:lco]]

Attachment: Brightness slider

In general feedback, people were not sure which icon dimmed the screen and which icon made it brighter. Currently, the left icon dims the screen, but it looks visually "brighter" because it's lighter in color and is hollow. The right icon increases brightness, but it looks visually "darker" because it's darker in color and is filled in.

Comment 1

[Eric Pang [:epang] UX]

Attachment: Brightness Icons

why don't we use the same icon twice, but change the size to signify increasing/decreasing the brightness. I can export the icons if needed.

Comment 2

[Eric Pang [:epang] UX]

Attachment: Brightness Mock Ups

We have 2 solutions to the issue (labelled on mock up):

1. Sun icon with number - This solution is preferred if possible.

2. Brightness adjustment with Moon (lower) and Sun (higher) icons.  Only if solution one is not possible.

Comment 3

[Eric Pang [:epang] UX]

Attachment: Icons for brightness adjustment

Comment 4

[Larissa Co [:lco]]

I don't know if we decided on #2, but I did like #1 better. #1 Has a slight problem of people not thinking of brightness as a number though.

With #2, I am slightly worried that people will interpret this as day or night mode, and think the screen would be brighter in night mode :P

Anyway, this isn't a blocker for me. If people decided while I was away, I'm ok with that.

Comment 5

[Eric Pang [:epang] UX]

Attachment: Icons for brightness adjustment

Updated the higher brightness icon to match the one in settings.

Comment 6

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch

Comment 7

[Sam Joch [:samjoch]]

Landed in UX Branch https://github.com/gordonbrander/gaia/commit/546d87666adf3c22eb66d7e7629654afe7487da7

Comment 8

[Sam Joch [:samjoch]]

Here is the js part to fix the bug. 
Landed in UX Branch: https://github.com/gordonbrander/gaia/commit/f2abb84a855e020330257578128a6a6d6ac650a4

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/812bd047c5c70794d1a0d481047e6578db0a1ff2

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I verified the follow up patch, it will still need to be pushed : 
https://github.com/gordonbrander/gaia/commit/f2abb84a855e020330257578128a6a6d6ac650a4

Comment 11

[Sam Joch [:samjoch]]

Vivien,

Just to ping you that the fix has been PRd with the UX weekly commits.

https://github.com/mozilla-b2g/gaia/pull/7704

;-)

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/5cb6a64dbf643e2c8774f4aadedd57a6e0d6dafb

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Master build : 2013-03-25-03-07-06
"mozilla-central" revision="bef19bca23f9"
"integration/gaia-central" revision="8c401e09a6a9"
"gecko.git"revision="bbaac6bbbf5084ef1a3e178ced676ea77db8b24b"
"gaia.git" revision="a1259a6c80629daa458428d6a96403841aaab0f0"

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/dec694d585e1
Gaia   5a31a56b96a8fc559232d35dabf20411b9c2ca1d
BuildID 20130325070203
Version 18.0
Unagi

Comment 14

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Mistakenly set this to verify for master:

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/dec694d585e1
Gaia   5a31a56b96a8fc559232d35dabf20411b9c2ca1d
BuildID 20130325070203
Version 18.0
unagi 

Can't verify in master due to bug 854138

Comment 15

[Eric Pang [:epang] UX]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #14)
> Mistakenly set this to verify for master:
> 
> Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/dec694d585e1
> Gaia   5a31a56b96a8fc559232d35dabf20411b9c2ca1d
> BuildID 20130325070203
> Version 18.0
> unagi 
> 
> Can't verify in master due to bug 854138

Hi Naoki, what's holding this bug from being verified?

Comment 16

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

The previous hold up was that there was a bug that caused the settings panel not to show.  

Currently, it's regressed and I'm showing a difference between master and v1 train.  bug 858698

Comment 17

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Side note : sliders are not working correctly in master : bug 854469

Comment 18

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Figured out why it's showing the moon on today's version versus the unfilled sun.  Today's version isn't using the @2x.png : 
https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/style/images/range_icons_brightness_min%402x.png

Can we get that updated please?

The non-hidpi is showing correctly : https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/style/images/range_icons_brightness_min.png

Comment 19

[Eric Pang [:epang] UX]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #18)
> Figured out why it's showing the moon on today's version versus the unfilled
> sun.  Today's version isn't using the @2x.png : 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/style/images/
> range_icons_brightness_min%402x.png
> 
> Can we get that updated please?
> 
> The non-hidpi is showing correctly :
> https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/style/images/
> range_icons_brightness_min.png

Hi Naoki, just need a little clarification on what's needed.  Is it the @2x pngs of the sun and moon?  Let me know and I'll attached and flag Pavel to help fix. Thanks!

Comment 20

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Hi epang, just @x2 version of the moon.

Comment 21

[Eric Pang [:epang] UX]

Attachment: @2x Moon Icon (lower brightness)

Hi Naoki, I've attached the @2x Moon icon :)

Comment 22

[Eric Pang [:epang] UX]

Pavel, can you update this icon on the hdpi version? Thanks!

Comment 23

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia repo

Comment 24

[Sam Joch [:samjoch]]

Comment on attachment 737872 [details]
patch for Gaia repo

Followup landed in master https://github.com/mozilla-b2g/gaia/commit/b65dbaace1bfc940db6f10aee6ad3c54187d25aa

Comment 25

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified on:
## Environment :
BuildID : 2013-04-25-03-09-50
"mozilla-central" revision="690b5e0f6562"
"gecko.git" revision="33f9b3898b66dfb95874866269463d4b6bc749a0"
Unagi

Comment 26

[Sam Joch [:samjoch]]

Comment on attachment 737872 [details]
patch for Gaia repo

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 #): Slider thumb are not rendering well.
User impact if declined: May see broken slider
Testing completed: Yes 
Risk to taking this patch (and alternatives if risky): low, assets updates and css rules redefine
String or UUID changes made by this patch: -

Comment 27

[Eric Pang [:epang] UX]

Sam, has this been uplifted to V1 Train?  If not, what needs to be done to have this uplifted?

Comment 28

[Sam Joch [:samjoch]]

I just checked that: 

- 812bd047c5c70794d1a0d481047e6578db0a1ff2 already in v1-train
- 5cb6a64dbf643e2c8774f4aadedd57a6e0d6dafb already in v1-train too

Uplifed b65dbaace1bfc940db6f10aee6ad3c54187d25aa -> 64b1a7b339acb21c3f074184a4e7284953856647 in v1-train

I notice that the js fix of this bug has been fixed here https://bugzilla.mozilla.org/show_bug.cgi?id=830782.