663343 - The "List all Tabs" menu should visually identify which tabs are on-screen (rather than scrolled off)

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Philip Chee]

From: Firefox Bug 626903 The "List all Tabs" menu should visually identify which tabs are on-screen (rather than scrolled off)

> The List all Tabs menu should visually display the set of tabs on screen.  
> This will help users to quickly identify the areas outside of the current 
> set of tabs displayed (that likely contain the tab that they are actually 
> looking for).

Comment 1

[Philip Chee]

Attachment: Patch v1.0

Not doing Mac/Classic because I remember Stefanh doesn't like fancy stuff like this.

Comment 2

[Philip Chee]

Attachment: Patch v1.1 Include Mac

Stefanh says he wants this for Mac as well.

Comment 3

[Philip Chee]

Comment on attachment 541271 [details] [diff] [review]
Patch v1.1 Include Mac

Stefanh: please review the Mac changes.

Comment 4

[Stefan [:stefanh]]

(In reply to comment #3) 
> Stefanh: please review the Mac changes.
Hopefully I'll be able to look at this Saturday/Sunday.

Comment 5

[Stefan [:stefanh]]

Comment on attachment 541271 [details] [diff] [review]
Patch v1.1 Include Mac

Thanks. Yeah, in this case it makes sense to do the same for all platforms ;-)

One possible objection would be that it might have made sense to instead style the menu items (tabs) not in sight (greyed out?).

Comment 6

[Philip Chee]

Attachment: Patch 2.0 Use Opacity to grey out invisible tabs instead.

> One possible objection would be that it might have made sense to instead style
> the menu items (tabs) not in sight (greyed out?).
OK here is a patch that does this. What do you think?

Comment 7

[[e-mail]]

Sorry, this looks poor in Modern and worse in Classic.

However, I have managed to come up with an alternate suggestion, which is simply to insert menuseparators between menuitems corresponding to on-screen and off-screen tabs, something like this:
                  [v]  
+-------------------+
|So Much Pun        |
|Set Phasers To LOL |
|-------------------|
|Failbook           |
|WIN!               |
|Learn From My Fail |
|Monday Thru Friday |
|Autocowrecks       |
|-------------------|
|Memebase           |
|Very Demotivational|
|GraphJam           |
|Comixed            |
|Superheroes        |
|Rage Comics        |
+-------------------+

I'm not asking you to implement this, I just wanted to run the idea past you.

Comment 8

[[e-mail]]

Attachment: Use menuseparators to delimit visible tabs

Attaching for completeness although Ratty said he didn't like the idea.

Comment 9

[[e-mail]]

Comment on attachment 619428 [details] [diff] [review]
Patch 2.0 Use Opacity to grey out invisible tabs instead.

This also greys out all the tabs if there is no overflow... your suggestion of using italics has the same issue. (Otherwise it's the best idea so far.)

Comment 10

[[e-mail]]

Comment on attachment 541271 [details] [diff] [review]
Patch v1.1 Include Mac

Sorry, but I'm really not keen on the visual effect of this patch.

Comment 11

[[e-mail]]

Attachment: Activate menuitem on tab hover

Just to throw another idea in to the ring, this activates the appropriate menuitem when you hover the tab.

Comment 12

[Stefan [:stefanh]]

Comment on attachment 619428 [details] [diff] [review]
Patch 2.0 Use Opacity to grey out invisible tabs instead.

Removing obsolete request.

Comment 13

[Philip Chee]

Attachment: Patch 2.1 Use Italics for overflow tabs.

> This also greys out all the tabs if there is no overflow... your suggestion
> of using italics has the same issue. (Otherwise it's the best idea so far.)

1. This patch modifies _updateTabsVisibilityStatus() so that the tabIsVisible attribute is always applied even if there is no overflow.
2. Uses font-style: italic;

Comment 14

[[e-mail]]

Comment on attachment 626028 [details] [diff] [review]
Patch 2.1 Use Italics for overflow tabs.

>-            // We don't want menu item decoration unless there is overflow.
>-            if (tabContainer.getAttribute("overflow") != "true")
>-              return;
Perhaps it would be better to modify the CSS so that it only styles the invisible items if the tabcontainer is overflowing?

>+              if (underflow ||
>+                  curTabBO.screenX >= tabstripBO.screenX &&
>                   curTabBO.screenX + curTabBO.width <= tabstripBO.screenX + tabstripBO.width)
I doubt this will be false if the tab strip is not overflowing...

>+.alltabs-item:not([tabIsVisible]) {
>+  font-style: italic;
I can't remember whether I preferred the opacity or the italic. Would you mind running up another opacity patch for comparison? (Preferably using the above suggestion to restrict the opacity to overflowing tabstrips, of course.)

Comment 15

[Philip Chee]

>>+              if (underflow ||
>>+                  curTabBO.screenX >= tabstripBO.screenX &&
>>                   curTabBO.screenX + curTabBO.width <= tabstripBO.screenX + tabstripBO.width)
> I doubt this will be false if the tab strip is not overflowing...
That's the point of this part of the patch. Otherwise I'm not following you.

Comment 16

[Philip Chee]

Attachment: Patch v2.2 Use Opacity for overflow tabs. Only add attribute for hidden tabs.

> Perhaps it would be better to modify the CSS so that it only styles the
> invisible items if the tabcontainer is overflowing?
Fixed.

>>+.alltabs-item:not([tabIsVisible]) {
>>+  font-style: italic;
> I can't remember whether I preferred the opacity or the italic. Would you
> mind running up another opacity patch for comparison? (Preferably using
> the above suggestion to restrict the opacity to overflowing tabstrips, of
> course.)
Done.

Comment 17

[[e-mail]]

OK, now I remember why I didn't like opacity. (Not that I like italic...)

Comment 18

[[e-mail]]

(In reply to [e-mail] from comment #17)
> OK, now I remember why I didn't like opacity. (Not that I like italic...)
Maybe we could just apply the opacity to the icons and/or text? The problem with applying it to the whole menuitem is that it has a background colour.

Comment 19

[[e-mail]]

Comment on attachment 627664 [details] [diff] [review]
Patch v2.2 Use Opacity for overflow tabs. Only add attribute for hidden tabs.

>+            let underflow = tabContainer.getAttribute("overflow") != "true";
Don't actually need to check for underflow, if we're not overflowing then all the tabs will be visible anyway.

Comment 20

[[e-mail]]

Comment on attachment 626028 [details] [diff] [review]
Patch 2.1 Use Italics for overflow tabs.

OK, so this is the least worst option, but I prefer the tabIsHidden attribute (although maybe tabIsScrolled would be more accurate), and the simplification of the overflow check that I commented on for the other patch.

Comment 21

[Stefan [:stefanh]]

I'll look at this in a couple of days.

Comment 22

[Philip Chee]

Attachment: Patch v3.0 as checked in r=stefanh r=Neil.

Changes in this patch:

1. Uses an attribute name "tabIsScrolled"
2. CSS only styles the invisible items if the tabcontainer is overflowing.
3. Uses Italics for overflow tabs.

Comment 23

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/22b9eecf711b