bookmarks: Convert "Remove" link into a LabelButton.

chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h"
 
 #include "base/string16.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
@@ skipping 19 matching lines @@
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
 
 using content::UserMetricsAction;
 using views::ColumnSet;
 using views::GridLayout;
 
 namespace {
 
-// Padding between "Title:" and the actual title.
-const int kTitlePadding = 4;
+// Minimum size of the bubble. This is used to shrink the width of the bubble

[msw, 2013/06/01 18:04:38]

Actually, this minimum width is clamping the bubble to be bigger. If you remove
this minimum width (now that the kMinimumFieldSize is gone), the bubble would be
smaller than we'd want. You should just say "Minimum width of the bubble.", and
increase the value to 340 (336 is a little smaller than the spec).

[tfarina, 2013/06/01 19:43:37]

Done.

+// in order to reduce/compress the size of the textfields and thus the size
+// of the "Remove" button, otherwise they just look big and ugly.
+const int kMinBubbleWidth = 336;
 
-// Minimum width for the fields - they will push out the size of the bubble if
-// necessary. This should be big enough so that the field pushes the right side
-// of the bubble far enough so that the edit button's left edge is to the right
-// of the field's left edge.
-const int kMinimumFieldSize = 180;
+// The padding used between textfield and combobox and between combobox and the
+// button's row.
+const int kVerticalSpacing = 10;
 
 }  // namespace
 
 // Declared in browser_dialogs.h so callers don't have to depend on our header.
 
 namespace chrome {
 
 void ShowBookmarkBubbleView(views::View* anchor_view,
                             BookmarkBubbleViewObserver* observer,
                             Profile* profile,
@@ skipping 81 matching lines @@
      return true;
   } else if (accelerator.key_code() == ui::VKEY_ESCAPE) {
     remove_bookmark_ = newly_bookmarked_;
     apply_edits_ = false;
   }
 
   return BubbleDelegateView::AcceleratorPressed(accelerator);
 }
 
 void BookmarkBubbleView::Init() {
-  remove_link_ = new views::Link(l10n_util::GetStringUTF16(
+  remove_button_ = new views::LabelButton(this, l10n_util::GetStringUTF16(
       IDS_BOOKMARK_BUBBLE_REMOVE_BOOKMARK));
-  remove_link_->set_listener(this);
+  remove_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
 
   edit_button_ = new views::LabelButton(
       this, l10n_util::GetStringUTF16(IDS_BOOKMARK_BUBBLE_OPTIONS));
   edit_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
 
   close_button_ = new views::LabelButton(
       this, l10n_util::GetStringUTF16(IDS_DONE));
   close_button_->SetStyle(views::Button::STYLE_NATIVE_TEXTBUTTON);
   close_button_->SetIsDefault(true);
 
   views::Label* combobox_label = new views::Label(
       l10n_util::GetStringUTF16(IDS_BOOKMARK_BUBBLE_FOLDER_TEXT));
 
   parent_combobox_ = new views::Combobox(&parent_model_);
   parent_combobox_->set_listener(this);
   parent_combobox_->SetAccessibleName(combobox_label->text());
 
   views::Label* title_label = new views::Label(
       l10n_util::GetStringUTF16(
           newly_bookmarked_ ? IDS_BOOKMARK_BUBBLE_PAGE_BOOKMARKED :
                               IDS_BOOKMARK_BUBBLE_PAGE_BOOKMARK));
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   title_label->SetFont(rb.GetFont(ui::ResourceBundle::MediumFont));
 
   GridLayout* layout = new GridLayout(this);
   SetLayoutManager(layout);
 
-  ColumnSet* cs = layout->AddColumnSet(0);
+  const int kFirstColumnSetID = 0;
+  ColumnSet* cs = layout->AddColumnSet(kFirstColumnSetID);
 
   // Top (title) row.

[msw, 2013/06/01 18:04:38]

Still hoping you'll use the bubble's title feature with a smaller top inset.

[tfarina, 2013/06/01 19:43:37]

Done.

   cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0, GridLayout::USE_PREF,
                 0, 0);
   cs->AddPaddingColumn(1, views::kUnrelatedControlHorizontalSpacing);
   cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0, GridLayout::USE_PREF,
                 0, 0);
 
-  // Middle (input field) rows.
-  cs = layout->AddColumnSet(2);
+  // The column layout used for middle and bottom rows.
+  const int kSecondColumnSetID = 1;
+  cs = layout->AddColumnSet(kSecondColumnSetID);
   cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 0,
                 GridLayout::USE_PREF, 0, 0);
-  cs->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
-  cs->AddColumn(GridLayout::FILL, GridLayout::CENTER, 1,
-                GridLayout::USE_PREF, 0, kMinimumFieldSize);
+  cs->AddPaddingColumn(0, views::kUnrelatedControlHorizontalSpacing);
 
-  // Bottom (buttons) row.
-  cs = layout->AddColumnSet(3);
-  cs->AddPaddingColumn(1, views::kRelatedControlHorizontalSpacing);
+  cs->AddColumn(GridLayout::FILL, GridLayout::CENTER, 0,
+                GridLayout::USE_PREF, 0, 0);
+  cs->AddPaddingColumn(1, views::kUnrelatedControlLargeHorizontalSpacing);
+
   cs->AddColumn(GridLayout::LEADING, GridLayout::TRAILING, 0,
                 GridLayout::USE_PREF, 0, 0);
   // We subtract 2 to account for the natural button padding, and
   // to bring the separation visually in line with the row separation
   // height.
   cs->AddPaddingColumn(0, views::kRelatedButtonHSpacing - 2);
   cs->AddColumn(GridLayout::LEADING, GridLayout::TRAILING, 0,
                 GridLayout::USE_PREF, 0, 0);
 
-  layout->StartRow(0, 0);
+  layout->StartRow(0, kFirstColumnSetID);
   layout->AddView(title_label);
-  layout->AddView(remove_link_);
+  layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing);
 
-  layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing);
-  layout->StartRow(0, 2);
+  layout->StartRow(0, kSecondColumnSetID);
   views::Label* label = new views::Label(
       l10n_util::GetStringUTF16(IDS_BOOKMARK_BUBBLE_TITLE_TEXT));
   layout->AddView(label);
   title_tf_ = new views::Textfield();
   title_tf_->SetText(GetTitle());
-  layout->AddView(title_tf_);
+  layout->AddView(title_tf_, 5, 1);
 
-  layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing);
+  layout->AddPaddingRow(0, kVerticalSpacing);
 
-  layout->StartRow(0, 2);
+  layout->StartRow(0, kSecondColumnSetID);
   layout->AddView(combobox_label);
-  layout->AddView(parent_combobox_);
-  layout->AddPaddingRow(0, views::kRelatedControlSmallVerticalSpacing);
+  layout->AddView(parent_combobox_, 5, 1);
 
-  layout->StartRow(0, 3);
+  layout->AddPaddingRow(0, kVerticalSpacing);
+
+  layout->StartRow(0, kSecondColumnSetID);
+  layout->SkipColumns(2);
+  layout->AddView(remove_button_);
   layout->AddView(edit_button_);
   layout->AddView(close_button_);
 
   AddAccelerator(ui::Accelerator(ui::VKEY_RETURN, ui::EF_NONE));
 }
 
 BookmarkBubbleView::BookmarkBubbleView(views::View* anchor_view,
                                        BookmarkBubbleViewObserver* observer,
                                        Profile* profile,
                                        const GURL& url,
                                        bool newly_bookmarked)
     : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
       observer_(observer),
       profile_(profile),
       url_(url),
       newly_bookmarked_(newly_bookmarked),
       parent_model_(
           BookmarkModelFactory::GetForProfile(profile_),
           BookmarkModelFactory::GetForProfile(profile_)->
               GetMostRecentlyAddedNodeForURL(url)),
-      remove_link_(NULL),
+      remove_button_(NULL),
       edit_button_(NULL),
       close_button_(NULL),
       title_tf_(NULL),
       parent_combobox_(NULL),
       remove_bookmark_(false),
       apply_edits_(true) {
+  set_margins(gfx::Insets(13,

[msw, 2013/06/01 18:04:38]

reduce the top inset here (probably to 0), if you're using the bubble title.

[tfarina, 2013/06/01 19:43:37]

Done.

+                          views::kUnrelatedControlLargeHorizontalSpacing,
+                          views::kUnrelatedControlVerticalSpacing,
+                          views::kUnrelatedControlLargeHorizontalSpacing));
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(5, 0, 5, 0));
 }
 
 string16 BookmarkBubbleView::GetTitle() {
   BookmarkModel* bookmark_model =
       BookmarkModelFactory::GetForProfile(profile_);
   const BookmarkNode* node =
       bookmark_model->GetMostRecentlyAddedNodeForURL(url_);
   if (node)
     return node->GetTitle();
   else
     NOTREACHED();
   return string16();
 }
 
+gfx::Size BookmarkBubbleView::GetMinimumSize() {
+  gfx::Size size(views::BubbleDelegateView::GetPreferredSize());
+  size.SetToMax(gfx::Size(kMinBubbleWidth, 0));
+  return size;
+}
+
 void BookmarkBubbleView::ButtonPressed(views::Button* sender,
                                        const ui::Event& event) {
   HandleButtonPressed(sender);
 }
 
-void BookmarkBubbleView::LinkClicked(views::Link* source, int event_flags) {
-  DCHECK_EQ(remove_link_, source);
-  content::RecordAction(UserMetricsAction("BookmarkBubble_Unstar"));
-
-  // Set this so we remove the bookmark after the window closes.
-  remove_bookmark_ = true;
-  apply_edits_ = false;
-  StartFade(false);
-}
-
 void BookmarkBubbleView::OnSelectedIndexChanged(views::Combobox* combobox) {
   if (combobox->selected_index() + 1 == parent_model_.GetItemCount()) {
     content::RecordAction(UserMetricsAction("BookmarkBubble_EditFromCombobox"));
     ShowEditor();
   }
 }
 
 void BookmarkBubbleView::HandleButtonPressed(views::Button* sender) {
-  if (sender == edit_button_) {
+  if (sender == remove_button_) {
+    content::RecordAction(UserMetricsAction("BookmarkBubble_Unstar"));
+    // Set this so we remove the bookmark after the window closes.
+    remove_bookmark_ = true;
+    apply_edits_ = false;
+    StartFade(false);
+  } else if (sender == edit_button_) {
     content::RecordAction(UserMetricsAction("BookmarkBubble_Edit"));
     ShowEditor();
   } else {
     DCHECK_EQ(close_button_, sender);
     StartFade(false);
   }
 }
 
 void BookmarkBubbleView::ShowEditor() {
   const BookmarkNode* node = BookmarkModelFactory::GetForProfile(
@@ skipping 20 matching lines @@
   if (node) {
     const string16 new_title = title_tf_->text();
     if (new_title != node->GetTitle()) {
       model->SetTitle(node, new_title);
       content::RecordAction(
           UserMetricsAction("BookmarkBubble_ChangeTitleInBubble"));
     }
     parent_model_.MaybeChangeParent(node, parent_combobox_->selected_index());
   }
 }