Closed Bug 663343 Opened 9 years ago Closed 8 years ago

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

Categories

(SeaMonkey :: Tabbed Browser, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: philip.chee)

Details

Attachments

(6 files, 2 obsolete files)

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).
Attached patch Patch v1.0 (obsolete) — Splinter Review
Not doing Mac/Classic because I remember Stefanh doesn't like fancy stuff like this.
Attachment #538471 - Flags: review?(neil)
Attached patch Patch v1.1 Include Mac (obsolete) — Splinter Review
Stefanh says he wants this for Mac as well.
Attachment #538471 - Attachment is obsolete: true
Attachment #538471 - Flags: review?(neil)
Attachment #541271 - Flags: review?(neil)
Comment on attachment 541271 [details] [diff] [review]
Patch v1.1 Include Mac

Stefanh: please review the Mac changes.
Attachment #541271 - Flags: review?(stefanh)
(In reply to comment #3) 
> Stefanh: please review the Mac changes.
Hopefully I'll be able to look at this Saturday/Sunday.
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?).
Attachment #541271 - Flags: review?(stefanh) → review+
> 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?
Attachment #619428 - Flags: review?(stefanh)
Attachment #619428 - Flags: review?(neil)
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.
Attaching for completeness although Ratty said he didn't like the idea.
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.)
Attachment #619428 - Flags: review?(neil) → review-
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.
Attachment #541271 - Flags: review?(neil) → review-
Just to throw another idea in to the ring, this activates the appropriate menuitem when you hover the tab.
Comment on attachment 619428 [details] [diff] [review]
Patch 2.0 Use Opacity to grey out invisible tabs instead.

Removing obsolete request.
Attachment #619428 - Flags: review?(stefanh)
> 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;
Attachment #626028 - Flags: review?(stefanh)
Attachment #626028 - Flags: review?(neil)
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.)
>>+              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.
> 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.
Attachment #541271 - Attachment is obsolete: true
Attachment #627664 - Flags: review?(neil)
OK, now I remember why I didn't like opacity. (Not that I like italic...)
(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 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.
Attachment #627664 - Attachment is patch: true
Attachment #627664 - Flags: review?(neil)
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.
Attachment #626028 - Flags: review?(neil) → review+
I'll look at this in a couple of days.
Attachment #626028 - Flags: review?(stefanh) → review+
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.
Attachment #630259 - Flags: review+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/22b9eecf711b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.