From Bug 319777 comment 107: "we need a new bug covering the need for some kind of direct menu (menu bar, extra tab bar widget/widgets, or both) access to specific tabs." To expand: with the introduction of the scrolling tab bar, one strength we lose from the overflow menu is the ability to quickly focus an arbitrary tab from a long list. This is an extremely important ability for the use-case with lots (>50) of tabs, and quite useful for any number of tabs over a window's width. One long-standing idea for alleviating this problem has been listing all the tabs in the Window menu or a "Tab Palette." These are both strong ideas, which have had a lot of good discussion, which is centered around bug 197720. Since that bug existed before 319777's conception, I believe we should leave that as it is, with the plan to implement either a menu or palette with all the tabs listed. In addition, however, we should return some sort of access to all tabs from the tab bar itself. Ideas discussed for doing so include: - Have menus, invoked by control-click, on each overflow button which exposes all tabs to that side of the current tab view. This has the advantage of strengthening the metaphor of the tabs as a single strip. It also has the disadvantage of breaking the "invisible" tabs into two groups - if you didn't know which side your tab was on, you might have to check both menus. - Have a single menu, invoked either through a new widget on the tab bar or by some other means, which lists all tabs. This could have separators to indicate where the tabs lie in relation to the current tab view, but it has the disadvantage of weakening the already shaky association with the correct "side" for tabs. However, it has the advantage of keeping all tabs available in a single place. Take your pick really (or provide another idea). I support the first, since I think the new "scrolled tab strip" metaphor needs as much support as it can get.

> Have menus, invoked by control-click, on each overflow button which exposes > all tabs to that side of the current tab view. This has the advantage of > strengthening the metaphor of the tabs as a single strip. It also has the > disadvantage of breaking the "invisible" tabs into two groups - if you didn't > know which side your tab was on, you might have to check both menus. Since this will mainly be needed when you have to many tabs open to remeber to which side the wanted tab is, I think the disadvantage of this strongly outweights the advantage. IMHO it will be so useless that the time used to implement this would be wasted. > - Have a single menu, invoked either through a new widget on the tab bar or > by some other means, which lists all tabs. This could have separators to > indicate where the tabs lie in relation to the current tab view, but it has > the disadvantage of weakening the already shaky association with the correct > "side" for tabs. However, it has the advantage of keeping all tabs available > in a single place. If we're able to implement this in a way that fixes the original bug 319777 (scroll and select the active tab - or maybe the current visible tabs), I think the association will be good enough.

A menu pop-up button has been added to the right of the tab bar. Internet Explorer 7 and Firefox 2 take this approach so it seems reasonable to follow what people will be used to. The dividers are intended to give the impression of which tabs are visible in the tab bar. Clicking on a tab to the left makes it left-most, clicking on a tab to the right makes it right-most. There are some issues with this patch - it places the button up by 1 pixel to make it seem aligned, the button is drawn at 1.5 * distance which is another cheap hack to make things look visually appealing. If this approach to this bug is accepted then I will tidy these things up.

Comment on attachment 243930 [details] [diff] [review] Adds a new button to the right of the bar with a menu. Based on 319777 WIP7 This will definitely be a big usability boost in the many-tab case. Behaviorally, the only real issue here is that to address the problem of the right overflow becoming a second-class citizen, the decision was to have this menu behave like a pop-up, not a pull-down, so that the current tab is under the cursor when the menu appears. We'll have to think about what that means for the icon (in another bug; we'll go ahead with the down arrow for now unless a new icon is quick to appear). Nitpicky stuff: First, there are some really long lines here; break them up into a few lines as necessary. >+ if (mOverflowTabs) >+ [mOverflowMenu addItem:[NSMenuItem separatorItem]]; This needs a more specific condition to make sure there are actually tabs overflowing on the left. > -(void)drawButtons { >... >- [mOverflowRightButton setFrame:NSMakeRect(NSMaxX([self tabsRect]), ([self tabBarHeight] - kOverflowButtonHeight) / 2, kOverflowButtonWidth, kOverflowButtonHeight)]; >+ [mOverflowRightButton setFrame:NSMakeRect(NSMaxX([self tabsRect]) - [mButtonDividerImage size].width, ([self tabBarHeight] - kOverflowButtonHeight) / 2, kOverflowButtonWidth, kOverflowButtonHeight)]; >... >+ [mOverflowMenuButton setFrame:NSMakeRect(NSMaxX([self tabsRect]) + [mButtonDividerImage size].width + kOverflowButtonWidth, (([self tabBarHeight] - kOverflowButtonHeight) / 2) + 1, 1.5*kOverflowButtonWidth, kOverflowButtonHeight)]; Get [self tabsRect] once and store it, instead of calling it twice. >+ - (IBAction)overflowMenu:(id)sender showOverflowMenu: >+ rect.size.width -= 3 * kTabBarMarginWhenOverflowTabs; I don't think we want to bind ourselves to an implementation where the overflow button must be the same size as the scroll buttons. Make a new constant for the width of the overflow button, and do math like this accordingly.

Does selecting a tab from the menu in the current approach scroll all the tabs whichever way to bring the selected tab into the visible tab bar (e.g., the last part of torben's comment 1)?

(In reply to comment #5) > Does selecting a tab from the menu in the current approach scroll all the tabs > whichever way to bring the selected tab into the visible tab bar Yep, we made sure that was part of the original implementation.

(In reply to comment #4) > First, there are some really long lines here; break them up into a few lines as > necessary. Done > >+ if (mOverflowTabs) > >+ [mOverflowMenu addItem:[NSMenuItem separatorItem]]; > > This needs a more specific condition to make sure there are actually tabs > overflowing on the left. > Done. > > > -(void)drawButtons { > >... > >- [mOverflowRightButton setFrame:NSMakeRect(NSMaxX([self tabsRect]), ([self tabBarHeight] - kOverflowButtonHeight) / 2, kOverflowButtonWidth, kOverflowButtonHeight)]; > >+ [mOverflowRightButton setFrame:NSMakeRect(NSMaxX([self tabsRect]) - [mButtonDividerImage size].width, ([self tabBarHeight] - kOverflowButtonHeight) / 2, kOverflowButtonWidth, kOverflowButtonHeight)]; > >... > >+ [mOverflowMenuButton setFrame:NSMakeRect(NSMaxX([self tabsRect]) + [mButtonDividerImage size].width + kOverflowButtonWidth, (([self tabBarHeight] - kOverflowButtonHeight) / 2) + 1, 1.5*kOverflowButtonWidth, kOverflowButtonHeight)]; > > Get [self tabsRect] once and store it, instead of calling it twice. Done > >+ - (IBAction)overflowMenu:(id)sender > > showOverflowMenu: Done > Make a new constant for > the width of the overflow button, and do math like this accordingly. Done.

Comment on attachment 248665 [details] [diff] [review] Second patch, addresses all of the previous issues >+static const int kTabBarMarginForOverflowMenu = 10; // margin for overflow menu button >+static const float kOverflowMenuButtonWidth = 12; I don't think we need both of these constants. Unless I'm missing something, we just want to increase the margin by the width of the tab button, meaning we only need the second one. >+ if ([[mOverflowMenu itemAtIndex:0] title] == @"Dummy Item") { >+ // No point in adding the dummy item to the menu so remove it. >+ [mOverflowMenu removeItemAtIndex:0]; ... >+ // insert dummy item then remove any items on the menu other than the dummy item >+ [mOverflowMenu insertItem:[[NSMenuItem alloc] initWithTitle:@"Dummy Item" action:NULL keyEquivalent:@""] atIndex:0]; Why are you constantly adding and removing a dummy item if you don't want it to be there?

Hi Desmond, I've been playing around on the trunk lately, and decided to have a crack at this to help the trunk-experience some - it's been a while since the last patch, so I hope you don't mind my taking this bug under my wings. This patch still has some little button-placement niggles, which I think are probably best-covered by Bug 374623 and the bugs it mentions. In my opinion, the only thing we should really do as far as button-placement goes in this bug is to make sure that tab_menu_button is 12x16px.

Comment on attachment 259134 [details] [diff] [review] Patch v3 Nice work, delliott and froodian. I like the behavior this patch is introducing and especially favor the choice of a pop up button over its drop down counterpart. I think we should probably enhance and polish up the appearance of the control's UI just a bit more. Some suggestions for doing that: Avoid using the same ">" triangle image we're using for scroll buttons, since the click action behavior each responds with is completely different. The left and right scroll triangles do not display a menu and take action immediately. A similar appearance might indicate similar behavior. I think we should match behavior of NSToolbar's overflow menu control, the ">>", and just invert it downward. This might present the user with something familiar and allow them to more intuitively realize that it displays a menu for clipped items. Another possible enhancement is to draw a lighter gray area behind the control when the mouse is hovering over either scroll button or the overflow menu button, matching the hover appearance of the regular tab buttons. (e.g. drawTiledInRect:origin:operation: with "tab_hover.tif"). This also means tracking the entire rectangle on each end as opposed to only recognizing a click inside the actual image bounds (as we currently do). One last thought is to skip the divider image in between the right scroll button and the overflow menu button [screenshot of this attached]. This suggestion is kinda mutually exclusive with the prior one, though, since drawing a lighter background on mouse hover might not look right without a border/divider. For those following along who are unable to test the patch directly but have an opinion to offer on the UI look, I've attached a screenshot to demonstrate the current appearance of the overflow items. Also, the drawing locations and rects need to be slightly adjusted for each of the overflow items. Those are separate issues: Bug 355558, Bug 355790, and Bug 374623. froodian reported all three of them, so he's aware ;) Focusing on the rest of the code… > + if (mOverflowTabs) > + [mButtonDividerImage compositeToPoint:NSMakePoint(NSMaxX([self tabsRect]) + kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) > + operation:NSCompositeSourceOver]; Fetching [self tabsRect] doesn't seem to be necessary, since the value of the local |tabsRect| variable from earlier in the method is still relevant. This reference would stretch |tabsRect| out for a longer span, though. > // The button divider image should only be drawn if the left most visible tab is not selected. > if ([mTabView indexOfTabViewItem:[mTabView selectedTabViewItem]] != mLeftMostVisibleTabIndex) { > if (mOverflowTabs) > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) operation:NSCompositeSourceOver]; > if (mOverflowTabs) { > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) > operation:NSCompositeSourceOver]; > } > else > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMargin - [mButtonDividerImage size].width, 0) operation:NSCompositeSourceOver]; > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMargin - [mButtonDividerImage size].width, 0) > operation:NSCompositeSourceOver]; > } > if (mOverflowTabs) > [mButtonDividerImage compositeToPoint:NSMakePoint(NSMaxX([self tabsRect]) + kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) > operation:NSCompositeSourceOver]; The drawing of these button divider images is unclear until you've looked at this code a while or debugged through it. Since we reuse |mButtonDividerImage|, it's difficult to comprehend the purpose (or location) of each divider drawing based solely upon deciphering each inline NSMakePoint() calculation. Littering more comments can often be a distraction from the code itself and not always represent the best ultimate solution, but I think doing so could have merit here since there's no quick way to glance at all that point calculation code and determine what's going on. Comments like the following might be useful for the next time we have to look at this code: // Draws the leftmost button image divider (right sides are drawn by the buttons themselves). // A divider is not needed if the leftmost button is selected. > if ([mTabView indexOfTabViewItem:[mTabView selectedTabViewItem]] != mLeftMostVisibleTabIndex) { > if (mOverflowTabs) > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) operation:NSCompositeSourceOver]; > if (mOverflowTabs) { > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) > operation:NSCompositeSourceOver]; > } > else > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMargin - [mButtonDividerImage size].width, 0) operation:NSCompositeSourceOver]; > [mButtonDividerImage compositeToPoint:NSMakePoint(kTabBarMargin - [mButtonDividerImage size].width, 0) > operation:NSCompositeSourceOver]; > } // Draw a divider to the left of the overflow menu button > if (mOverflowTabs) > [mButtonDividerImage compositeToPoint:NSMakePoint(NSMaxX([self tabsRect]) + kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) > operation:NSCompositeSourceOver]; Or, the other approach is to use some descriptive NSPoint variables to self-document in code where the dividers will be drawn. I guess you can consider this idea optional, since adding either comments or variables could have its downfalls. > -(void)drawButtons { The name of this method, while it wasn't introduced in this particular patch, is unclear since we also refer to the each page tab as a 'button.' Maybe change this name to a more explicit |drawOverflowButtons|, since it only seems to be concerned with overflow buttons and does nothing to the tab buttons. - Considering that the last two items I mentioned aren't show stoppers or even required, the only pressing fix is removing the call to [self tabsRect]. With that addressed and my other two suggestions considered, r=me.

Appearance of the overflow UI as of patch v3. At the very least, I think the overflow menu down triangle needs to be centered in that last rectangle.

A visual example for one of my UI suggestions. This is probably my least favorite of the three ideas I mentioned in comment 10, but the easiest to mockup into a screenshot :)

Since there are separate controls there, I think they need separators ;) Also, since we're mimicking a popup menu, how about using the up/down arrow that those menus use?

Comment on attachment 259134 [details] [diff] [review] Patch v3 I like that autorelease NSMenu it is a more elegant approach than adding each menu item when the tabs are drawn. More comments are needed before this goes into trunk because as we move towards 1.6 this file is only going to grow. We also need to get the image that represents the menu button nailed down. r+ with more descriptive comments in a respin sr? targetted at smorgan

This addresses the code comments in the above reviews.

Comment on attachment 260894 [details] [diff] [review] Patch v4 Looks good. Using something other than placeholder images should be filed as a follow-up bug. >+ [mButtonDividerImage compositeToPoint:NSMakePoint(NSMaxX(tabsRect) + kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) Break this line up into something like: [mButtonDividerImage compositeToPoint:NSMakePoint(NSMaxX(tabsRect) + kTabBarMarginWhenOverflowTabs - [mButtonDividerImage size].width, 0) >+ [mOverflowLeftButton setFrame:NSMakeRect(0, (tabBarHeight - kOverflowButtonHeight) / 2, >+ kOverflowButtonWidth, kOverflowButtonHeight)]; Change all of the setFrame:NSMakeRect(a, b, c, d) to setFrameOrigin:NSMakePoint(a, b) since the sizes don't ever change. >+ rect.size.width -= ((2 * kTabBarMarginWhenOverflowTabs) + [mButtonDividerImage size].width + kOverflowMenuButtonWidth); Remove the outer parens. Also, there's a lot of trailing whitespace in this patch. Don't worry about the whole file, but strip it off all the lines that you are adding or touching. sr=smorgan with those changes.

Uploading for posterity (and for future branch-landing). Also removed kOverflowButtonMargin, since it's unused.

Above patch checked in on trunk.

Project patch to add the new image.

Image and project patch checked in on trunk. This should be really-truly fixed on trunk now.

bug 359483 will deal with the image for the menu button.