Closed Bug 980491 Opened 6 years ago Closed 6 years ago

Use Australis styling for the menu button and toolbar buttons.

Categories

(Thunderbird :: Theme, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 30.0

People

(Reporter: jsbruner, Assigned: jsbruner)

Details

Attachments

(3 files, 7 obsolete files)

Like bug 979632, but for Windows and Linux.
Actually, Linux already seems to be using a practically identical icon. So I'm making this Windows only.
Summary: [Linux + Windows] Use the Australis graphic for the menu button. → [Windows] Use the Australis graphic for the menu button.
Summary: [Windows] Use the Australis graphic for the menu button. → Use Australis styling for the menu button
Attached patch Patch. (obsolete) β€” β€” Splinter Review
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.
Assignee: nobody → josiah
Status: NEW → ASSIGNED
Attachment #8387073 - Flags: review?(richard.marti)
Attachment #8387073 - Flags: feedback?(mconley)
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 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.
Attachment #8387073 - Flags: review?(richard.marti)
Attachment #8387073 - Flags: feedback?(mconley)
Attached patch Patch V2. (obsolete) β€” β€” Splinter Review
This patch makes the OS X buttons have a similar feel to the recent change to Australis.
Attachment #8387073 - Attachment is obsolete: true
Attachment #8387690 - Flags: ui-review?(richard.marti)
Attachment #8387690 - Flags: review?(richard.marti)
Attachment #8387690 - Flags: feedback?(mconley)
Attached patch Patch V2. (obsolete) β€” β€” Splinter Review
Missed some trailing white space.
Attachment #8387690 - Attachment is obsolete: true
Attachment #8387690 - Flags: ui-review?(richard.marti)
Attachment #8387690 - Flags: review?(richard.marti)
Attachment #8387690 - Flags: feedback?(mconley)
Attachment #8387694 - Flags: ui-review?(richard.marti)
Attachment #8387694 - Flags: review?(richard.marti)
Attachment #8387694 - Flags: feedback?(mconley)
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?
Attachment #8387694 - Flags: ui-review?(richard.marti)
Attachment #8387694 - Flags: ui-review-
Attachment #8387694 - Flags: review?(richard.marti)
Attachment #8387694 - Flags: review-
Attached image menu-button.png β€”
Difference between TB and Fx buttons.
Josiah, bug 980374 reduced the button height on Fx. So this may be of interest for you.
Where is the blue text set? I can't locate it even with DOMi?
Attached patch WIP. (obsolete) β€” β€” Splinter Review
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?
Attachment #8387694 - Attachment is obsolete: true
Attachment #8387694 - Flags: feedback?(mconley)
(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
Attached patch Patch. (obsolete) β€” β€” Splinter Review
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.
Attachment #8388861 - Attachment is obsolete: true
Attachment #8389406 - Flags: review?(richard.marti)
Comment on attachment 8389406 [details] [diff] [review]
Patch.

Oh wait. Forgot about that double-line issue...
Attachment #8389406 - Flags: review?(richard.marti)
Attached patch button.patch (obsolete) β€” β€” Splinter Review
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.
Attached patch Patch. (obsolete) β€” β€” Splinter Review
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.
Attachment #8389406 - Attachment is obsolete: true
Attachment #8389426 - Attachment is obsolete: true
Attachment #8389562 - Flags: review?(richard.marti)
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.
Attachment #8389562 - Flags: ui-review+
Attachment #8389562 - Flags: review?(richard.marti)
Attachment #8389562 - Flags: review+
Attached patch Current Patch. β€” β€” Splinter Review
This still suffers from a lack of a gradient, but am uploading what I have now.
Attachment #8389562 - Attachment is obsolete: true
Attachment #8391429 - Flags: review+
Landed with gradient fix:

https://hg.mozilla.org/comm-central/rev/aeb5580d77da
Severity: minor → normal
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 30.0
Summary: Use Australis styling for the menu button → Use Australis styling for the menu button and toolbar buttons.
You need to log in before you can comment on or make changes to this bug.