980491 - Use Australis styling for the menu button and toolbar buttons.

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Josiah Bruner [:jsbruner]]

Like bug 979632, but for Windows and Linux.

Comment 1

[Josiah Bruner [:jsbruner]]

Actually, Linux already seems to be using a practically identical icon. So I'm making this Windows only.

Comment 2

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

Alright, this removes the open states for the toolbar buttons on OS X (minus the chat, since it's used as an indicator) and switches the mail-toolbar-aero and inverted graphics in windows/ to use the new menu button.

I didn't do the classic graphics on windows or Linux, since the Australis coloring was too inconsistent with these older style icons.

Comment 3

[Richard Marti (:Paenglab)]

Do you on OS X really want to only remove the open state? Bug 909349 adds also button shapes on hover/open, see: https://hg.mozilla.org/mozilla-central/file/ccbe695b5f40/browser/themes/osx/browser.css#l460

Comment 4

[Josiah Bruner [:jsbruner]]

Comment on attachment 8387073 [details] [diff] [review]
Patch.

Yes, I just noticed that. Apparently things changed a lot more than I thought.

Clearing review flags for now.

Comment 5

[Josiah Bruner [:jsbruner]]

Attachment: Patch V2.

This patch makes the OS X buttons have a similar feel to the recent change to Australis.

Comment 6

[Josiah Bruner [:jsbruner]]

Attachment: Patch V2.

Missed some trailing white space.

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 8387694 [details] [diff] [review]
Patch V2.

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

Thank you for the quick update.
The icon files are looking good, except they can be optimized in size ;).

I give a ui-r- because on menu-buttons on dropmarkers the border has on all four edges a radius. Please remove this like Firefox has for the bookmark-button (I'll attach a image to be clear what I mean).
The checked QFB button text is still blue. I think this is no more needed with the now non-blue icon.
The save button on attachment bar has a double border on hover.

The Fx buttons are also 30px tall and the TB buttons 25px. Maybe it's good if we follow this to be consistent.

Please file also a followup bug for the AB and Composer to be in synch with the main window.

::: mail/themes/osx/mail/primaryToolbar.css
@@ +75,5 @@
> +
> +.toolbarbutton-1:not([type="menu-button"]):hover:not([disabled]),
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-button,
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-dropmarker {
> +  border-color: hsla(0,0%,0%,.2);

Please use hsla(0, 0%, 0%, .2) here and on all other parts in this patch.

@@ +77,5 @@
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-button,
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-dropmarker {
> +  border-color: hsla(0,0%,0%,.2);
> +  background: hsla(0,0%,100%,.1) linear-gradient(hsla(0,0%,100%,.3),
> +  			  hsla(0,0%,100%,.1)) padding-box;

tabs instead of spaces.

@@ +113,5 @@
> +.toolbarbutton-1:active:not([disabled]),
> +.button-appmenu[open="true"] {
> +  background-color: transparent !important;
> +  background: hsla(0,0%,0%,.02) linear-gradient(hsla(0,0%,0%,.12),
> +  			  hsla(0,0%,0%,0)) border-box !important;

Tabs.

@@ +117,5 @@
> +  			  hsla(0,0%,0%,0)) border-box !important;
> +  border-color: hsla(0,0%,0%,.3) !important;
> +  box-shadow: 0 1px 0 hsla(0,0%,100%,.5),
> +  			  0 1px 0 hsla(0,0%,0%,.05) inset,
> +			  0 1px 1px hsla(0,0%,0%,.2) inset !important;

Both lines have tabs.

@@ +118,5 @@
> +  border-color: hsla(0,0%,0%,.3) !important;
> +  box-shadow: 0 1px 0 hsla(0,0%,100%,.5),
> +  			  0 1px 0 hsla(0,0%,0%,.05) inset,
> +			  0 1px 1px hsla(0,0%,0%,.2) inset !important;
> +  transition-duration: 10ms !important;

Are all the !important needed?

Comment 8

[Richard Marti (:Paenglab)]

Attachment: menu-button.png

Difference between TB and Fx buttons.

Comment 9

[Richard Marti (:Paenglab)]

Josiah, bug 980374 reduced the button height on Fx. So this may be of interest for you.

Comment 10

[Josiah Bruner [:jsbruner]]

Where is the blue text set? I can't locate it even with DOMi?

Comment 11

[Josiah Bruner [:jsbruner]]

Attachment: WIP.

This is my current patch, but I'm facing some issues.

So in addition to the blue text, I also am facing an issue with buttons that contain a dropdown indicator.

It seems that .toolbarbutton-menubutton-dropmarker:active doesn't actually do anything in my patch, as pressing the dropmarker causes no change in styling. I was able to make the menubutton work by applying those !important rules, but it still doesn't help with the dropmarker.

Any thoughts?

Comment 12

[Richard Marti (:Paenglab)]

(In reply to Josiah Bruner [:JosiahOne] from comment #10)
> Where is the blue text set? I can't locate it even with DOMi?

This should be http://mxr.mozilla.org/comm-central/source/mail/themes/osx/mail/primaryToolbar.css#130

Comment 13

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

Well, I think we'll have to deal with the the dropmarker issue in a separate bug, as it seems to be a separate issue entirely. So here's the new patch.

Comment 14

[Josiah Bruner [:jsbruner]]

Comment on attachment 8389406 [details] [diff] [review]
Patch.

Oh wait. Forgot about that double-line issue...

Comment 15

[Richard Marti (:Paenglab)]

Attachment: button.patch

This patch fixes dropmarker issue. Your solution with the 0.5px border doesn't work on LoDPI as the they are shown as 1px border. My patch also fixes this.

Comment 16

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

Ah, thanks for that Richard, very helpful.

This patch integrates your changes as well as fixes the double-border and size issues in the message header.

Comment 17

[Richard Marti (:Paenglab)]

Comment on attachment 8389562 [details] [diff] [review]
Patch.

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

This looks good.
r+ with the comments fixed.

Additionally the messageHeader-buttons[type="menu-button"] in active state are darker than the normal active buttons. With this changes in messageHeader.css it would look as before:

Add to 
.toolbarbutton-1.msgHeaderView-button > .toolbarbutton-menubutton-dropmarker,
.toolbarbutton-1.msgHeaderView-button:hover > .toolbarbutton-menubutton-button,
.toolbarbutton-1.msgHeaderView-button:hover > .toolbarbutton-menubutton-dropmarker {

a
 background-image: none !important;

and remove the whole rule
.msgHeaderView-button:active,
.msgHeaderView-button[type="menu-button"][open] {
  box-shadow: 0 4px 3px 0 rgb(181,181,181) inset;
}

What do you think to this? Also r+ if you change this, but please check it before.
I'll attach a screenshot how it looks actual.

::: mail/themes/osx/mail/primaryToolbar.css
@@ +73,5 @@
> +  transition-duration: 250ms;
> +}
> +
> +.toolbarbutton-1:not([type="menu-button"]):hover:not([disabled]),
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-button,

Please can you insert here

.toolbarbutton-1[open="true"]:not([disabled]) > .toolbarbutton-menubutton-button,

then the .toolbarbutton-menubutton-button has always the depressed state when the dropmarker is pressed and you move the cursor outside the popup and button area?

@@ +77,5 @@
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-button,
> +.toolbarbutton-menubutton-button:active + .toolbarbutton-menubutton-dropmarker,
> +.toolbarbutton-1:hover:not([disabled]) > .toolbarbutton-menubutton-dropmarker {
> +  border-color: hsla(0, 0%, 0%, .2);
> +  background: hsla(0, 0%, 100%, .1) linear-gradient(hsla(0, 0%, 100%, .3), 

Nit, trailing space.

@@ +89,5 @@
> +.toolbarbutton-1:not([disabled]) > .toolbarbutton-menubutton-button:active,
> +.toolbarbutton-1[open="true"] >
> +.toolbarbutton-menubutton-dropmarker:not([disabled="true"]) {
> +  background-color: transparent !important;
> +  background: hsla(0, 0%, 0%, .02) linear-gradient(hsla(0, 0%, 0%, .12), 

Nit, trailing space.

@@ +93,5 @@
> +  background: hsla(0, 0%, 0%, .02) linear-gradient(hsla(0, 0%, 0%, .12), 
> +              hsla(0, 0%, 0%, 0)) border-box;
> +  border-color: hsla(0, 0%, 0%, .3);
> +  box-shadow: 0 1px 0 hsla(0, 0%, 100%, .5), 
> +              0 1px 0 hsla(0, 0%, 0%, .05) inset, 

Nit, trailing space on this two lines.

Comment 18

[Richard Marti (:Paenglab)]

Attachment: Comparison with both buttons active

Comment 19

[Josiah Bruner [:jsbruner]]

Attachment: Current Patch.

This still suffers from a lack of a gradient, but am uploading what I have now.

Comment 20

[Josiah Bruner [:jsbruner]]

Landed with gradient fix:

https://hg.mozilla.org/comm-central/rev/aeb5580d77da