Skip to content

Commit

Permalink
Improve tab strip layout in case of overflow, so that tabs will not d…
Browse files Browse the repository at this point in the history
…raw on top or

behind the fullscreen and profile buttons.
Calculate the maximum number of times we can show and hide the rest, similar
to how it works on Linux and Windows.
Also changed so that only the one active tab has a bigger minimum width (and
draws the close button) instead of potentially multiple selected tabs. This also
matches Linux and Windows.

BUG=392137

Review URL: https://codereview.chromium.org/476313003

Cr-Commit-Position: refs/heads/master@{#293654}
  • Loading branch information
andresantoso authored and Commit bot committed Sep 7, 2014
1 parent 53ffdf9 commit d564db4
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 58 deletions.
44 changes: 17 additions & 27 deletions chrome/browser/ui/cocoa/browser_window_controller_private.mm
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/command_line.h"
#include "base/mac/mac_util.h"
#import "base/mac/scoped_nsobject.h"
#import "base/mac/sdk_forward_declarations.h"
#include "base/prefs/pref_service.h"
#include "base/prefs/scoped_user_pref_update.h"
#include "chrome/browser/browser_process.h"
Expand Down Expand Up @@ -53,14 +54,6 @@
// Space between the incognito badge and the right edge of the window.
const CGFloat kAvatarRightOffset = 4;

// The amount by which to shrink the tab strip (on the right) when the
// incognito badge is present.
const CGFloat kAvatarTabStripShrink = 18;

// Width of the full screen icon. Used to position the AvatarButton to the
// left of the icon.
const CGFloat kFullscreenIconWidth = 32;

// Insets for the location bar, used when the full toolbar is hidden.
// TODO(viettrungluu): We can argue about the "correct" insetting; I like the
// following best, though arguably 0 inset is better/more correct.
Expand Down Expand Up @@ -349,6 +342,12 @@ - (CGFloat)layoutTabStripAtMaxY:(CGFloat)maxY
else
[tabStripController_ removeWindowControls];

// fullScreenButton is non-nil when isInAnyFullscreenMode is NO, and OS
// version is in the range 10.7 <= version <= 10.9. Starting with 10.10, the
// zoom/maximize button acts as the fullscreen button.
NSButton* fullScreenButton =
[[self window] standardWindowButton:NSWindowFullScreenButton];

// Lay out the icognito/avatar badge because calculating the indentation on
// the right depends on it.
NSView* avatarButton = [avatarButtonController_ view];
Expand All @@ -359,8 +358,8 @@ - (CGFloat)layoutTabStripAtMaxY:(CGFloat)maxY

if ([self shouldUseNewAvatarButton]) {
// The fullscreen icon is displayed to the right of the avatar button.
if (![self isInAnyFullscreenMode])
badgeXOffset -= kFullscreenIconWidth;
if (![self isInAnyFullscreenMode] && fullScreenButton)
badgeXOffset -= width - NSMinX([fullScreenButton frame]);
// Center the button vertically on the tabstrip.
badgeYOffset = (tabStripHeight - buttonHeight) / 2;
} else {
Expand All @@ -378,29 +377,20 @@ - (CGFloat)layoutTabStripAtMaxY:(CGFloat)maxY
}

// Calculate the right indentation. The default indentation built into the
// tabstrip leaves enough room for the fullscreen button or presentation mode
// toggle button on Lion. On non-Lion systems, the right indent needs to be
// tabstrip leaves enough room for the fullscreen button on Lion (10.7) to
// Mavericks (10.9). On 10.6 and >=10.10, the right indent needs to be
// adjusted to make room for the new tab button when an avatar is present.
CGFloat rightIndent = 0;
if (base::mac::IsOSLionOrLater() &&
[[self window] isKindOfClass:[FramedBrowserWindow class]]) {
FramedBrowserWindow* window =
static_cast<FramedBrowserWindow*>([self window]);
rightIndent += -[window fullScreenButtonOriginAdjustment].x;
if (![self isInAnyFullscreenMode] && fullScreenButton) {
rightIndent = width - NSMinX([fullScreenButton frame]);

if ([self shouldUseNewAvatarButton]) {
// The new avatar is wider than the default indentation, so we need to
// account for its width.
rightIndent += NSWidth([avatarButton frame]) + kAvatarTabStripShrink;

// When the fullscreen icon is not displayed, return its width to the
// tabstrip.
if ([self isInAnyFullscreenMode])
rightIndent -= kFullscreenIconWidth;
// The new avatar button is to the left of the fullscreen button.
// (The old avatar button is to the right).
rightIndent += NSWidth([avatarButton frame]) + kAvatarRightOffset;
}
} else if ([self shouldShowAvatar]) {
rightIndent += kAvatarTabStripShrink +
NSWidth([avatarButton frame]) + kAvatarRightOffset;
rightIndent += NSWidth([avatarButton frame]) + kAvatarRightOffset;
}
[tabStripController_ setRightIndentForControls:rightIndent];

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/cocoa/tabs/tab_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ class MenuDelegate;
@property(readonly, nonatomic) HoverCloseButton* closeButton;

// Minimum and maximum allowable tab width. The minimum width does not show
// the icon or the close button. The selected tab always has at least a close
// the icon or the close button. The active tab always has at least a close
// button so it has a different minimum width.
+ (CGFloat)minTabWidth;
+ (CGFloat)maxTabWidth;
+ (CGFloat)minSelectedTabWidth;
+ (CGFloat)minActiveTabWidth;
+ (CGFloat)miniTabWidth;
+ (CGFloat)appTabWidth;

Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/ui/cocoa/tabs/tab_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ virtual void ExecuteCommand(int command_id, int event_flags) OVERRIDE {
// of the two tab edge bitmaps because these bitmaps have a few transparent
// pixels on the side. The selected tab width includes the close button width.
+ (CGFloat)minTabWidth { return 36; }
+ (CGFloat)minSelectedTabWidth { return 52; }
+ (CGFloat)minActiveTabWidth { return 52; }
+ (CGFloat)maxTabWidth { return 214; }
+ (CGFloat)miniTabWidth { return 58; }
+ (CGFloat)appTabWidth { return 66; }
Expand Down Expand Up @@ -187,7 +187,7 @@ - (void)setTitle:(NSString*)title {
TabView* tabView = [self tabView];
[tabView setTitle:title];

if ([self mini] && ![self selected]) {
if ([self mini] && ![self active]) {
[tabView startAlert];
}
[super setTitle:title];
Expand Down Expand Up @@ -273,7 +273,7 @@ - (int)iconCapacity {

- (BOOL)shouldShowIcon {
return chrome::ShouldTabShowFavicon(
[self iconCapacity], [self mini], [self selected], iconView_ != nil,
[self iconCapacity], [self mini], [self active], iconView_ != nil,
!mediaIndicatorView_ ? TAB_MEDIA_STATE_NONE :
[mediaIndicatorView_ animatingMediaState]);
}
Expand All @@ -282,13 +282,13 @@ - (BOOL)shouldShowMediaIndicator {
if (!mediaIndicatorView_)
return NO;
return chrome::ShouldTabShowMediaIndicator(
[self iconCapacity], [self mini], [self selected], iconView_ != nil,
[self iconCapacity], [self mini], [self active], iconView_ != nil,
[mediaIndicatorView_ animatingMediaState]);
}

- (BOOL)shouldShowCloseButton {
return chrome::ShouldTabShowCloseButton(
[self iconCapacity], [self mini], [self selected]);
[self iconCapacity], [self mini], [self active]);
}

- (void)setIconImage:(NSImage*)image {
Expand Down
12 changes: 6 additions & 6 deletions chrome/browser/ui/cocoa/tabs/tab_controller_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -368,24 +368,24 @@ static void CheckForExpectedLayoutAndVisibilityOfSubviews(
NSView* newIcon = [controller iconView];
EXPECT_TRUE([newIcon isHidden]);

// Tab is at selected minimum width. Since it's selected, the close box
// Tab is at active minimum width. Since it's active, the close box
// should be visible.
[controller setSelected:YES];
[controller setActive:YES];
frame = [[controller view] frame];
frame.size.width = [TabController minSelectedTabWidth];
frame.size.width = [TabController minActiveTabWidth];
[[controller view] setFrame:frame];
EXPECT_FALSE([controller shouldShowIcon]);
EXPECT_TRUE([newIcon isHidden]);
EXPECT_TRUE([controller shouldShowCloseButton]);

// Test expanding the tab to max width and ensure the icon and close box
// get put back, even when de-selected.
// get put back, even when de-activated.
frame.size.width = [TabController maxTabWidth];
[[controller view] setFrame:frame];
EXPECT_TRUE([controller shouldShowIcon]);
EXPECT_FALSE([newIcon isHidden]);
EXPECT_TRUE([controller shouldShowCloseButton]);
[controller setSelected:NO];
[controller setActive:NO];
EXPECT_TRUE([controller shouldShowIcon]);
EXPECT_TRUE([controller shouldShowCloseButton]);

Expand Down Expand Up @@ -524,7 +524,7 @@ static void CheckForExpectedLayoutAndVisibilityOfSubviews(
tabFrame.size.width = minWidth = [TabController miniTabWidth];
} else {
tabFrame.size.width = [TabController maxTabWidth];
minWidth = isActiveTab ? [TabController minSelectedTabWidth] :
minWidth = isActiveTab ? [TabController minActiveTabWidth] :
[TabController minTabWidth];
}
while (NSWidth(tabFrame) >= minWidth) {
Expand Down
66 changes: 48 additions & 18 deletions chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ - (void)layoutTabsWithAnimation:(BOOL)animate

const CGFloat kMaxTabWidth = [TabController maxTabWidth];
const CGFloat kMinTabWidth = [TabController minTabWidth];
const CGFloat kMinSelectedTabWidth = [TabController minSelectedTabWidth];
const CGFloat kMinActiveTabWidth = [TabController minActiveTabWidth];
const CGFloat kMiniTabWidth = [TabController miniTabWidth];
const CGFloat kAppTabWidth = [TabController appTabWidth];

Expand All @@ -968,7 +968,8 @@ - (void)layoutTabsWithAnimation:(BOOL)animate
availableSpace = NSWidth([tabStripView_ frame]);

// Account for the width of the new tab button.
availableSpace -= NSWidth([newTabButton_ frame]) + kNewTabButtonOffset;
availableSpace -=
NSWidth([newTabButton_ frame]) + kNewTabButtonOffset - kTabOverlap;

// Account for the right-side controls if not in rapid closure mode.
// (In rapid closure mode, the available width is set based on the
Expand All @@ -980,34 +981,56 @@ - (void)layoutTabsWithAnimation:(BOOL)animate
// Need to leave room for the left-side controls even in rapid closure mode.
availableSpace -= [self leftIndentForControls];

// If there are any mini tabs, account for the extra spacing between the last
// mini tab and the first regular tab.
if ([self numberOfOpenMiniTabs])
availableSpace -= kLastMiniTabSpacing;

// This may be negative, but that's okay (taken care of by |MAX()| when
// calculating tab sizes). "mini" tabs in horizontal mode just get a special
// section, they don't change size.
CGFloat availableSpaceForNonMini = availableSpace;
availableSpaceForNonMini -=
[self numberOfOpenMiniTabs] * (kMiniTabWidth - kTabOverlap);
if ([self numberOfOpenMiniTabs]) {
availableSpaceForNonMini -=
[self numberOfOpenMiniTabs] * (kMiniTabWidth - kTabOverlap);
availableSpaceForNonMini -= kLastMiniTabSpacing;
}

// Initialize |nonMiniTabWidth| in case there aren't any non-mini-tabs; this
// value shouldn't actually be used.
CGFloat nonMiniTabWidth = kMaxTabWidth;
CGFloat nonMiniTabWidthFraction = 0;
const NSInteger numberOfOpenNonMiniTabs = [self numberOfOpenNonMiniTabs];
if (numberOfOpenNonMiniTabs) {
NSInteger numberOfNonMiniTabs = MIN(
[self numberOfOpenNonMiniTabs],
(availableSpaceForNonMini - kTabOverlap) / (kMinTabWidth - kTabOverlap));

if (numberOfNonMiniTabs) {
// Find the width of a non-mini-tab. This only applies to horizontal
// mode. Add in the amount we "get back" from the tabs overlapping.
availableSpaceForNonMini += (numberOfOpenNonMiniTabs - 1) * kTabOverlap;

// Divide up the space between the non-mini-tabs.
nonMiniTabWidth = availableSpaceForNonMini / numberOfOpenNonMiniTabs;
nonMiniTabWidth =
((availableSpaceForNonMini - kTabOverlap) / numberOfNonMiniTabs) +
kTabOverlap;

// Clamp the width between the max and min.
nonMiniTabWidth = MAX(MIN(nonMiniTabWidth, kMaxTabWidth), kMinTabWidth);

// When there are multiple tabs, we'll have one active and some inactive
// tabs. If the desired width was between the minimum sizes of these types,
// try to shrink the tabs with the smaller minimum. For example, if we have
// a strip of width 10 with 4 tabs, the desired width per tab will be 2.5.
// If selected tabs have a minimum width of 4 and unselected tabs have
// minimum width of 1, the above code would set *unselected_width = 2.5,
// *selected_width = 4, which results in a total width of 11.5. Instead, we
// want to set *unselected_width = 2, *selected_width = 4, for a total width
// of 10.
if (numberOfNonMiniTabs > 1 && nonMiniTabWidth < kMinActiveTabWidth) {
nonMiniTabWidth = (availableSpaceForNonMini - kMinActiveTabWidth) /
(numberOfNonMiniTabs - 1) +
kTabOverlap;
if (nonMiniTabWidth < kMinTabWidth) {
// The above adjustment caused the tabs to not fit, show 1 less tab.
--numberOfNonMiniTabs;
nonMiniTabWidth =
((availableSpaceForNonMini - kTabOverlap) / numberOfNonMiniTabs) +
kTabOverlap;
}
}

// Separate integral and fractional parts.
CGFloat integralPart = std::floor(nonMiniTabWidth);
nonMiniTabWidthFraction = nonMiniTabWidth - integralPart;
Expand Down Expand Up @@ -1092,16 +1115,16 @@ - (void)layoutTabsWithAnimation:(BOOL)animate
}

// In case of rounding error, give any left over pixels to the last tab.
if (laidOutNonMiniTabs == numberOfOpenNonMiniTabs - 1 &&
if (laidOutNonMiniTabs == numberOfNonMiniTabs - 1 &&
tabWidthAccumulatedFraction > 0.5) {
++tabFrame.size.width;
}

++laidOutNonMiniTabs;
}

if ([tab selected])
tabFrame.size.width = MAX(tabFrame.size.width, kMinSelectedTabWidth);
if ([tab active])
tabFrame.size.width = MAX(tabFrame.size.width, kMinActiveTabWidth);

// If this is the first non-mini tab, then add a bit of spacing between this
// and the last mini tab.
Expand All @@ -1111,6 +1134,13 @@ - (void)layoutTabsWithAnimation:(BOOL)animate
}
isLastTabMini = isMini;

if (laidOutNonMiniTabs > numberOfNonMiniTabs) {
// There is not enough space to fit this tab.
tabFrame.size.width = 0;
[self setFrame:tabFrame ofTabView:[tab view]];
continue;
}

// Animate a new tab in by putting it below the horizon unless told to put
// it in a specific location (i.e., from a drop).
if (newTab && visible && animate) {
Expand Down

0 comments on commit d564db4

Please sign in to comment.