734373 - Implement Australis toolbar button design on Windows

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Comment 1

[Dão Gottwald [:dao]]

Stephen, I thought I saw a mockup indicating that each button's actual target area should be larger than the button shape on hover. Did I misinterpret this, has it changed?

Comment 2

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #1)
> Stephen, I thought I saw a mockup indicating that each button's actual
> target area should be larger than the button shape on hover. Did I
> misinterpret this, has it changed?

Yes, initially I had the hit area extending the height of the toolbar and to the edges of the toolbar for first and last buttons.

I didn't include that in the final specs for Windows mostly because I wasn't sure how viable/complicated it would be to have the button shape differ from the target area.

If it's something we can do I think we should and I can update to the specs to reflect that :)

Comment 3

[Dão Gottwald [:dao]]

Joshua, if you want to work on this, please do it on top of the patch in bug 735691.

Comment 4

[Joshua M [:soapy]]

(In reply to Dão Gottwald [:dao] from comment #3)
> Joshua, if you want to work on this, please do it on top of the patch in bug
> 735691.

Sure I can take this. Should I wait until work in finished on bug 735691 before I start?

Comment 5

[Dão Gottwald [:dao]]

(In reply to Joshua M from comment #4)
> Sure I can take this. Should I wait until work in finished on bug 735691
> before I start?

I don't expect that patch to change much, but who knows. Since we have five weeks left to finish this, it probably makes sense to wait.

Comment 6

[u428464]

(In reply to Dão Gottwald [:dao] from comment #5)
> I don't expect that patch to change much, but who knows. Since we have five
> weeks left to finish this, it probably makes sense to wait.
Is there already a planning about the different phases of the Australis redesign ?
Because Stephen Horlander is updating the specs quite often. When will he release the final ones ?

Comment 7

[Joshua M [:soapy]]

Attachment: Patch v1

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 610865 [details] [diff] [review]
Patch v1

Real nice job. I like how you were able to fix the border color on the disabled back button to match the border on the url bar.

Comment 9

[Frank Yan (:fryn)]

(In reply to Jared Wein [:jaws] from comment #8)
> Real nice job. I like how you were able to fix the border color on the
> disabled back button to match the border on the url bar.

\o/ Stephen provided the following mockup to show kinda what it should look like, and this patch gets the border even more polished than that:
http://people.mozilla.com/~shorlander/files/australis-designSpecs/images-win7/style-navBar-buttons-inactive.png

Great work, Joshua! :)

Comment 10

[Dão Gottwald [:dao]]

(In reply to Frank Yan (:fryn) from comment #9)
> (In reply to Jared Wein [:jaws] from comment #8)
> > Real nice job. I like how you were able to fix the border color on the
> > disabled back button to match the border on the url bar.
> 
> \o/ Stephen provided the following mockup to show kinda what it should look
> like, and this patch gets the border even more polished than that:
> http://people.mozilla.com/~shorlander/files/australis-designSpecs/images-
> win7/style-navBar-buttons-inactive.png

I prefer the mockup, the button looks more disabled there. I don't agree with the premise that the border should continue the nav bar border as closely as possible regardless of the state.

Comment 11

[Dão Gottwald [:dao]]

Attachment: back button background opacity

Can we make the back button texture less bright or less opaque? We invert the icon for dark personas and tabs on bottom, so the current background doesn't make much sense.

The attached screenshot shows what the current patch is doing (above) and what it would look like with just the background color removed (below).

Comment 12

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [:dao] from comment #10)
> I prefer the mockup, the button looks more disabled there. I don't agree
> with the premise that the border should continue the nav bar border as
> closely as possible regardless of the state.

Yeah I think a subtle opacity shift for disabled is fine (i.e. the mockup) and would make the state a little more obvious.


(In reply to Dão Gottwald [:dao] from comment #11)
> The attached screenshot shows what the current patch is doing (above) and
> what it would look like with just the background color removed (below).

Looks good to me :)

Comment 13

[Joshua M [:soapy]]

Attachment: Patch v2

Tweaked disabled back button look based on comments 11 and 12.

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 613406 [details] [diff] [review]
Patch v2

>+#navigator-toolbox[iconsize=large][mode=icons][tabsontop=false] > #nav-bar > #unified-back-forward-button > :-moz-any(#back-button,#forward-button):-moz-any(:not(:hover):not(:active):not([open="true"]),[disabled="true"]) > .toolbarbutton-icon:-moz-system-metric(windows-compositor):not(:-moz-lwtheme),
>+#navigator-toolbox[iconsize=large][mode=icons] > #nav-bar > #unified-back-forward-button > :-moz-any(#back-button,#forward-button):-moz-any(:not(:hover):not(:active):not([open="true"]),[disabled="true"]) > .toolbarbutton-icon:-moz-lwtheme-brighttext {
>+  background-color: transparent;
>+}

Can we avoid the background color in more states? It doesn't seem to add much in the non-hovered state, and I noticed that it causes the border to be rendered less smooth in the disabled state.

I think we should remove the special treatment of the zoom buttons. They work just fine without the separator; it's unnecessary complexity for buttons used by a tiny minority.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 613406 [details] [diff] [review]
Patch v2

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

::: browser/themes/winstripe/browser.css
@@ +768,5 @@
> +}
> +
> +/* Separator between menu and split type buttons */
> +@navbarLargeIcons@ .toolbarbutton-1:not(:hover):not(:active):not([open]):not([checked]) > .toolbarbutton-menubutton-dropmarker::before,
> +@navbarLargeIcons@ #zoom-controls:not(:hover) > #zoom-in-button::before {

I don't think this adds enough complexity to warrant its removal. As we move the bookmark star outside of the address bar, the presence of this style will be more visible.

Comment 16

[Dão Gottwald [:dao]]

The bookmarks button has exactly nothing to do with zoom buttons.

Comment 17

[Stephen Horlander [:shorlander]]

I would rather have slightly more complex CSS if it means we don't compromise on the design.

Stylistically split-buttons (e.g. theoretical bookmark button) and connected buttons (e.g. zoom, small forward+back) should all look and act the same.

I guess structurally they are different? Is there a better way to structure them to so that there is less redundancy in the CSS?

Comment 18

[Dão Gottwald [:dao]]

(In reply to Stephen Horlander from comment #17)
> I would rather have slightly more complex CSS if it means we don't
> compromise on the design.

I don't think that not optimizing for edge cases compromises the design. I've never seen the zoom buttons on a toolbar in the wild. It's cool that we have them for those who want them, but I don't want to spend more time than necessary on them.

> Stylistically split-buttons (e.g. theoretical bookmark button) and connected
> buttons (e.g. zoom, small forward+back) should all look and act the same.
>
> I guess structurally they are different?

The zoom buttons are just two adjacent buttons in a container. They happen to be able to reuse some of the menu-button styling, but this means that whenever we touch that styling, we need to make sure to not regress the zoom button styling. It's a burden we don't need.

Comment 19

[Stephen Horlander [:shorlander]]

I will file a follow-up bug for the zoom-buttons so we don't block on it.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2 without zoom-controls changes

This is soapy's Patch v2 without the zoom-controls changes.

Comment 21

[Dão Gottwald [:dao]]

See the first part of comment 14.

Comment 22

[Joshua M [:soapy]]

Attachment: Patch v3

Removed special zoom control styling.
Addressed comment 14.

Comment 23

[Dão Gottwald [:dao]]

Comment on attachment 617188 [details] [diff] [review]
Patch v3

>+@navbarLargeIcons@ .toolbarbutton-1:not([disabled="true"]):-moz-any(:hover,[open="true"]) > .toolbarbutton-menubutton-button > .toolbarbutton-icon:not(:-moz-lwtheme-brighttext),
>+@navbarLargeIcons@ .toolbarbutton-1:not([disabled="true"]):hover > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon:not(:-moz-lwtheme-brighttext),
>+@navbarLargeIcons@ .toolbarbutton-1:not([type="menu-button"]):not([disabled="true"]):not([checked="true"]):not([open="true"]):not(:active):hover > .toolbarbutton-icon:not(:-moz-lwtheme-brighttext),
>+window:not([chromehidden~=toolbar]) #navigator-toolbox[iconsize=large][mode=icons] > #nav-bar[currentset*="unified-back-forward-button,urlbar-container"]:-moz-any([tabsontop=true],[tabsontop=false]:not(:-moz-system-metric(windows-compositor))):not(:-moz-lwtheme-brighttext) > #unified-back-forward-button > .toolbarbutton-1:-moz-any([disabled="true"],:not([open="true"]):not([disabled="true"]):not(:active)) > .toolbarbutton-icon {
>+  background-color: hsla(210,32%,93%,.2);
>+}

Not sure what the point of this is. I'll remove it for now, please file a new bug on it if it's really needed or desirable.

I'll also remove |="true"| from the attribute selectors that you touched.

Comment 24

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/d11d6ff6ae89

Comment 25

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d11d6ff6ae89

Comment 26

[Jean-Yves Perrier [:teoli]]

Am I correct in understanding this change is only for Win7 right now? Not for MacOS, Linux or older Windows? (These are other bugs)

Comment 27

[Jared Wein [:jaws] (please needinfo? me)]

This bug is only for Windows XP and higher. Can you file bugs for MacOS and Linux?

Comment 28

[Josiah Bruner [:jsbruner]]

Bug 856665 is for toolbar icons in OS X.

Comment 29

[Marco Castelluccio [:marco]]

(In reply to Jared Wein [:jaws] from comment #27)
> This bug is only for Windows XP and higher. Can you file bugs for MacOS and
> Linux?

I filed bug 875479 for Linux.