Last Comment Bug 663343 - The "List all Tabs" menu should visually identify which tabs are on-screen (rather than scrolled off)
: The "List all Tabs" menu should visually identify which tabs are on-screen (r...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-10 02:33 PDT by Philip Chee
Modified: 2012-06-05 12:04 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 (3.06 KB, patch)
2011-06-10 02:38 PDT, Philip Chee
no flags Details | Diff | Review
Patch v1.1 Include Mac (4.49 KB, patch)
2011-06-22 20:11 PDT, Philip Chee
neil: review-
stefanh: review+
Details | Diff | Review
Patch 2.0 Use Opacity to grey out invisible tabs instead. (3.87 KB, patch)
2012-04-29 11:35 PDT, Philip Chee
neil: review-
Details | Diff | Review
Use menuseparators to delimit visible tabs (5.79 KB, patch)
2012-05-03 12:57 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Review
Activate menuitem on tab hover (2.18 KB, patch)
2012-05-03 13:27 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Review
Patch 2.1 Use Italics for overflow tabs. (6.53 KB, patch)
2012-05-22 08:37 PDT, Philip Chee
neil: review+
stefanh: review+
Details | Diff | Review
Patch v2.2 Use Opacity for overflow tabs. Only add attribute for hidden tabs. (6.92 KB, patch)
2012-05-28 04:39 PDT, Philip Chee
no flags Details | Diff | Review
Patch v3.0 as checked in r=stefanh r=Neil. (6.89 KB, patch)
2012-06-05 12:04 PDT, Philip Chee
philip.chee: review+
Details | Diff | Review

Description Philip Chee 2011-06-10 02:33:33 PDT
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 2011-06-10 02:38:23 PDT
Created attachment 538471 [details] [diff] [review]
Patch v1.0

Not doing Mac/Classic because I remember Stefanh doesn't like fancy stuff like this.
Comment 2 Philip Chee 2011-06-22 20:11:11 PDT
Created attachment 541271 [details] [diff] [review]
Patch v1.1 Include Mac

Stefanh says he wants this for Mac as well.
Comment 3 Philip Chee 2011-06-22 20:12:25 PDT
Comment on attachment 541271 [details] [diff] [review]
Patch v1.1 Include Mac

Stefanh: please review the Mac changes.
Comment 4 Stefan [:stefanh] 2011-06-23 10:58:00 PDT
(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] 2011-06-25 15:26:10 PDT
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 2012-04-29 11:35:53 PDT
Created attachment 619428 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2012-05-03 09:21:01 PDT
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 neil@parkwaycc.co.uk 2012-05-03 12:57:48 PDT
Created attachment 620824 [details] [diff] [review]
Use menuseparators to delimit visible tabs

Attaching for completeness although Ratty said he didn't like the idea.
Comment 9 neil@parkwaycc.co.uk 2012-05-03 13:14:07 PDT
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 neil@parkwaycc.co.uk 2012-05-03 13:17:05 PDT
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 neil@parkwaycc.co.uk 2012-05-03 13:27:34 PDT
Created attachment 620832 [details] [diff] [review]
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] 2012-05-13 06:43:20 PDT
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 2012-05-22 08:37:59 PDT
Created attachment 626028 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2012-05-22 16:27:02 PDT
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 2012-05-22 21:10:32 PDT
>>+              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 2012-05-28 04:39:56 PDT
Created attachment 627664 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2012-05-28 06:58:33 PDT
OK, now I remember why I didn't like opacity. (Not that I like italic...)
Comment 18 neil@parkwaycc.co.uk 2012-05-28 07:09:34 PDT
(In reply to neil@parkwaycc.co.uk 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 neil@parkwaycc.co.uk 2012-05-28 07:34:01 PDT
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 neil@parkwaycc.co.uk 2012-05-28 07:36:53 PDT
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] 2012-05-28 12:55:20 PDT
I'll look at this in a couple of days.
Comment 22 Philip Chee 2012-06-05 12:04:04 PDT
Created attachment 630259 [details] [diff] [review]
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 2012-06-05 12:04:36 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/22b9eecf711b

Note You need to log in before you can comment on or make changes to this bug.