Adding (empty) placeholder button to 'Other bookmarks' folder is unnecessary.

When user removes the last item (folder/page) from 'Other bookmarks' folder,
the folder itself is hidden; hence adding (empty) button is unnecessary. So
just close the folder before hiding it.

TEST=
1) Launch chrome and bookmark a page or folder into 'Other Bookmarks'.
2) Right-click on the bookmarked page/folder in the 'Other Bookmarks' from Bookmark bar.
3) Choose to 'Cut'/'Delete' and observe.
4) (empty) placeholder button should not remain after 'Other bookmarks' folder is hidden.

BUG=257538
R=asvitkine@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#291160}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291160 0039d316-1c4b-4281-b951-d872f2087c98
parent 2fedea44
...@@ -762,7 +762,7 @@ NSRect GetFirstButtonFrameForHeight(CGFloat height) { ...@@ -762,7 +762,7 @@ NSRect GetFirstButtonFrameForHeight(CGFloat height) {
- (void)adjustWindowForButtonCount:(NSUInteger)buttonCount { - (void)adjustWindowForButtonCount:(NSUInteger)buttonCount {
NSRect folderFrame = [folderView_ frame]; NSRect folderFrame = [folderView_ frame];
CGFloat newMenuHeight = CGFloat newMenuHeight =
(CGFloat)[self menuHeightForButtonCount:[buttons_ count]]; (CGFloat)[self menuHeightForButtonCount:buttonCount];
CGFloat deltaMenuHeight = newMenuHeight - NSHeight(folderFrame); CGFloat deltaMenuHeight = newMenuHeight - NSHeight(folderFrame);
// If the height has changed then also change the origin, and adjust the // If the height has changed then also change the origin, and adjust the
// scroll (if scrolling). // scroll (if scrolling).
...@@ -1936,9 +1936,10 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { ...@@ -1936,9 +1936,10 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
[folderController_ [folderController_
offsetFolderMenuWindow:NSMakeSize(0.0, chrome::kBookmarkBarHeight)]; offsetFolderMenuWindow:NSMakeSize(0.0, chrome::kBookmarkBarHeight)];
} }
} else { } else if (parentButton_ != [barController_ otherBookmarksButton]) {
// If all nodes have been removed from this folder then add in the // If all nodes have been removed from this folder then add in the
// 'empty' placeholder button. // 'empty' placeholder button except for "Other bookmarks" folder
// as we are going to hide it.
NSRect buttonFrame = NSRect buttonFrame =
GetFirstButtonFrameForHeight([self menuHeightForButtonCount:1]); GetFirstButtonFrameForHeight([self menuHeightForButtonCount:1]);
BookmarkButton* button = [self makeButtonForNode:nil BookmarkButton* button = [self makeButtonForNode:nil
...@@ -1948,7 +1949,12 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { ...@@ -1948,7 +1949,12 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
buttonCount = 1; buttonCount = 1;
} }
[self adjustWindowForButtonCount:buttonCount]; // buttonCount will be 0 if "Other bookmarks" folder is empty, so close
// the folder before hiding it.
if (buttonCount == 0)
[barController_ closeBookmarkFolder:nil];
else if (buttonCount > 0)
[self adjustWindowForButtonCount:buttonCount];
if (animate && !ignoreAnimations_) if (animate && !ignoreAnimations_)
NSShowAnimationEffect(NSAnimationEffectDisappearingItemDefault, poofPoint, NSShowAnimationEffect(NSAnimationEffectDisappearingItemDefault, poofPoint,
......
...@@ -1166,6 +1166,43 @@ TEST_F(BookmarkBarFolderControllerMenuTest, MoveRemoveAddButtons) { ...@@ -1166,6 +1166,43 @@ TEST_F(BookmarkBarFolderControllerMenuTest, MoveRemoveAddButtons) {
[folder validateMenuSpacing]; [folder validateMenuSpacing];
} }
TEST_F(BookmarkBarFolderControllerMenuTest, RemoveLastButtonOtherBookmarks) {
BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile());
const BookmarkNode* otherBookmarks = model->other_node();
BookmarkButton* otherButton = [bar_ otherBookmarksButton];
ASSERT_TRUE(otherButton);
// Open the folder to get the folderController_.
[[otherButton target] openBookmarkFolderFromButton:otherButton];
BookmarkBarFolderController* folder = [bar_ folderController];
EXPECT_TRUE(folder);
// Initially there is only (empty) placeholder button, hence buttonCount
// should be 1.
NSArray* buttons = [folder buttons];
EXPECT_TRUE(buttons);
EXPECT_EQ(1U, [buttons count]);
// Add a new bookmark into 'Other bookmarks' folder.
model->AddURL(otherBookmarks, otherBookmarks->child_count(),
ASCIIToUTF16("TheOther"),
GURL("http://www.other.com"));
// buttonCount still should be 1, as we remove the (empty) placeholder button
// when adding a new button to an empty folder.
EXPECT_EQ(1U, [buttons count]);
// Now we have only 1 button; remove it so that 'Other bookmarks' folder
// is hidden.
[folder removeButton:0 animate:NO];
EXPECT_EQ(0U, [buttons count]);
// 'Other bookmarks' folder gets closed once we remove the last button. Hence
// folderController_ should be NULL.
EXPECT_FALSE([bar_ folderController]);
}
TEST_F(BookmarkBarFolderControllerMenuTest, ControllerForNode) { TEST_F(BookmarkBarFolderControllerMenuTest, ControllerForNode) {
BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile()); BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile());
const BookmarkNode* root = model->bookmark_bar_node(); const BookmarkNode* root = model->bookmark_bar_node();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment