589435 - Style Firefox button's submenus

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[u88484]

The Firefox button's submenus still have the same old plain menu styling while the main menu is purdy.  i6 of the mockup shows the menus with the plain styling but I think they should be styled like the main menu for consistency sake and to add some eye candy.  Look at the screenshot and notice how ugly the submenu's look on top of the main menu.

Comment 1

[Josh Tumath]

For what it's worth, I think it would look better if we left it as it is now with the default style.

Comment 2

[u88484]

Attachment: Screenshot

Comment 3

[u88484]

Should submenus spawned from the left pane be white and from the right pane be the bluish color, or both white?  Faaborg, shorlander?

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

>For what it's worth, I think it would look better if we left it as it is now
>with the default style.

yeah, the plan was to maintain the default style.  It doesn't mesh incredibly well with the main menu, but I think it makes sense in the context of the overall OS. I'm worried that if we create submenus unlike anywhere else on the system we'll start to feel uncanny. (we got into this situation since the normal approach of changing the contents of the panes is really inefficient for mouse distance).

>Should submenus spawned from the left pane be white and from the right pane be
>the bluish color, or both white?  Faaborg, shorlander?

I'm going to toss the question over to Stephen

Comment 5

[Asa Dotzler [:asa]]

Attachment: here's the menu attached to the shut down button in the start menu

I propose we go with the minor style change you see in the sub-menu hanging off of the shut-down menubutton in the start menu.  It's only a minor color difference and border difference but it helps smooth out the jarring-ness of the two menu types side by side

Comment 6

[:Margaret Leibovic]

Has there been any decision on this? Does it need to block?

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Yes, it needs to block. It's all the way ugly. Needs input from Shorlander if not Faaborg.

Comment 8

[:Margaret Leibovic]

Okay, I can take it then. I just need to know how it should look :)

Comment 9

[Alex Faaborg [:faaborg] (Firefox UX)]

Stephen: the platform doesn't really give us much to go on for combining direct sub-menus with a ribbon style main menu.  I was thinking using white backgrounds for the sub-menus created on the left side, and blue for the ones created on the right side (to further focus on the actions versus places distinction).  Did you have any other ideas?  Alternatively we could just make all of them white, since that goes with everything.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #9)
> Alternatively we could just make all of them white, since that goes with 
> everything.

Not after labour day it doesn't!

Comment 11

[Stephen Horlander [:shorlander]]

(In reply to comment #10)
> (In reply to comment #9)
> > Alternatively we could just make all of them white, since that goes with 
> > everything.
> 
> Not after labour day it doesn't!

Well we certainly don't want to commit a fashion faux pas with our menus!

Coloring the menus blue or white based on which side they originate from seems like it could be rather strange to me. However without seeing it in action it is hard to say.

I will mockup a few ideas.

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Matching the respective pane color

Quick mockup of sub-menus that would match the background color of the pane they are originating from.  I think this would generally work since you can only see one of them at a time, and it might potentially slightly reinforce that items on the left carry out an action while items on the right (generally) take you to a location in the UI or on the Web.

Comment 13

[:Margaret Leibovic]

Attachment: wip

Setting -moz-appearance: none; on the menupopup gets rid of the vertical line in the menupopup, and I'm having trouble figuring out what I need to do to get that back.

Comment 14

[:Margaret Leibovic]

Attachment: patch

I ended up using a gradient as the background to reproduce the gutter.

Comment 15

[:Margaret Leibovic]

Attachment: screenshot

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 491063 [details] [diff] [review]
patch

>+  #appmenuPrimaryPane menupopup {

Worrisome selector, I count 112 menupopups in my window.

>+    background-image: -moz-linear-gradient(left, white 26px, threedshadow 26px,
>+                                                 threedhighlight 27px, white 28px);

This draws the line behind the menu item labels on XP. It probably shouldn't be there at all on XP.

Comment 17

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 491064 [details]
screenshot

nit: padding is a little off on the left side of the checked items compared to native menus.  With that fixed we can land this for feedback.

Comment 18

[:Margaret Leibovic]

(In reply to comment #16)
> Comment on attachment 491063 [details] [diff] [review]
> patch
> 
> >+  #appmenuPrimaryPane menupopup {
> 
> Worrisome selector, I count 112 menupopups in my window.

Yeah, I was thinking that as I wrote it, but I wasn't sure of a better way to select all the submenus. In particular, there can be many levels of submenus under the Web Developer menu and the History/Bookmarks menus. Should I make different child selectors for each of the possible submenus? I feel like that could be error-prone. It would be nice if I could add a class to the menupopups, but I don't know how that would work for the dynamically generated menupopups.

Comment 19

[:Margaret Leibovic]

Attachment: screenshot v2

All sides of the menupopup needed padding to match the native theme. I also got rid of the gutter on XP.

I'm still not sure what to do about the selector to make it more efficient :(

Comment 20

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 491335 [details]
screenshot v2

just discussed in person, we are going to go for hard coded colors for the vertical line as well.  On 7 the colors to match are #e2e3e3 and #ffffff

line should be 28px from the edge, and connect with the horizontal line seperators.

Comment 21

[Dão Gottwald [:dao]]

You probably want ThreeDLightShadow and ThreeDHighlight.

Comment 22

[:Margaret Leibovic]

Attachment: patch v2

To avoid using the descendant selector, I added an onpopupshowing handler to the popup that figures out how the popup should be styled based on its ancestors. I'm not sure if this is more efficient, but it seemed like it could be a potential solution.

I also made image files to use for the background instead of the gradients because Alex and I discovered that the gradient colors were blending into each other, preventing us from rendering the colors we wanted. I also used hard-coded color values for the aero border color because ThreeDShadow was not matching the color used for the native menu borders.

Comment 23

[:Margaret Leibovic]

Attachment: screenshots: native, primary pane, secondary pane (aero and luna)

Comment 24

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 492428 [details]
screenshots: native, primary pane, secondary pane (aero and luna)

small nit: 30px in the print sub menu from the left edge to the gutter instead of 28, perhaps the icon is expanding it?

Comment 25

[Dão Gottwald [:dao]]

Comment on attachment 492427 [details] [diff] [review]
patch v2

(In reply to comment #22)
> To avoid using the descendant selector, I added an onpopupshowing handler to
> the popup that figures out how the popup should be styled based on its
> ancestors. I'm not sure if this is more efficient, but it seemed like it could
> be a potential solution.

I'm doubtful, and adding that code to the toolkit binding certainly seems suboptimal to me. Let's just use the descendant selector.

> I also made image files to use for the background instead of the gradients
> because Alex and I discovered that the gradient colors were blending into each
> other, preventing us from rendering the colors we wanted.

This sounds like you didn't specify the color stops correctly. You'll also need to mirror it for RTL, btw.

> I also used
> hard-coded color values for the aero border color because ThreeDShadow was not
> matching the color used for the native menu borders.

See comment 21.

Comment 26

[:Margaret Leibovic]

(In reply to comment #25)
> > I also used
> > hard-coded color values for the aero border color because ThreeDShadow was not
> > matching the color used for the native menu borders.
> 
> See comment 21.

ThreeDLightShadow is too light for the border. ThreeDShadow is closer, but it's not the same as the native menupopup borders. However, they are very similar, so for the ease of maintaining this in the future, maybe we should just use ThreeDShadow. It's up to Alex. (ThreeDShadow is #a0a0a0 and the native menu popup borders are #979797.)

Comment 27

[:Margaret Leibovic]

Attachment: patch v3

This patch uses ThreeDShadow, but I can change it back to the hard-coded value if that's what Alex wants.

I tried messing around with the gradients more, but I couldn't get the correct colors. I could get the right color to appear in the middle of a wider gradient, but it seems like gradients don't support rendering a 1px wide line of the color of your choice.

Comment 28

[Alex Faaborg [:faaborg] (Firefox UX)]

Everything else in the sub-menu is a hard coded color, so I think we should just hard code to the border as well so that we can get the native shade.

Comment 29

[Dão Gottwald [:dao]]

(In reply to comment #26)
> ThreeDLightShadow is too light for the border. ThreeDShadow is closer, but it's
> not the same as the native menupopup borders. However, they are very similar,
> so for the ease of maintaining this in the future, maybe we should just use
> ThreeDShadow. It's up to Alex. (ThreeDShadow is #a0a0a0 and the native menu
> popup borders are #979797.)

Oh, I meant the separator, not the border, when I was talking about ThreeDLightShadow.

(In reply to comment #27)
> I tried messing around with the gradients more, but I couldn't get the correct
> colors. I could get the right color to appear in the middle of a wider
> gradient, but it seems like gradients don't support rendering a 1px wide line
> of the color of your choice.

They should, please let me see the code.

Comment 30

[Dão Gottwald [:dao]]

Comment on attachment 492772 [details] [diff] [review]
patch v3

Also, I don't think you should do anything for "#appmenuPrimaryPane menupopup" on XP -- those are white already.

Comment 31

[:Margaret Leibovic]

(In reply to comment #29) 
> (In reply to comment #27)
> > I tried messing around with the gradients more, but I couldn't get the correct
> > colors. I could get the right color to appear in the middle of a wider
> > gradient, but it seems like gradients don't support rendering a 1px wide line
> > of the color of your choice.
> 
> They should, please let me see the code.

I tried:

background-image: -moz-linear-gradient(left, [white/#f1f5fb] 26px, ThreeDLightShadow 26px, ThreeDHighlight 27px, [white/#f1f5fb] 28px);

and

background-image: -moz-linear-gradient(left, [white/#f1f5fb] 26px, ThreeDLightShadow 27px, ThreeDHighlight 27px, [white/#f1f5fb] 28px);

In the primary pane they both made a 1px line of #f1f1f1 (ThreeDLightShadow is #e3e3e3), and in the secondary pane the highlight line picked up some of the blue from the background. I was curious about what was going on, so I experimented with making the color stops farther apart, which made the correct colors eventually appear in the middle of the gradient, but that's not helpful because we want a 1px line.

Comment 32

[Dão Gottwald [:dao]]

> background-image: -moz-linear-gradient(left, white 26px, ThreeDLightShadow 26px,
>                                        ThreeDLightShadow 27px, ThreeDHighlight 27px,
>                                        ThreeDHighlight 28px, white 28px);

Comment 33

[:Margaret Leibovic]

Attachment: patch v4

Thanks, Dão. I obviously have not mastered CSS gradients yet :)

Comment 34

[Dão Gottwald [:dao]]

Comment on attachment 493794 [details] [diff] [review]
patch v4

>+    border: 3px solid;
>+    -moz-border-top-colors: ThreeDShadow white;
>+    -moz-border-bottom-colors: ThreeDShadow white;
>+    -moz-border-left-colors: ThreeDShadow white;
>+    -moz-border-right-colors: ThreeDShadow white;

Why is this a 3px border?

Comment 35

[:Margaret Leibovic]

(In reply to comment #34)
> Comment on attachment 493794 [details] [diff] [review]
> patch v4
> 
> >+    border: 3px solid;
> >+    -moz-border-top-colors: ThreeDShadow white;
> >+    -moz-border-bottom-colors: ThreeDShadow white;
> >+    -moz-border-left-colors: ThreeDShadow white;
> >+    -moz-border-right-colors: ThreeDShadow white;
> 
> Why is this a 3px border?

With the multiple colors, it makes 1px of ThreeDShadow and 2px of white. The reason I did this is because there is supposed to be 2px between the top and bottom of the gutter and the gray border (you can see this in the screenshot of the native menupopup). Also, because of this, I got rid of the padding: 2px; rule for aero submenus, since the extra 2px of white border creates the same effect as the padding.

Comment 36

[:Margaret Leibovic]

http://hg.mozilla.org/mozilla-central/rev/c3ad6eaa6f2f