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

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Theme
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

(Depends on: 1 bug)

Trunk
Firefox 28
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

Created attachment 747637 [details]
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.
Blocks: 869101
Created attachment 747737 [details] [diff] [review]
WIP Patch

Mike, do you know how I can disable the background on the tabbar (not the selected tab though)?
Flags: needinfo?(mconley)
It's probably the ::before pseudoelement: https://hg.mozilla.org/projects/ux/file/39b1b4eee94b/browser/themes/osx/browser.css#l2168
Yes, Markus is correct.
Flags: needinfo?(mconley)
Created attachment 748016 [details] [diff] [review]
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?
Attachment #747737 - Attachment is obsolete: true
Attachment #748016 - Flags: feedback?(mconley)
Created attachment 748049 [details] [diff] [review]
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! :)
Attachment #748016 - Attachment is obsolete: true
Attachment #748016 - Flags: feedback?(mconley)
Attachment #748049 - Flags: review?(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 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.
Attachment #748049 - Flags: review?(mconley) → feedback+
Created attachment 748088 [details] [diff] [review]
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).
Attachment #748049 - Attachment is obsolete: true
Attachment #748088 - Flags: review?(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.
Attachment #748088 - Flags: review?(mconley) → review+
Created attachment 748126 [details] [diff] [review]
Patch v1.2

Fixed the issues mentioned. Filed bug 870931 for the UX team to investigate the smoothness of the transition.
Attachment #748088 - Attachment is obsolete: true
Attachment #748126 - Flags: review+
Depends on: 870931
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.
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.
Depends on: 870990
Whiteboard: [fixed in ux]
Depends on: 880779

Comment 13

4 years ago
https://hg.mozilla.org/mozilla-central/rev/295ba37d2991
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in ux]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.