610080 - Border effect for tab overflow

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug is a subset of:

Bug 570564 - Improve tab overflow navigation
and 
Bug 579728 - when scrolling the tab container, show half of the first overflow tab in the scroll direction

the goal here is just to land the shadow being drawn on the overflowed tabs, shown here:

https://bug570564.bugzilla.mozilla.org/attachment.cgi?id=449696

Comment 1

[Frank Yan (:fryn)]

I've done this with CSS box-shadow hacks, but it looks really messy.

An image would be better. CC'ing shorlander for help with that.

Comment 2

[Asa Dotzler [:asa]]

Attachment: overflow button (hovered) is kind of wrong

Can we fix this overflow button hover state while we're at it?

Comment 4

[Alex Limi (:limi) — Firefox UX Team]

This is a theme regression since the separator was removed from the tab bar buttons earlier. I believe this is serious enough to be a soft blocker, as it just looks plain broken right now.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

I actually think this might be a hardblocker ... looks *awful* on Windows 7.

Comment 6

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Mockup: Divider line current design vs intended design on OSX

Comment 7

[Stephen Horlander [:shorlander]]

Attachment: WIP Tab Overflow Patch - 01

I took a shot at this using -moz-border-image and setting a negative margin on the scrollbuttons.

Comment 8

[Stephen Horlander [:shorlander]]

Attachment: Screenshot

Comment 9

[Frank Yan (:fryn)]

Comment on attachment 507117 [details] [diff] [review]
WIP Tab Overflow Patch - 01

IIRC, you mentioned that this breaks app tab positioning.
The shadow should extend to the bottom border of the tab strip, but it doesn't seem to do that on the right side in the screenshot.
Also, the shadow should ignore mouse events.

It's a good start. I'll see what I can do to improve upon it.
Thanks for pitching in. :)

Comment 10

[Stephen Horlander [:shorlander]]

(In reply to comment #9)
> IIRC, you mentioned that this breaks app tab positioning.

The "margin-left: -6px" moved the apptabs over 6px as well. Adding a balancing "margin-right: 6px" fixed this and still positioned the tabs behind the shadow. However I can't claim to understand the ramifications of doing such a thing :)

> Also, the shadow should ignore mouse events.

Yes. This method won't work for that. As we discussed an additional non-interactive element might work.

> It's a good start. I'll see what I can do to improve upon it.

Thank you!

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

The attached patch seems to miss tab-overflow-shadow.png.

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to comment #11)
> The attached patch seems to miss tab-overflow-shadow.png.

It doesn't. Sorry for the noise.

Comment 13

[Johnathan Nightingale [:johnath]]

Frank - ETA for a new patch, here?

Comment 14

[Frank Yan (:fryn)]

(In reply to comment #13)
> Frank - ETA for a new patch, here?

We ran into problems trying to get the shadow effect without invasive surgery, so we're scoping this down to a border effect, since that's the hard-blocking part anyway.

ETA: tonight.

Comment 15

[Frank Yan (:fryn)]

Attachment: patch

Filed bug 632156 for the shadow effect followup.

(In reply to comment #2)
> Created attachment 504049 [details]
> overflow button (hovered) is kind of wrong

This patch mitigates the problem, but it's still there. It's non-trivial to make small tweaks to native styling. We may decide to do non-native styling on the buttons to circumvent this, but that will be in a followup.

Comment 16

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 510442 [details] [diff] [review]
patch

Taking the review from shorlander to accelerate us landing this.  I checked classic, basic, glass and panorama on fryn's machine.  Overall looked good, no shadow but it kind of looks like there is one, due to the gradient on tabs (our mental hardware seems to kind of fill in the shadow for us).

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 510442 [details] [diff] [review]
patch

Only tested this on Linux right now. The custom hover effect looks bogus, I think we can drop that without replacement (there's no hover effect without this patch either).

Windows and Mac are next up.

>+.tabbrowser-arrowscrollbox > .scrollbutton-up:not([disabled="true"]):-moz-locale-dir(ltr),
>+.tabbrowser-arrowscrollbox > .scrollbutton-down:not([disabled="true"]):-moz-locale-dir(rtl) {

nit: just [disabled] will do

Comment 18

[Frank Yan (:fryn)]

(In reply to comment #17)

Thanks for the Linux review; I will address those asap. Also, shorlander noted that all these need to work in tabs-on-bottom mode. I'll also adjust the patch for that.

I put up tryserver builds here with the above patch:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-dbddb463e8a2/

Comment 19

[Dão Gottwald [:dao]]

The rest of the patch seems fine. Tabs on bottom on Mac also works surprisingly well.

Comment 20

[Frank Yan (:fryn)]

Attachment: patch v2

Removed linux hover effect and fixed nit as per comment 17.

Fixed tabs-on-bottom mode on OS X.

Comment 21

[Frank Yan (:fryn)]

Attachment: patch v3

Oops, missed a couple [disabled="true"]'s. No change in patch behavior.

Comment 22

[Dão Gottwald [:dao]]

Comment on attachment 510934 [details] [diff] [review]
patch v3

>--- a/browser/themes/pinstripe/browser/browser.css
>+++ b/browser/themes/pinstripe/browser/browser.css

>+#tabbrowser-tabs[tabsontop="false"] > .tabbrowser-arrowscrollbox > .scrollbutton-up,
>+#tabbrowser-tabs[tabsontop="false"] > .tabbrowser-arrowscrollbox > .scrollbutton-down {
>+  -moz-transform: scaleY(-1);
>+}

I think you need to flip the buttons' icons a second time for this to produce the right result.

Comment 23

[Frank Yan (:fryn)]

Attachment: patch v4

(In reply to comment #22)

The buttons' icons are almost completely symmetrical across the horizontal axis, so I didn't notice it, but it's addressed now.

Comment 24

[Frank Yan (:fryn)]

Attachment: hg export for patch v4 [r=dao ui-r=faaborg]

Margaret kindly offered to land this later today. Thanks! :)

Comment 25

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/d37fedadd4ec

Comment 26

[kolubinowicki]

Attachment: Blue overflow button upon startup

Regression after landing (please check attachment). When browser session is restored, blue overflow button is visible for a few moment.

Comment 27

[Josh Tumath]

This bug seems to have triggered the return of bug 624157. Should I file a new bug or should that bug be reopened?

Comment 28

[Dão Gottwald [:dao]]

(In reply to comment #27)
> This bug seems to have triggered the return of bug 624157. Should I file a new
> bug or should that bug be reopened?

You should file a new bug.