1052537 - Follow-up for Developer HUD toggle

Status: VERIFIED FIXED

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

Description

[Théo Chevalier]

Attachment: Screenshot Flame 2.1, French

Bug 1033392 introduced a new toggle button in Settings > Developer > Developer HUD, however it introduced some issues as well:

- The label now overlaps with the toggle button instead of going on a new line (see bug 892700), like other labels. See attached screenshot in French for instance, the last character ("t") is being hidden by the toggle.

- The string "hud-devtools" is used for both the label and the header, we should use two strings instead, see https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_best_practices#Don%27t_reuse_strings_in_different_contexts

- The string "hud" is not used anymore, hence it should be removed.

Comment 1

[Francesco Lodolo [:flod]]

Actually it wraps, only too late.

Taking a look into this, since it's a consequence of the bug that lets long labels wrap.

Comment 2

[Francesco Lodolo [:flod]]

Attachment: Pull request on Github

CSS: patch restores the right padding used in other parts of Gaia (9 rem) for checkboxes. Since rules added in bug 892700 are more specific, padding was wrongly overriden.

Also removes the unused 'hud' string. Unfortunately it requires to change the two strings referencing {{hud}}

Comment 3

[Francesco Lodolo [:flod]]

Attachment: Patched Flame

Language set to Italian with fake long string for the switch.

Comment 4

[Théo Chevalier]

Attachment: French, patch applied

Tried with French, looks good to me as well.
Thanks Flod!

Comment 5

[Jan Keromnes [:janx]]

Comment on attachment 8471730 [details] [review]
Pull request on Github

Thanks a lot for fixing the HUD! Stealing the review because the problem is in my code.

Mostly looks good, but r- because the `label.pack-switch` rule should be in apps/settings/style/lists.css and not settings.css. Also, there already is a similar rule in lists.css: https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/style/lists.css#L252 any clue why it doesn't apply?

Comment 6

[Jan Keromnes [:janx]]

Comment on attachment 8471730 [details] [review]
Pull request on Github

Actually I reviewed way too quickly, and didn't know about the changes you did in bug 892700. In their regard your current patch makes sense, sorry about the r-.

Comment 7

[Francesco Lodolo [:flod]]

(In reply to Jan Keromnes [:janx] from comment #6)
> Actually I reviewed way too quickly, and didn't know about the changes you
> did in bug 892700. In their regard your current patch makes sense, sorry
> about the r-.

No problem :-)

I basically kept them together in settings.css to make them easier to manage and find, since they're overrides specific for these 2 panels.

I see a couple of unrelated errors in test results, the rest is green. Setting keyword for checkin.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Francesco Lodolo [:flod]]

Verified on
Gaia      0cd506156fb6f2431d01cf37e6e5db37cea68c07
Gecko     https://hg.mozilla.org/mozilla-central/rev/9eb591a1a203
BuildID   20140814160206
Version   34.0a1

Comment 10

[SandKing]

This issue has been verified successfully on Flame2.1
Verify video:"verify_1052537.mp4".

Flame 2.1 build:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880

Comment 11

[SandKing]

Attachment: Verified video: verify_1052537.MP4