918551 - Australis menu panel alignment issues

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: Screenshot

Items in the Australis menu panel seem to be misaligned. (This is on Retina, which may or may not be related.)

In attached screenshot:

* Zoom controls (-/100%/+) appear to be properly aligned. The - and + are centered in their buttons, and both are the same size.

* The Help / Customize / Quit are oddly aligned. They don't align with the items above (which I can see being intentional, to break up the grid), but they still seem to be haphazardly placed. I suspect they're just picking an inherent size from the label lengths + flex, so they're all different (with Help being narrowest).

* The hilight box (around Preferences, here) is misaligned. Both purple boxes are the same size, so the icon is offset to the right relative to its hilight box.

* The main 3x3 icon grid seems to be offset ~4 pixels to the right compared to the zoom buttons, or vice-versa. (Check my pixel math, though)

Comment 1

[Justin Dolske [:Dolske]]

(This is on a current 9/19 UX Nightly)

Comment 2

[:Gijs (he/him)]

The box being off compared to the image also means that the text isn't centered compared to the icons.

Comment 3

[u428464]

Maybe changing the wide widgets styling would help resolve this (bug 878065)

Comment 4

[:Gijs (he/him)]

So at least some of this is because icons have a ~4.5 pixel left margin, and a 0 pixel right margin. Removing the 0 margin rule (so there's 5em / 12 margin on both sides) fixes the highlight box, the alignment of text to icons, and the icon grid offset vis-a-vis the zoom buttons.  Waiting on Marco's answer in bug 895938 on whether we can remove this outright and/or how he intended this rule to work.

Regarding the bottom row, I'm not sure what we should be doing. We could fix the size of some/all of the items, but I'm afraid of ending up with too little space for non-en-US localizations.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Note that these alignment issues are somewhat amplified if the menu panel anchor is near the edge of the screen, via bug 874792.

Comment 6

[Justin Dolske [:Dolske]]

Attachment: Hack

I did a little fiddling in DOMI... From top to bottom:

1) Original (current UX)

2) Changed label to "Quit Firefox". The longer string makes the alignment even more crooked, since the short "Help" is no longer balanced by a nearly-as-short "Quit UX".

3) Changed flex-basis on for the buttons from "auto" to "33.3%". Buttons are now aligned, but... Labels are badly cropped.

4) Removed padding-left and padding-right from buttons (was 10px). This seems not-terrible.

I don't know why the panel got a few pixels wider in #2 and #3.

Two other observations:

* On Windows, the label is simply "Exit", instead of "Exit Firefox". Should we be consistent with the product name? No sure if platform-convention for the File menu needs to hold here. (Although it's fair to keep the different Quit/Exit.)

* The menu panel on Windows is significantly wider. Should it be so different? To me, OS X still feels a bit cramped while Windows feels like a bit too much whitespace.

Comment 7

[Justin Dolske [:Dolske]]

Attachment: Windows vs OS X panel width

Comment 8

[Justin Dolske [:Dolske]]

Attachment: PoC patch

This fixes the buttons at the bottom, and the alignment of the other buttons. But as Gijs noted in comment 4, bug 895938 added some broken style here, I just removed it without checking to see what impact it has on that bug. (Also, what's up with the other -moz-margin-end I XXX'd?)

Comment 9

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #8)
> Created attachment 812927 [details] [diff] [review]
> PoC patch
> 
> This fixes the buttons at the bottom, and the alignment of the other
> buttons. But as Gijs noted in comment 4, bug 895938 added some broken style
> here, I just removed it without checking to see what impact it has on that
> bug. (Also, what's up with the other -moz-margin-end I XXX'd?)

Not 100% sure, but I'm guessing it's to get the spacing between the icon and the label right for the edit buttons. But then, no idea why that needs to happen for the zoom control, too... Mike (de Boer)?

(In reply to Justin Dolske [:Dolske] from comment #6)
> Created attachment 812403 [details]
> Hack
> 
> 4) Removed padding-left and padding-right from buttons (was 10px). This
> seems not-terrible.

Does this still look OK when hovered? I suspect it needs *some* padding to get the label to not edge up to the button border...

Also, I'm wondering if we should get these buttons to have the wrapping labels (like the ones in the main content area) so that if German ends up being "Firefox abschließen" or something, it wraps to fit instead of doing something more crazy.

> I don't know why the panel got a few pixels wider in #2 and #3.

My suspicion, at least: panels on OS X are finicky and get resized when you open them next to the edge of the screen. :-(

> Two other observations:
> 
> * On Windows, the label is simply "Exit", instead of "Exit Firefox". Should
> we be consistent with the product name? No sure if platform-convention for
> the File menu needs to hold here. (Although it's fair to keep the different
> Quit/Exit.)

I would agree, although I wish we had a more dangerous-looking icon. I get that the symmetry with the Customize button is pleasing, but it really should give a more clear indication that this will kill the entire app, not just the panel menu. OTOH, if people really don't see that coming, I guess they'll learn quickly...

> * The menu panel on Windows is significantly wider. Should it be so
> different? To me, OS X still feels a bit cramped while Windows feels like a
> bit too much whitespace.

I think this is because of the scrollbar width and... stuff. Other Mike (i.e. Conley) knows more here. See bug 861703.

Comment 10

[:Gijs (he/him)]

Attachment: improve icon-label alignment in menupanel, rs=mak

Marco says this will be OK ( https://bugzilla.mozilla.org/show_bug.cgi?id=895938#c27 ), so landing this in a bit.

Comment 11

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/f20417eb9e12

Comment 12

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Gijs Kruitbosch from comment #9)
> 
> I think this is because of the scrollbar width and... stuff. Other Mike
> (i.e. Conley) knows more here. See bug 861703.

Yep, I believe you're right.

If you enable perma-scrollbars on OS X, you get something more like this:

http://i.imgur.com/iTNYvCq.png

This is because the menu panel code adds padding to either side of the grid equal to half of the sampled scrollbar size (plus some fudge factor), to make sure that if a scrollbar shows up, we don't accidentally cause our 3 columns to reduce to 2.

But OS X's hide-y scrollbars sample at 0 width. So we're going to need to solve that somehow...

Comment 13

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #12)
> (In reply to :Gijs Kruitbosch from comment #9)
> > 
> > I think this is because of the scrollbar width and... stuff. Other Mike
> > (i.e. Conley) knows more here. See bug 861703.
> 
> Yep, I believe you're right.
> 
> If you enable perma-scrollbars on OS X, you get something more like this:
> 
> http://i.imgur.com/iTNYvCq.png
> 
> This is because the menu panel code adds padding to either side of the grid
> equal to half of the sampled scrollbar size (plus some fudge factor), to
> make sure that if a scrollbar shows up, we don't accidentally cause our 3
> columns to reduce to 2.
> 
> But OS X's hide-y scrollbars sample at 0 width. So we're going to need to
> solve that somehow...

I wonder if we should just hardcode an increased width on OS X to make it not feel cramped. We could theoretically also increase the panel's width if it's scrolling, and then we can sample the scrollbars themselves, to make Windows not be that wide by default? Is that a reasonable or a terrible idea? :-)

Comment 14

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs Kruitbosch from comment #9)
> > This fixes the buttons at the bottom, and the alignment of the other
> > buttons. But as Gijs noted in comment 4, bug 895938 added some broken style
> > here, I just removed it without checking to see what impact it has on that
> > bug. (Also, what's up with the other -moz-margin-end I XXX'd?)
> 
> Not 100% sure, but I'm guessing it's to get the spacing between the icon and
> the label right for the edit buttons. But then, no idea why that needs to
> happen for the zoom control, too... Mike (de Boer)?

Well, having `-moz-margin-end: 0` AND `margin: 0` there seems a bit superfluous. I must've put it there because icon alignment was broken. However, that might not at all be the case anymore. In fact, I think it's good to remove the rules and carry on.

Comment 15

[:Gijs (he/him)]

Attachment: remove superfluous margin-end reset,

We can't remove the margin: 0, at least on OS X, because otherwise the buttons become much too high, but the -moz-margin-end is indeed superfluous.

Comment 16

[Mike de Boer [:mikedeboer]]

Comment on attachment 817142 [details] [diff] [review]
remove superfluous margin-end reset,

Review of attachment 817142 [details] [diff] [review]:
-----------------------------------------------------------------

I think rs=me would suffice even ;)

Comment 17

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/d6e1bf3ad3bd

Comment 18

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Gijs Kruitbosch from comment #13)
> I wonder if we should just hardcode an increased width on OS X to make it
> not feel cramped. We could theoretically also increase the panel's width if
> it's scrolling, and then we can sample the scrollbars themselves, to make
> Windows not be that wide by default? Is that a reasonable or a terrible
> idea? :-)

You have to be a bit careful doing any sort of adjustment or detection when scrollbars show up. I ran into that a few times - I had overflow and underflow handlers for the grid to do width adjustments, but as soon as I overflowed, my overflow handler would adjust the width so as to cause an underflow, which would fire my underflow handler which would make an adjustment that made us overflow and...and infinite loop.

So that's a thing to be aware of. :)

The trouble with the scrollbars on OS X is that sometimes they're at width = 0, because they're overlay scrollbars. I think what'd be ideal is if we could sample the scrollbar width on that hidden window and force non-overlay scrollbars...not sure if that's possible. spohl might know.

But just some additional padding on either side isn't a bad idea either.

Comment 19

[:Gijs (he/him)]

Attachment: divide the footer in 3, drop brand name in panel,

This is basically the remainder of Justin's fix, plus removing the specialcasing of OS X for the panel menu. The product name doesn't need to be included here. Justin, I've stuck this under your name as you wrote the important parts of this... let me know if that's not OK (this would be a f? or needinfo? if bzexport didn't not have those).

Comment 20

[:Gijs (he/him)]

Comment on attachment 817195 [details] [diff] [review]
divide the footer in 3, drop brand name in panel,

https://hg.mozilla.org/projects/ux/rev/85a33a8d95ef

Comment 21

[:Gijs (he/him)]

Attachment: add a minimum size left for scrollbars, so that OS X doesn't make us look cramped,

So let's at least add a minimum width here so we're not so cramped on OS X. FWIW, permanent scrollbars on OS X have a scrollWidth as computed here of 15. 10 seemed nice to me because it means we have 10px padding, AIUI, as the side padding is 5px already, and 10px padding there matches the spec if memory serves me. Sound like a plan? We can add a lower-prio followup to mess with this logic more, but I suspect that dynamically changing the size of the panel after it's been opened is another can of worms, and I think after this patch we're in a good enough state to call this bug fixed.

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 817287 [details] [diff] [review]
add a minimum size left for scrollbars, so that OS X doesn't make us look cramped,

Review of attachment 817287 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, let's do this. Thanks Gijs. :)

Comment 23

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/9296e86e4d72

Comment 24

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/f20417eb9e12
https://hg.mozilla.org/mozilla-central/rev/d6e1bf3ad3bd
https://hg.mozilla.org/mozilla-central/rev/85a33a8d95ef
https://hg.mozilla.org/mozilla-central/rev/9296e86e4d72