Closed
Bug 663343
Opened 13 years ago
Closed 12 years ago
The "List all Tabs" menu should visually identify which tabs are on-screen (rather than scrolled off)
Categories
(SeaMonkey :: Tabbed Browser, defect)
SeaMonkey
Tabbed Browser
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: philip.chee, Assigned: philip.chee)
Details
Attachments
(6 files, 2 obsolete files)
3.87 KB,
patch
|
neil
:
review-
|
Details | Diff | Splinter Review |
5.79 KB,
patch
|
Details | Diff | Splinter Review | |
2.18 KB,
patch
|
Details | Diff | Splinter Review | |
6.53 KB,
patch
|
neil
:
review+
stefanh
:
review+
|
Details | Diff | Splinter Review |
6.92 KB,
patch
|
Details | Diff | Splinter Review | |
6.89 KB,
patch
|
philip.chee
:
review+
|
Details | Diff | Splinter Review |
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).
![]() |
Assignee | |
Comment 1•13 years ago
|
||
Not doing Mac/Classic because I remember Stefanh doesn't like fancy stuff like this.
Attachment #538471 -
Flags: review?(neil)
![]() |
Assignee | |
Comment 2•13 years ago
|
||
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)
![]() |
Assignee | |
Comment 3•13 years ago
|
||
Comment on attachment 541271 [details] [diff] [review] Patch v1.1 Include Mac Stefanh: please review the Mac changes.
Attachment #541271 -
Flags: review?(stefanh)
Comment 4•13 years ago
|
||
(In reply to comment #3) > Stefanh: please review the Mac changes. Hopefully I'll be able to look at this Saturday/Sunday.
Comment 5•13 years ago
|
||
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+
![]() |
Assignee | |
Comment 6•13 years ago
|
||
> 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)
Comment 7•13 years ago
|
||
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 9•13 years ago
|
||
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 10•13 years ago
|
||
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-
Comment 11•13 years ago
|
||
Just to throw another idea in to the ring, this activates the appropriate menuitem when you hover the tab.
Comment 12•13 years ago
|
||
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)
![]() |
Assignee | |
Comment 13•13 years ago
|
||
> 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 14•13 years ago
|
||
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.)
![]() |
Assignee | |
Comment 15•13 years ago
|
||
>>+ 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.
![]() |
Assignee | |
Comment 16•13 years ago
|
||
> 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)
Comment 18•13 years ago
|
||
(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•13 years ago
|
||
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 20•13 years ago
|
||
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+
Updated•12 years ago
|
Attachment #626028 -
Flags: review?(stefanh) → review+
![]() |
Assignee | |
Comment 22•12 years ago
|
||
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+
![]() |
Assignee | |
Comment 23•12 years ago
|
||
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/22b9eecf711b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•