From 04a3942cf4419e0f21105934e92168583e1d49c4 Mon Sep 17 00:00:00 2001 From: "viettrungluu@chromium.org" Date: Fri, 4 Sep 2009 20:43:14 +0000 Subject: (Mac) Display the bookmark bar off-the-side menu on mouse down. Also align the menu. (Basically, make it like Safari's button.) We make the button into a |MenuButton|, attach a fixed menu to it in the nib, and make the |BookmarkBarController| the delegate for this menu so that we can update the menu just before it's displayed. As a side effect, also fixes bug 20813. BUG=21093,20813 TEST=Make sure the bookmark bar off-the-side button works properly under a variety of circumstances, making sure the menu is properly aligned, etc. Review URL: http://codereview.chromium.org/199024 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@25507 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/nibs/BookmarkBar.xib | 224 ++++++++++++++------- chrome/browser/cocoa/bookmark_bar_controller.h | 6 +- chrome/browser/cocoa/bookmark_bar_controller.mm | 40 ++-- .../cocoa/bookmark_bar_controller_unittest.mm | 43 ++++ chrome/browser/cocoa/menu_button.h | 8 +- chrome/browser/cocoa/menu_button.mm | 2 +- 6 files changed, 234 insertions(+), 89 deletions(-) (limited to 'chrome') diff --git a/chrome/app/nibs/BookmarkBar.xib b/chrome/app/nibs/BookmarkBar.xib index 30beaba..c2b1784 100644 --- a/chrome/app/nibs/BookmarkBar.xib +++ b/chrome/app/nibs/BookmarkBar.xib @@ -2,15 +2,16 @@ 1050 - 9L30 + 9L31a 677 949.54 353.00 YES - - + + + YES @@ -36,6 +37,26 @@ NSApplication + + + + YES + + + + + 2147483647 + + NSImage + NSMenuCheckmark + + + NSImage + NSMenuMixedState + + + + -2147483358 @@ -85,30 +106,24 @@ ^IDS_BOOMARK_BAR_OPEN_ALL 2147483647 - - NSImage - NSMenuCheckmark - - - NSImage - NSMenuMixedState - + + ^IDS_BOOMARK_BAR_OPEN_ALL_NEW_WINDOW 2147483647 - - + + ^IDS_BOOMARK_BAR_OPEN_ALL_INCOGNITO 2147483647 - - + + @@ -117,24 +132,24 @@ 2147483647 - - + + ^IDS_BOOKMARK_BAR_RENAME_FOLDER 2147483647 - - + + ^IDS_BOOKMARK_BAR_REMOVE 2147483647 - - + + @@ -143,24 +158,24 @@ 2147483647 - - + + ^IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK 2147483647 - - + + ^IDS_BOOMARK_BAR_NEW_FOLDER 2147483647 - - + + @@ -169,16 +184,16 @@ 2147483647 - - + + ^IDS_BOOKMARK_MANAGER 2147483647 - - + + 40011 @@ -186,8 +201,8 @@ ^IDS_BOOMARK_BAR_ALWAYS_SHOW 2147483647 - - + + 40009 @@ -201,24 +216,24 @@ ^IDS_BOOMARK_BAR_OPEN_IN_NEW_TAB 2147483647 - - + + ^IDS_BOOMARK_BAR_OPEN_IN_NEW_WINDOW 2147483647 - - + + ^IDS_BOOMARK_BAR_OPEN_INCOGNITO 2147483647 - - + + @@ -227,24 +242,24 @@ 2147483647 - - + + ^IDS_BOOKMARK_BAR_EDIT 2147483647 - - + + ^IDS_BOOKMARK_BAR_REMOVE 2147483647 - - + + @@ -253,24 +268,24 @@ 2147483647 - - + + ^IDS_BOOMARK_BAR_ADD_NEW_BOOKMARK 2147483647 - - + + ^IDS_BOOMARK_BAR_NEW_FOLDER 2147483647 - - + + @@ -279,16 +294,16 @@ 2147483647 - - + + ^IDS_BOOKMARK_MANAGER 2147483647 - - + + 40011 @@ -296,8 +311,8 @@ ^IDS_BOOMARK_BAR_ALWAYS_SHOW 2147483647 - - + + 40009 @@ -486,14 +501,6 @@ 66 - - openOffTheSideMenuFromButton: - - - - 69 - - menu @@ -525,6 +532,22 @@ 74 + + + menu_ + + + + 79 + + + + delegate + + + + 81 + @@ -751,6 +774,21 @@ + + 75 + + + YES + + + + Off-the-side Menu + + + 76 + + + @@ -788,10 +826,15 @@ 5.IBPluginDependency 6.IBPluginDependency 62.IBPluginDependency + 63.CustomClassName 63.IBPluginDependency + 64.CustomClassName 64.IBPluginDependency 7.IBPluginDependency 72.IBPluginDependency + 75.IBEditorWindowLastContentRect + 75.IBPluginDependency + 76.IBPluginDependency 8.IBPluginDependency 9.IBPluginDependency @@ -800,16 +843,18 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - {{519, 477}, {600, 144}} + {{96, 477}, {600, 144}} + com.apple.InterfaceBuilder.CocoaPlugin + com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin + {{380, 543}, {365, 213}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - {{859, 729}, {365, 213}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin @@ -821,15 +866,18 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin + {{370, 543}, {375, 213}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - {{655, 726}, {375, 213}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin + MenuButton com.apple.InterfaceBuilder.CocoaPlugin + ClickHoldButtonCell com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin + {{0, 722}, {75, 23}} com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin @@ -856,7 +904,7 @@ - 74 + 81 @@ -909,7 +957,7 @@ YES NSMenu NSView - NSButton + MenuButton id id @@ -936,6 +984,18 @@ + ClickHoldButtonCell + GradientButtonCell + + clickHoldTarget_ + id + + + IBProjectSource + browser/cocoa/clickhold_button_cell.h + + + GTMUILocalizer NSObject @@ -959,6 +1019,26 @@ + GradientButtonCell + NSButtonCell + + IBProjectSource + browser/cocoa/gradient_button_cell.h + + + + MenuButton + NSButton + + menu_ + NSMenu + + + IBProjectSource + browser/cocoa/menu_button.h + + + NSObject IBProjectSource @@ -992,19 +1072,19 @@ YES YES - backgroundButton_ closeButton_ contextMenu_ iconView_ target_ + titleView_ YES NSButton - NSButton NSMenu NSView id + NSTextField diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h index a2ae151..548210e 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.h +++ b/chrome/browser/cocoa/bookmark_bar_controller.h @@ -18,6 +18,7 @@ class BookmarkModel; class BookmarkNode; @class BookmarkBarView; class GURL; +@class MenuButton; class Profile; class PrefService; @protocol ViewResizer; @@ -68,7 +69,7 @@ class PrefService; id urlDelegate_; // weak IBOutlet NSView* buttonView_; - IBOutlet NSButton* offTheSideButton_; + IBOutlet MenuButton* offTheSideButton_; IBOutlet NSMenu* buttonContextMenu_; } @@ -98,7 +99,6 @@ class PrefService; // From a button, ... - (IBAction)openBookmark:(id)sender; - (IBAction)openFolderMenuFromButton:(id)sender; -- (IBAction)openOffTheSideMenuFromButton:(id)sender; // From a context menu over the button, ... - (IBAction)openBookmarkInNewForegroundTab:(id)sender; - (IBAction)openBookmarkInNewWindow:(id)sender; @@ -148,6 +148,8 @@ class PrefService; - (NSMenu *)menuForFolderNode:(const BookmarkNode*)node; - (int64)nodeIdFromMenuTag:(int32)tag; - (int32)menuTagFromNodeId:(int64)menuid; +- (void)buildOffTheSideMenu; +- (NSMenu*)offTheSideMenu; @end #endif // CHROME_BROWSER_COCOA_BOOKMARK_BAR_CONTROLLER_H_ diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm index e8e5daf..a8cac69 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller.mm @@ -15,6 +15,7 @@ #import "chrome/browser/cocoa/bookmark_name_folder_controller.h" #import "chrome/browser/cocoa/bookmark_menu_cocoa_controller.h" #import "chrome/browser/cocoa/event_utils.h" +#import "chrome/browser/cocoa/menu_button.h" #import "chrome/browser/cocoa/view_resizer.h" #include "chrome/browser/cocoa/nsimage_cache.h" #include "chrome/browser/profile.h" @@ -101,6 +102,8 @@ const CGFloat kBookmarkHorizontalPadding = 1.0; selector:@selector(frameDidChange) name:NSViewFrameDidChangeNotification object:[self view]]; + + DCHECK([offTheSideButton_ menu]); } - (void)showIfNeeded { @@ -294,28 +297,41 @@ const CGFloat kBookmarkHorizontalPadding = 1.0; } } -// TODO(jrg): cache the menu so we don't need to build it every time. +// Rebuild the off-the-side menu, taking into account which buttons are +// displayed. +// TODO(jrg,viettrungluu): only (re)build the menu when necessary. // TODO(jrg): if we get smarter such that we don't even bother // creating buttons which aren't visible, we'll need to be smarter // here. -- (IBAction)openOffTheSideMenuFromButton:(id)sender { - scoped_nsobject menu([[NSMenu alloc] initWithTitle:@""]); +- (void)buildOffTheSideMenu { + NSMenu* menu = [self offTheSideMenu]; + DCHECK(menu); + + // Remove old menu items (backwards order is as good as any); leave the + // blank one at position 0 (see menu_button.h). + for (NSInteger i = [menu numberOfItems] - 1; i >= 1 ; i--) + [menu removeItemAtIndex:i]; + + // Add items corresponding to buttons which aren't displayed or are only + // partly displayed. for (NSButton* each_button in buttons_.get()) { if (NSMaxX([each_button frame]) > NSMaxX([[each_button superview] frame])) { - [self addNode:[self nodeFromButton:each_button] toMenu:menu.get()]; + [self addNode:[self nodeFromButton:each_button] toMenu:menu]; } } +} - // TODO(jrg): once we disable the button when the menu should be - // empty, remove this 'helper'. - if (![menu numberOfItems]) { - [self tagEmptyMenu:menu]; - } +// Get the off-the-side menu. +- (NSMenu*)offTheSideMenu { + return [offTheSideButton_ menu]; +} - [NSMenu popUpContextMenu:menu - withEvent:[NSApp currentEvent] - forView:sender]; +// Called by any menus which have set us as their delegate (right now just the +// off-the-side menu?). +- (void)menuNeedsUpdate:(NSMenu*)menu { + if (menu == [self offTheSideMenu]) + [self buildOffTheSideMenu]; } // As a convention we set the menu's delegate to be the button's cell diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm index 55c4c18..7799e5c 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm @@ -524,6 +524,49 @@ TEST_F(BookmarkBarControllerTest, MiddleClick) { [bar_ setUrlDelegate:nil]; } +TEST_F(BookmarkBarControllerTest, TestBuildOffTheSideMenu) { + BookmarkModel* model = helper_.profile()->GetBookmarkModel(); + NSMenu* menu = [bar_ offTheSideMenu]; + ASSERT_TRUE(menu); + + // The bookmark bar should start out with nothing. + EXPECT_EQ(0U, [[bar_ buttons] count]); + + // Make sure things work when there's nothing. Note that there should always + // be a blank first menu item. + [bar_ buildOffTheSideMenu]; + EXPECT_EQ(1, [menu numberOfItems]); + + // We add lots of bookmarks. At first, we expect nothing to be added to the + // off-the-side menu. But once they start getting added, we expect the + // remaining ones to be added too. We expect a reasonably substantial number + // of items to be added by the end. + int num_off_the_side = 0; + for (int i = 0; i < 50; i++) { + const BookmarkNode* parent = model->GetBookmarkBarNode(); + model->AddURL(parent, parent->GetChildCount(), + L"very wide title", + GURL("http://www.foobar.com/")); + [bar_ buildOffTheSideMenu]; + + if (num_off_the_side) { + num_off_the_side++; + EXPECT_EQ(1 + num_off_the_side, [menu numberOfItems]); + } else { + EXPECT_TRUE([menu numberOfItems] == 1 || [menu numberOfItems] == 2); + if ([menu numberOfItems] == 2) + num_off_the_side++; + } + } + EXPECT_GE(num_off_the_side, 20); + + // Reset, and check that the built menu is "empty" again. + [bar_ clearBookmarkBar]; + EXPECT_EQ(0U, [[bar_ buttons] count]); + [bar_ buildOffTheSideMenu]; + EXPECT_EQ(1, [menu numberOfItems]); +} + // Cannot test these methods since they simply call a single static // method, BookmarkEditor::Show(), which is impossible to mock. // editBookmark:, addPage: diff --git a/chrome/browser/cocoa/menu_button.h b/chrome/browser/cocoa/menu_button.h index 92b0569..1fe6853 100644 --- a/chrome/browser/cocoa/menu_button.h +++ b/chrome/browser/cocoa/menu_button.h @@ -13,10 +13,14 @@ @interface MenuButton : NSButton { @private IBOutlet NSMenu* menu_; - BOOL openAtRight_; } -// The menu to display. +// The menu to display. Note that it should have no (i.e., a blank) title and +// that the 0-th entry should be blank (and won't be displayed). (This is +// because we use a pulldown list, for which Cocoa uses the 0-th item as "title" +// in the button. This might change if we ever switch to a pop-up. Our direct +// use of the given NSMenu object means that the one can set and use NSMenu's +// delegate as usual.) @property(assign, nonatomic) NSMenu* menu; @end // @interface MenuButton diff --git a/chrome/browser/cocoa/menu_button.mm b/chrome/browser/cocoa/menu_button.mm index aed8034..23d5d0e 100644 --- a/chrome/browser/cocoa/menu_button.mm +++ b/chrome/browser/cocoa/menu_button.mm @@ -54,7 +54,7 @@ // Reset various settings of the button and its associated |ClickHoldButtonCell| // to the standard state which provides reasonable defaults. - (void)resetToDefaults { - id cell = [self cell]; + ClickHoldButtonCell* cell = [self cell]; DCHECK([cell isKindOfClass:[ClickHoldButtonCell class]]); [cell setEnableClickHold:YES]; [cell setClickHoldTimeout:0.0]; // Make menu trigger immediately. -- cgit v1.1