Fix app menu button preferred size issue.
App menu now sets its label text context to toolbar button, like other
toolbar buttons.
Also prevent toolbar and app menu buttons from unnecessarily destroying
and recreating empty borders if the inset size has not changed.
Bug: 931575
Change-Id: Id2b7e146a97b1da56f08caa9b79b4c7072c9de04
Reviewed-on: https://chromium-review.googlesource.com/c/1474687
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632703}diff --git a/chrome/browser/ui/views/frame/app_menu_button.cc b/chrome/browser/ui/views/frame/app_menu_button.cc
index b7baf66..676dd1e 100644
--- a/chrome/browser/ui/views/frame/app_menu_button.cc
+++ b/chrome/browser/ui/views/frame/app_menu_button.cc
@@ -7,12 +7,15 @@
#include <utility>
#include "chrome/browser/ui/toolbar/app_menu_model.h"
+#include "chrome/browser/ui/views/chrome_typography.h"
#include "chrome/browser/ui/views/frame/app_menu_button_observer.h"
#include "chrome/browser/ui/views/toolbar/app_menu.h"
#include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
AppMenuButton::AppMenuButton(views::MenuButtonListener* menu_button_listener)
- : views::MenuButton(base::string16(), menu_button_listener) {}
+ : views::MenuButton(base::string16(),
+ menu_button_listener,
+ CONTEXT_TOOLBAR_BUTTON) {}
AppMenuButton::~AppMenuButton() {}
diff --git a/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc b/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc
index 4a025ea..f51f7bd 100644
--- a/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc
+++ b/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc
@@ -303,8 +303,10 @@
}
void BrowserAppMenuButton::UpdateBorder() {
- SetBorder(views::CreateEmptyBorder(GetLayoutInsets(TOOLBAR_BUTTON) +
- *GetProperty(views::kInternalPaddingKey)));
+ gfx::Insets new_insets = GetLayoutInsets(TOOLBAR_BUTTON) +
+ *GetProperty(views::kInternalPaddingKey);
+ if (!border() || border()->GetInsets() != new_insets)
+ SetBorder(views::CreateEmptyBorder(new_insets));
}
void BrowserAppMenuButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {
diff --git a/chrome/browser/ui/views/toolbar/toolbar_button.cc b/chrome/browser/ui/views/toolbar/toolbar_button.cc
index 5e7915e..6da32a3 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_button.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_button.cc
@@ -62,6 +62,8 @@
// make to the leading margin to handle Fitts' Law, it's easier to just
// allocate the property once and modify the value.
SetProperty(views::kInternalPaddingKey, new gfx::Insets());
+
+ UpdateHighlightBackgroundAndInsets();
}
ToolbarButton::~ToolbarButton() {}
@@ -101,21 +103,24 @@
SetEnabledTextColors(*highlight_color_);
}
- gfx::Insets insets = GetLayoutInsets(TOOLBAR_BUTTON) + layout_inset_delta_ +
- *GetProperty(views::kInternalPaddingKey);
+ gfx::Insets new_insets = GetLayoutInsets(TOOLBAR_BUTTON) +
+ layout_inset_delta_ +
+ *GetProperty(views::kInternalPaddingKey);
+
if (!GetText().empty()) {
const int text_side_inset = highlight_radius / 2;
// Some subclasses (AvatarToolbarButton) may be change alignment. This adds
// an inset to the text-label side.
if (horizontal_alignment() == gfx::ALIGN_RIGHT) {
- insets += gfx::Insets(0, text_side_inset, 0, 0);
+ new_insets += gfx::Insets(0, text_side_inset, 0, 0);
} else {
- insets += gfx::Insets(0, 0, 0, text_side_inset);
+ new_insets += gfx::Insets(0, 0, 0, text_side_inset);
}
}
- SetBorder(views::CreateEmptyBorder(insets));
+ if (!border() || new_insets != border()->GetInsets())
+ SetBorder(views::CreateEmptyBorder(new_insets));
}
void ToolbarButton::SetLayoutInsetDelta(const gfx::Insets& inset_delta) {
diff --git a/ui/views/controls/button/menu_button.cc b/ui/views/controls/button/menu_button.cc
index 23e5987..c58a17d 100644
--- a/ui/views/controls/button/menu_button.cc
+++ b/ui/views/controls/button/menu_button.cc
@@ -18,8 +18,9 @@
const int MenuButton::kMenuMarkerPaddingRight = -1;
MenuButton::MenuButton(const base::string16& text,
- MenuButtonListener* menu_button_listener)
- : LabelButton(nullptr, text),
+ MenuButtonListener* menu_button_listener,
+ int button_context)
+ : LabelButton(nullptr, text, button_context),
menu_button_event_handler_(this, menu_button_listener) {
SetHorizontalAlignment(gfx::ALIGN_LEFT);
}
@@ -83,4 +84,4 @@
menu_button_event_handler_.NotifyClick(event);
}
-} // namespace views
\ No newline at end of file
+} // namespace views
diff --git a/ui/views/controls/button/menu_button.h b/ui/views/controls/button/menu_button.h
index 87e8faf..3449f65 100644
--- a/ui/views/controls/button/menu_button.h
+++ b/ui/views/controls/button/menu_button.h
@@ -31,7 +31,8 @@
// Create a Button.
MenuButton(const base::string16& text,
- MenuButtonListener* menu_button_listener);
+ MenuButtonListener* menu_button_listener,
+ int button_context = style::CONTEXT_BUTTON);
~MenuButton() override;
bool Activate(const ui::Event* event);
@@ -57,7 +58,7 @@
void OnMouseMoved(const ui::MouseEvent& event) final;
// Protected methods needed for MenuButtonEventHandler.
- // TODO (cyan): Move these to a delegate interface.
+ // TODO(cyan): Move these to a delegate interface.
using InkDropHostView::GetInkDrop;
using View::InDrag;
using View::GetDragOperations;
@@ -80,7 +81,7 @@
private:
// All events get sent to this handler to be processed.
- // TODO (cyan): This will be generalized into a ButtonEventHandler and moved
+ // TODO(cyan): This will be generalized into a ButtonEventHandler and moved
// into the Button class.
MenuButtonEventHandler menu_button_event_handler_;
