Bug 823180 (australis-tabs-osx)

Australis tabs styling for OS X

RESOLVED FIXED in Firefox 28

Status

()

--
enhancement
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

(Depends on: 1 bug)

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [depends on windows implementation], URL)

Attachments

(1 attachment, 4 obsolete attachments)

Once bug 738491 (Winstripe) and bug 813786 (LWT) are reviewed, we can begin porting the Australis tabs design to OS X (Pinstripe).
Alias: australis-tabs-osx

Updated

6 years ago
Blocks: 826689
(In reply to Matthew N. [:MattN] from comment #0)
We're no longer blocking porting on LWT.
Assignee: nobody → mnoorenberghe+bmo
Status: NEW → ASSIGNED
Summary: Australis tabs styling for Pinstripe → Australis tabs styling for OS X
Whiteboard: [waiting on winstripe reviews] → [depends on windows implementation]
Created attachment 719400 [details] [diff] [review]
WIP1 - Working but needs polish and more testing

This builds on top of patches in bug 738491.

Comment 4

6 years ago
(In reply to Matthew N. [:MattN] from comment #3)
> Created attachment 719404 [details]
> Screenshot of WIP1

Looks very good ! Same remark concerning the close buttons though : they are a bit greyer on the mockups.

Comment 5

6 years ago
I forgot to add that the text of unselected tabs should be grey too.
Created attachment 724809 [details] [diff] [review]
v.1 Australis tabs styling for OS X

There is a known issue with the seams of background tabs but they are not caused by this patch AFAICT and it seems to be a rounding issue so I'd like to handle that outside this bug.

(In reply to Guillaume C. [:ge3k0s] from comment #4)
> Looks very good ! Same remark concerning the close buttons though : they are
> a bit greyer on the mockups.

We'll handle this in bug 851001 since there was a discrepancy with Windows mockups and there is no Hi-DPI image in the OS X mockups.

(In reply to Guillaume C. [:ge3k0s] from comment #5)
> I forgot to add that the text of unselected tabs should be grey too.

Fixed.
Attachment #719400 - Attachment is obsolete: true
Attachment #719404 - Attachment is obsolete: true
Attachment #724809 - Flags: review?(mconley)
Attachment #724809 - Flags: review?(dao)
Comment on attachment 724809 [details] [diff] [review]
v.1 Australis tabs styling for OS X

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

Hey Matt,

Everything in here looks OK to me. The shared tabs.inc.css include continues to make me really, really happy.

-Mike
Attachment #724809 - Flags: review?(mconley) → review+

Comment 8

6 years ago
(In reply to Matthew N. [:MattN] from comment #6)
> (In reply to Guillaume C. [:ge3k0s] from comment #5)
> > I forgot to add that the text of unselected tabs should be grey too.
> 
> Fixed.

I have tested UX branch on OSX this morning and text of unselected tabs hadn't appeared grey. Does someone see it grey ?
(In reply to Guillaume C. [:ge3k0s] from comment #8)
> (In reply to Matthew N. [:MattN] from comment #6)
> > (In reply to Guillaume C. [:ge3k0s] from comment #5)
> > > I forgot to add that the text of unselected tabs should be grey too.
> > 
> > Fixed.
> 
> I have tested UX branch on OSX this morning and text of unselected tabs
> hadn't appeared grey. Does someone see it grey ?

I did lighten the label color for all tabs but forget to adjust the opacity for background tabs.
Comment on attachment 724809 [details] [diff] [review]
v.1 Australis tabs styling for OS X

> .tab-stack {
>   /* ensure stable tab height with and without toolbarbuttons on the tab bar */
>-  height: 26px;
>+  height: @tabHeight@;
> }

Is this still needed? Generally, if you need to set a height for OS X where you didn't need it for Windows, that seems suspicious.

> .tabbrowser-tab,
> .tabs-newtab-button {
>   -moz-appearance: none;
>   font: message-box;
>   font-weight: bold;
>   text-shadow: @loweredShadow@;
>-  margin: 0;
>-  padding: 0;
>   border: none;
>   text-align: center;
>   -moz-box-align: stretch;
> }

I thought the tab label shouldn't be centered anymore.

>+.tabbrowser-tab:not(:-moz-lwtheme) {
>+  color: #333;
> }

Remove :not(:-moz-lwtheme), .tabbrowser-tab:-moz-lwtheme overrides this anyway.

> #TabsToolbar {
>   -moz-appearance: none;
>-  height: 26px;
>+  height: @tabHeight@;
>   background-repeat: repeat-x;
> }

Does the height still need to be specified here?

> #tabbrowser-tabs {
>   -moz-box-align: stretch;
>-  height: 26px;
>+  height: @tabHeight@;
> }

And here?

>+/* image preloading hack */
>+#TabsToolbar::after {
>+  content: '';
>+  display: block;
>+  background-image:
>+    url(chrome://browser/skin/tabbrowser/tab-active-middle.png),
>+    url(chrome://browser/skin/tabbrowser/tab-background-end.png),
>+    url(chrome://browser/skin/tabbrowser/tab-background-middle.png),
>+    url(chrome://browser/skin/tabbrowser/tab-background-start.png),
>+    url(chrome://browser/skin/tabbrowser/tab-stroke-end.png),
>+    url(chrome://browser/skin/tabbrowser/tab-stroke-start.png);
>+}

There's no point in preloading images that are displayed anyway in the selected tab, right?
Is any of this still useful in the first place? Seems like we didn't see the need for this in bug 738491.
Attachment #724809 - Flags: review?(dao) → review-
(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 724809 [details] [diff] [review]
> v.1 Australis tabs styling for OS X
> 
> > .tabbrowser-tab,
> > .tabs-newtab-button {
> >   -moz-appearance: none;
> >   font: message-box;
> >   font-weight: bold;
> >   text-shadow: @loweredShadow@;
> >-  margin: 0;
> >-  padding: 0;
> >   border: none;
> >   text-align: center;
> >   -moz-box-align: stretch;
> > }
> 
> I thought the tab label shouldn't be centered anymore.

Yup. They look much better with left-align, as per the spec. http://cl.ly/image/1T1X420X2s3p
Created attachment 729416 [details] [diff] [review]
v.2 Address review comments and fix label opacity on bg. tabs

* Fixed background tab label opacity (comment 9)
* Removed unnecessary heights since things seem to work fine with the min-heights
* Change tab label to left-alignment
* Removed preloading of selected tab images

(In reply to Dão Gottwald [:dao] from comment #10)
> Is any of this still useful in the first place? Seems like we didn't see the
> need for this in bug 738491.
I did sometimes notice a delay loading the images on first hover in a Windows debug build on a fast computer so I left this in.
Attachment #724809 - Attachment is obsolete: true
Attachment #729416 - Flags: review?(dao)
Hey Dao,

We're hoping to have this in an r+ state by April 3rd. Do you think you could take another look at this today?

Thanks,

-Mike
Flags: needinfo?(dao)
Comment on attachment 729416 [details] [diff] [review]
v.2 Address review comments and fix label opacity on bg. tabs

>+.tabbrowser-tab > .tab-stack > .tab-content > .tab-label:not([selected="true"]) {
>+  opacity: .7;
> }

remove ".tabbrowser-tab > .tab-stack > .tab-content > "
Attachment #729416 - Flags: review?(dao) → review+
Flags: needinfo?(dao)
Created attachment 732698 [details] [diff] [review]
v.3 Australis tab styling for OS X

Thanks for the reviews.
Attachment #729416 - Attachment is obsolete: true
Attachment #732698 - Flags: review+
Whiteboard: [depends on windows implementation] → [fixed-in-ux][depends on windows implementation]
Depends on: 857642
Depends on: 858089
No longer depends on: 856107
No longer blocks: 826689

Updated

6 years ago
Depends on: 868807

Updated

6 years ago
No longer depends on: 868807
Depends on: 869660
No longer depends on: 869660

Updated

6 years ago
Depends on: 873464
Depends on: 882578
Depends on: 884492

Updated

5 years ago
Depends on: 887397

Updated

5 years ago
Depends on: 897471

Updated

5 years ago
No longer depends on: 879607

Updated

5 years ago
Depends on: 839888

Updated

5 years ago
Depends on: 925884

Updated

5 years ago
Depends on: 932597

Updated

5 years ago
Depends on: 933964
Restrict Comments: true

Comment 16

5 years ago
https://hg.mozilla.org/mozilla-central/rev/bc3e29ff7277
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Restrict Comments: false
Whiteboard: [fixed-in-ux][depends on windows implementation] → [depends on windows implementation]
Target Milestone: --- → Firefox 28

Updated

5 years ago
Depends on: 941831

Updated

5 years ago
Depends on: 898875

Updated

5 years ago
Depends on: 946768

Updated

5 years ago
Depends on: 945547

Updated

5 years ago
Depends on: 964621

Updated

5 years ago
Depends on: 964300

Updated

5 years ago
Depends on: 973694

Updated

3 years ago
Depends on: 1218225

Updated

3 years ago
Depends on: 1246444
You need to log in before you can comment on or make changes to this bug.