Follow-up for Developer HUD toggle

VERIFIED FIXED in Firefox OS v2.1

Status

Firefox OS
Gaia::Settings
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: tchevalier, Assigned: flod)

Tracking

unspecified
2.1 S2 (15aug)

Firefox Tracking Flags

(b2g-v2.0 unaffected, b2g-v2.1 verified)

Details

Attachments

(5 attachments)

Created attachment 8471665 [details]
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.
status-b2g-v2.0: --- → unaffected
status-b2g-v2.1: --- → affected
(Assignee)

Comment 1

4 years ago
Actually it wraps, only too late.

Taking a look into this, since it's a consequence of the bug that lets long labels wrap.
Assignee: nobody → francesco.lodolo
Summary: Follow-up for Developer Tools toggle → Follow-up for Developer HUD toggle
(Assignee)

Comment 2

4 years ago
Created attachment 8471730 [details] [review]
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}}
Attachment #8471730 - Flags: review?(ehung)
(Assignee)

Comment 3

4 years ago
Created attachment 8471731 [details]
Patched Flame

Language set to Italian with fake long string for the switch.
Created attachment 8471757 [details]
French, patch applied

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

Comment 5

4 years ago
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?
Attachment #8471730 - Flags: review?(ehung) → review-

Comment 6

4 years ago
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-.
Attachment #8471730 - Flags: review- → review+

Updated

4 years ago
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 7

4 years ago
(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.
Status: NEW → ASSIGNED
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/a1c6f647f554773bd3872b836f958ef0fef3a14f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-b2g-v2.1: affected → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S2 (15aug)
(Assignee)

Comment 9

4 years ago
Verified on
Gaia      0cd506156fb6f2431d01cf37e6e5db37cea68c07
Gecko     https://hg.mozilla.org/mozilla-central/rev/9eb591a1a203
BuildID   20140814160206
Version   34.0a1
Status: RESOLVED → VERIFIED

Comment 10

4 years ago
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
status-b2g-v2.1: fixed → verified

Comment 11

4 years ago
Created attachment 8534174 [details]
Verified video: verify_1052537.MP4
You need to log in before you can comment on or make changes to this bug.