870574 - Adjust customization look of OS X browser window to have grid background and padding

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

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

Attachment: Mockup by shorlander

Bug 869101 added the grid background and padding for Windows, but this needs to be implemented still for OS X.

See the attached mockup for what the patch should look like.

Comment 1

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

Attachment: WIP Patch

Mike, do you know how I can disable the background on the tabbar (not the selected tab though)?

Comment 2

[Markus Stange [:mstange]]

It's probably the ::before pseudoelement: https://hg.mozilla.org/projects/ux/file/39b1b4eee94b/browser/themes/osx/browser.css#l2168

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Yes, Markus is correct.

Comment 4

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

Attachment: Patch v.1

Ok, so this patch implements the padding around the customization area.

I commented out the box-shadow for now, since it has a break between the toolbars and the content area. We would need an element that fits that same region to not have the break, but getting one that excludes the TabsToolbar doesn't seem easy.

I also commented out the code for TabsInTitlebar.allowedBy when entering/exiting customization mode, since the tabs move out to the left and right when this is called, which overlaps the tabs with the stoplight buttons. Is this code really needed?

Comment 5

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

Attachment: Patch v1

OK! I fixed the shadow thanks to some nifty CSS from Stephen, and the TabsInTitlebar lines no longer need to be commented out. This should be ready for review now! :)

Comment 6

[Mike Conley (:mconley) (:⚙️)]

This looks really good! Note that while transitioning *into* customization mode looks smooth, customizing out looks a little janky. I wonder if it's because we're doing too much all at once, instead of waiting for the padding transition to end? Might be worth filing a follow-up if we didn't want to investigate that here.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

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

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

Code looks fine, but a few small issues:

1) There's no scrollbar for the palette if it overflows
2) I'm noticing a weird artifact sometimes in the palette: http://i.imgur.com/DnqL5up.png Do you have any idea what this is? I get the feeling that it's related with scrollbars.

Comment 8

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

Attachment: Patch v1.1

This version sets an attribute on the documentElement while exiting customizeMode so we can keep the background present on the #tab-view-deck and also delay showing the #TabsToolbar background until the transition completes.

The scrollbar issue with them not appearing is likely due to the new Lion scrollbars, and the graphical glitch you saw also appears to be part of Lion scrollbars (I saw this now on a couple webpage [irccloud.com and bugzilla] after toggling the system pref for scrollbars).

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 748088 [details] [diff] [review]
Patch v1.1

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

Better. There's still a weird moment where we suddenly snap the tabstrip background on. I don't want to block on that though - let's file a follow-up bug to have UX look at the transition and give us their thoughts.

In the meantime, just a few tiny nits, but overall this looks good. r=me with the following changes.

Great work,

-Mike

::: browser/components/customizableui/src/CustomizeMode.jsm
@@ +169,5 @@
>  
>      let window = this.window;
>      let document = this.document;
>  
> +    document.documentElement.setAttribute("customize-exiting", "true");

That's a lot of document.documentElement. Let's alias that.

@@ +170,5 @@
>      let window = this.window;
>      let document = this.document;
>  
> +    document.documentElement.setAttribute("customize-exiting", "true");
> +    document.documentElement.addEventListener("transitionend", function onTransitionEnd(evt) {

Why not listen to the deck directly instead of waiting for it to bubble up to the document?

@@ +171,5 @@
>      let document = this.document;
>  
> +    document.documentElement.setAttribute("customize-exiting", "true");
> +    document.documentElement.addEventListener("transitionend", function onTransitionEnd(evt) {
> +      window.console.log("customizableui transitionend property:" + evt.propertyName);

Needs to go, I think - unless we're OK with log messages going in... at the very least, I think this should use LOG.

Comment 10

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

Attachment: Patch v1.2

Fixed the issues mentioned. Filed bug 870931 for the UX team to investigate the smoothness of the transition.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Filed bug 870941 about the OSX scrollbar artifact. Not marking as blocking because I think it's a pretty general problem unrelated to this patch.

Comment 12

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

Landed on UX:
https://hg.mozilla.org/projects/ux/rev/295ba37d2991

The landed patch also fixed the background tabs being too translucent and the padding at the top of the browser not being enough for some machine's font-sizes.

Filed bug 870990 as a follow-up to put the border-radius on bottom of the customization area.

Comment 13

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/295ba37d2991