660672 - Toolbar dropdowns looks different than the other buttons in Aero glass

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Andreas Nilsson (:andreasn)]

Attachment: screenshot of issue

The dropdowns looks different from the other buttons under Aero glass.

Comment 1

[rsx11m]

Preemptively blocking bug 608792 in case a patch here causes similar accessibility problems as are already seen with the toolbar buttons with high-contrast themes [or as a reminder to consider such aspects ;-) ].

Comment 2

[Richard Marti (:Paenglab)]

Attachment: Patch changing the menulist appearance

Comment 3

[Richard Marti (:Paenglab)]

Comment on attachment 536147 [details] [diff] [review]
Patch changing the menulist appearance

Patch giving in mainWindow menulists the toolbarbutton appearance. I removed the highlight background color on menulists with focus but leaved the focus ring.

rsx11m: The change applies only on Aero Basic and Glass. On Classic and High color the default appearance is used. If we want the new appearance on classic, we have to double the code because -moz-windows-default-theme applies only on Basic and Glass.

Comment 4

[Richard Marti (:Paenglab)]

Sorry for the spam. This patch needs Bug 658328 applied.

Comment 5

[Andreas Nilsson (:andreasn)]

Attachment: this label looks a bit odd

This label looks a bit odd. I think it looked better before.

Comment 6

[Andreas Nilsson (:andreasn)]

But apart from that it looks all good, so I would give this a ui-r+ with that fixed.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: New menulist appearance

The only change is a "connecting" background between label and menulist under Aero.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: Label background with last patch

Would this look better? The label needs a background on Aero like the menubar. With "connecting" the label to the menulist it's clearer for what the label is. I could also try with more padding on top/bottom to give the background the same high as the menulist, but I don't know if this always fitts with non default font height.

Comment 9

[Andreas Nilsson (:andreasn)]

(In reply to comment #8)
> Created attachment 543475 [details]
> Label background with last patch
> 
> Would this look better? The label needs a background on Aero like the
> menubar. With "connecting" the label to the menulist it's clearer for what
> the label is.

You're right, it does need some background, so the previous patch in comment #3 is good in that case (the connecting label don't look as good). Just to double check (since for some odd reason this patch fails to apply for me now), could you post a screenshot of these controls inside the customization dialog as well?

Comment 10

[Richard Marti (:Paenglab)]

Attachment: New menulist appearance

This is the old patch against the actual file in hg. This should apply now.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: Appearance in customization dialog

Comment 12

[Andreas Nilsson (:andreasn)]

Comment on attachment 544010 [details] [diff] [review]
New menulist appearance

Works great!

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 544010 [details] [diff] [review]
New menulist appearance

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

Other than the minor nit below, the code looks fine to me, although I'll be interested to see how it interacts with a different background.

r=me, assuming that that's fine.  (I'm building it now, and will test it after that.)

Thanks,
Blake.

::: mail/themes/qute/mail/mailWindow1-aero.css
@@ -174,0 +174,34 @@
> > +  menulist {
> > +    -moz-appearance: none;
> > +    padding: 1px 5px !important;
> > +    background: rgba(151, 152, 153, .05)
NaN more ...

I think these should line up with the "0 0" on the previous line.

Comment 14

[Richard Marti (:Paenglab)]

Do you mean the two spaces before menulist {  etc.? This is an indentation after the @media all and (-moz-windows-default-theme)

Comment 15

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in

Patch addressing the comment for check-in with ui-r+ and r+ from previous patch.

Comment 16

[Jim Porter (:squib)]

Checked in: http://hg.mozilla.org/comm-central/rev/1cb6eac2ee6f