1386726 - Alignment of update text in hamburger menu is slightly off in Photon

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Alex Thayer [:alexical] (she/her)]

When an update is available, the "Restart to update Nightly" text is slightly left of where it should be with the new photon hamburger menu.

Comment 1

[Sam Foster [:sfoster] (he/him)]

Can you still reproduce this? I looked a minute ago and the text looked aligned correctly to me (Linux/Nightly).

Comment 2

[Alex Thayer [:alexical] (she/her)]

Attachment: alignment.png

Yup - see attached.

Comment 4

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1386726 - Fix alignment of banner item labels in the hamburger menu.

The start padding should match the 12px defined for nearby toolbarbuttons:
.subview-subheader, panelview .toolbarbutton-1, .subviewbutton, .widget-overflow-list .toolbarbutton-1

Review commit: https://reviewboard.mozilla.org/r/181022/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/181022/

Comment 5

[Sam Foster [:sfoster] (he/him)]

Looks like the padding we're seeing on the other items is defined here: http://searchfox.org/mozilla-central/source/browser/themes/shared/customizableui/panelUI.inc.css#1208, so this patch just matches that value of 12px for the start padding.

Comment 6

[:Gijs (he/him)]

Comment on attachment 8909545 [details]
Bug 1386726 - Fix alignment of banner item labels in the hamburger menu.

https://reviewboard.mozilla.org/r/181022/#review186436

::: browser/themes/shared/customizableui/panelUI.inc.css:780
(Diff revision 1)
>  /* in Photon, we have a bottom border as well. Reconcile with the above rule
>   * after photon launch. */
>  #appMenu-mainView > .panel-subview-body > .panel-banner-item {
>    border-bottom: 1px solid var(--panel-separator-color);
>    margin-bottom: 3px;
> -  padding-inline-start: 10px;
> +  padding-inline-start: 12px;

This doesn't look right on OS X. I think it needs to be 18px to match http://searchfox.org/mozilla-central/source/browser/themes/osx/customizableui/panelUI.css#15-17 there.

It also looks to me like the ::after update icon needs adjusting (this rule: http://searchfox.org/mozilla-central/source/browser/themes/shared/customizableui/panelUI.inc.css#1373-1376 ) on both Windows and OS X.

Comment 7

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8909545 [details]
Bug 1386726 - Fix alignment of banner item labels in the hamburger menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/181022/diff/1-2/

Comment 8

[Sam Foster [:sfoster] (he/him)]

(In reply to :Gijs from comment #6)
> This doesn't look right on OS X. I think it needs to be 18px to match
> http://searchfox.org/mozilla-central/source/browser/themes/osx/
> customizableui/panelUI.css#15-17 there.

I added an override, not sure if using #appMenu-mainView > ... is overly specific?

Comment 9

[:Gijs (he/him)]

Comment on attachment 8909545 [details]
Bug 1386726 - Fix alignment of banner item labels in the hamburger menu.

https://reviewboard.mozilla.org/r/181022/#review186690

LGTM. I think that selector is fine like this. Thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2bd5e6f26886
Fix alignment of banner item labels in the hamburger menu. r=Gijs

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2bd5e6f26886

Comment 12

[Azmina]

I have reproduced this bug with Nightly 57.0a1 (2017-08-02) on Windows 8.1, 64 Bit!

This bug's fix is verified on Latest Nightly 57.0a1.

Build ID : 20170920100426
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0

Comment 13

[Timea Zsoldos [:tzsoldos], Desktop QA]

I have reproduced this issue using Firefox  57.0a1 (2017.08.02) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 58.0a1 (2017.10.01) on Win 8.1 x64, Mac OS X 10.11 and Ubuntu 14.04 x64.