Commit 1d730c3e authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Change ButtonPressed overrides to callbacks: c/b/ui/views/extensions/

Bug: 772945
Change-Id: I392d3bde5479806c1f9e03151dd25a8f394c7dcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441129
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarFinnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812670}
parent ff2a760b
...@@ -25,14 +25,18 @@ ExtensionsMenuButton::ExtensionsMenuButton( ...@@ -25,14 +25,18 @@ ExtensionsMenuButton::ExtensionsMenuButton(
ExtensionsMenuItemView* parent, ExtensionsMenuItemView* parent,
ToolbarActionViewController* controller, ToolbarActionViewController* controller,
bool allow_pinning) bool allow_pinning)
: views::LabelButton(this), : views::LabelButton(
base::BindRepeating(&ExtensionsMenuButton::ButtonPressed,
base::Unretained(this))),
browser_(browser), browser_(browser),
parent_(parent), parent_(parent),
controller_(controller), controller_(controller),
allow_pinning_(allow_pinning) { allow_pinning_(allow_pinning) {
ConfigureBubbleMenuItem(this, 0); ConfigureBubbleMenuItem(this, 0);
SetButtonController(std::make_unique<HoverButtonController>( SetButtonController(std::make_unique<HoverButtonController>(
this, PressedCallback(this, this), this,
base::BindRepeating(&ExtensionsMenuButton::ButtonPressed,
base::Unretained(this)),
std::make_unique<views::Button::DefaultButtonControllerDelegate>(this))); std::make_unique<views::Button::DefaultButtonControllerDelegate>(this)));
controller_->SetDelegate(this); controller_->SetDelegate(this);
UpdateState(); UpdateState();
...@@ -52,14 +56,6 @@ bool ExtensionsMenuButton::CanShowIconInToolbar() const { ...@@ -52,14 +56,6 @@ bool ExtensionsMenuButton::CanShowIconInToolbar() const {
return allow_pinning_; return allow_pinning_;
} }
void ExtensionsMenuButton::ButtonPressed(Button* sender,
const ui::Event& event) {
base::RecordAction(
base::UserMetricsAction("Extensions.Toolbar.ExtensionActivatedFromMenu"));
controller_->ExecuteAction(
true, ToolbarActionViewController::InvocationSource::kMenuEntry);
}
// ToolbarActionViewDelegateViews: // ToolbarActionViewDelegateViews:
views::View* ExtensionsMenuButton::GetAsView() { views::View* ExtensionsMenuButton::GetAsView() {
return this; return this;
...@@ -102,3 +98,10 @@ void ExtensionsMenuButton::UpdateState() { ...@@ -102,3 +98,10 @@ void ExtensionsMenuButton::UpdateState() {
bool ExtensionsMenuButton::IsMenuRunning() const { bool ExtensionsMenuButton::IsMenuRunning() const {
return parent_->IsContextMenuRunning(); return parent_->IsContextMenuRunning();
} }
void ExtensionsMenuButton::ButtonPressed() {
base::RecordAction(
base::UserMetricsAction("Extensions.Toolbar.ExtensionActivatedFromMenu"));
controller_->ExecuteAction(
true, ToolbarActionViewController::InvocationSource::kMenuEntry);
}
...@@ -24,7 +24,6 @@ class Button; ...@@ -24,7 +24,6 @@ class Button;
// the extensions menu. This includes the extension icon and name and triggers // the extensions menu. This includes the extension icon and name and triggers
// the extension action. // the extension action.
class ExtensionsMenuButton : public views::LabelButton, class ExtensionsMenuButton : public views::LabelButton,
public views::ButtonListener,
public ToolbarActionViewDelegateViews { public ToolbarActionViewDelegateViews {
public: public:
ExtensionsMenuButton(Browser* browser, ExtensionsMenuButton(Browser* browser,
...@@ -47,7 +46,6 @@ class ExtensionsMenuButton : public views::LabelButton, ...@@ -47,7 +46,6 @@ class ExtensionsMenuButton : public views::LabelButton,
private: private:
// views::ButtonListener: // views::ButtonListener:
const char* GetClassName() const override; const char* GetClassName() const override;
void ButtonPressed(Button* sender, const ui::Event& event) override;
// ToolbarActionViewDelegateViews: // ToolbarActionViewDelegateViews:
views::View* GetAsView() override; views::View* GetAsView() override;
...@@ -57,6 +55,8 @@ class ExtensionsMenuButton : public views::LabelButton, ...@@ -57,6 +55,8 @@ class ExtensionsMenuButton : public views::LabelButton,
void UpdateState() override; void UpdateState() override;
bool IsMenuRunning() const override; bool IsMenuRunning() const override;
void ButtonPressed();
Browser* const browser_; Browser* const browser_;
// The container containing this view. // The container containing this view.
......
...@@ -71,7 +71,10 @@ ExtensionsMenuItemView::ExtensionsMenuItemView( ...@@ -71,7 +71,10 @@ ExtensionsMenuItemView::ExtensionsMenuItemView(
views::MaximumFlexSizeRule::kUnbounded)); views::MaximumFlexSizeRule::kUnbounded));
if (primary_action_button_->CanShowIconInToolbar()) { if (primary_action_button_->CanShowIconInToolbar()) {
auto pin_button = CreateBubbleMenuItem(EXTENSION_PINNING, this); auto pin_button = CreateBubbleMenuItem(
EXTENSION_PINNING,
base::BindRepeating(&ExtensionsMenuItemView::PinButtonPressed,
base::Unretained(this)));
pin_button->SetBorder(views::CreateEmptyBorder(kSecondaryButtonInsets)); pin_button->SetBorder(views::CreateEmptyBorder(kSecondaryButtonInsets));
// Extension pinning is not available in Incognito as it leaves a trace of // Extension pinning is not available in Incognito as it leaves a trace of
// user activity. // user activity.
...@@ -89,35 +92,16 @@ ExtensionsMenuItemView::ExtensionsMenuItemView( ...@@ -89,35 +92,16 @@ ExtensionsMenuItemView::ExtensionsMenuItemView(
l10n_util::GetStringUTF16(IDS_EXTENSIONS_MENU_CONTEXT_MENU_TOOLTIP)); l10n_util::GetStringUTF16(IDS_EXTENSIONS_MENU_CONTEXT_MENU_TOOLTIP));
context_menu_button->SetButtonController( context_menu_button->SetButtonController(
std::make_unique<views::MenuButtonController>( std::make_unique<views::MenuButtonController>(
context_menu_button.get(), this, context_menu_button.get(),
base::BindRepeating(&ExtensionsMenuItemView::ContextMenuPressed,
base::Unretained(this)),
std::make_unique<views::Button::DefaultButtonControllerDelegate>( std::make_unique<views::Button::DefaultButtonControllerDelegate>(
context_menu_button.get()))); context_menu_button.get())));
context_menu_button_ = AddChildView(std::move(context_menu_button));
context_menu_button_ = context_menu_button.get();
AddChildView(std::move(context_menu_button));
} }
ExtensionsMenuItemView::~ExtensionsMenuItemView() = default; ExtensionsMenuItemView::~ExtensionsMenuItemView() = default;
void ExtensionsMenuItemView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender->GetID() == EXTENSION_PINNING) {
base::RecordAction(
base::UserMetricsAction("Extensions.Toolbar.PinButtonPressed"));
model_->SetActionVisibility(controller_->GetId(), !IsPinned());
return;
} else if (sender->GetID() == EXTENSION_CONTEXT_MENU) {
base::RecordAction(base::UserMetricsAction(
"Extensions.Toolbar.MoreActionsButtonPressedFromMenu"));
// TODO(crbug.com/998298): Cleanup the menu source type.
context_menu_controller_->ShowContextMenuForViewImpl(
sender, sender->GetMenuPosition(),
ui::MenuSourceType::MENU_SOURCE_MOUSE);
return;
}
NOTREACHED();
}
const char* ExtensionsMenuItemView::GetClassName() const { const char* ExtensionsMenuItemView::GetClassName() const {
return kClassName; return kClassName;
} }
...@@ -160,9 +144,22 @@ bool ExtensionsMenuItemView::IsContextMenuRunning() const { ...@@ -160,9 +144,22 @@ bool ExtensionsMenuItemView::IsContextMenuRunning() const {
bool ExtensionsMenuItemView::IsPinned() const { bool ExtensionsMenuItemView::IsPinned() const {
// |model_| can be null in unit tests. // |model_| can be null in unit tests.
if (!model_) return model_ && model_->IsActionPinned(controller_->GetId());
return false; }
return model_->IsActionPinned(controller_->GetId());
void ExtensionsMenuItemView::ContextMenuPressed() {
base::RecordAction(base::UserMetricsAction(
"Extensions.Toolbar.MoreActionsButtonPressedFromMenu"));
// TODO(crbug.com/998298): Cleanup the menu source type.
context_menu_controller_->ShowContextMenuForViewImpl(
context_menu_button_, context_menu_button_->GetMenuPosition(),
ui::MenuSourceType::MENU_SOURCE_MOUSE);
}
void ExtensionsMenuItemView::PinButtonPressed() {
base::RecordAction(
base::UserMetricsAction("Extensions.Toolbar.PinButtonPressed"));
model_->SetActionVisibility(controller_->GetId(), !IsPinned());
} }
ExtensionsMenuButton* ExtensionsMenuButton*
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <memory> #include <memory>
#include "ui/views/controls/button/button.h"
#include "ui/views/view.h" #include "ui/views/view.h"
class Browser; class Browser;
...@@ -24,8 +23,7 @@ class ImageButton; ...@@ -24,8 +23,7 @@ class ImageButton;
// particular extension. Includes information about the extension in addition to // particular extension. Includes information about the extension in addition to
// a button to pin the extension to the toolbar and a button for accessing the // a button to pin the extension to the toolbar and a button for accessing the
// associated context menu. // associated context menu.
class ExtensionsMenuItemView : public views::View, class ExtensionsMenuItemView : public views::View {
public views::ButtonListener {
public: public:
static constexpr int kMenuItemHeightDp = 40; static constexpr int kMenuItemHeightDp = 40;
static constexpr gfx::Size kIconSize{28, 28}; static constexpr gfx::Size kIconSize{28, 28};
...@@ -39,9 +37,6 @@ class ExtensionsMenuItemView : public views::View, ...@@ -39,9 +37,6 @@ class ExtensionsMenuItemView : public views::View,
ExtensionsMenuItemView& operator=(const ExtensionsMenuItemView&) = delete; ExtensionsMenuItemView& operator=(const ExtensionsMenuItemView&) = delete;
~ExtensionsMenuItemView() override; ~ExtensionsMenuItemView() override;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
// views::View: // views::View:
const char* GetClassName() const override; const char* GetClassName() const override;
void OnThemeChanged() override; void OnThemeChanged() override;
...@@ -49,9 +44,11 @@ class ExtensionsMenuItemView : public views::View, ...@@ -49,9 +44,11 @@ class ExtensionsMenuItemView : public views::View,
void UpdatePinButton(); void UpdatePinButton();
bool IsContextMenuRunning() const; bool IsContextMenuRunning() const;
bool IsPinned() const; bool IsPinned() const;
void ContextMenuPressed();
void PinButtonPressed();
ToolbarActionViewController* view_controller() { return controller_.get(); } ToolbarActionViewController* view_controller() { return controller_.get(); }
const ToolbarActionViewController* view_controller() const { const ToolbarActionViewController* view_controller() const {
return controller_.get(); return controller_.get();
......
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h" #include "ui/gfx/image/image.h"
#include "ui/views/controls/button/label_button.h" #include "ui/views/controls/button/label_button.h"
#include "ui/views/test/button_test_api.h"
#include "ui/views/view.h" #include "ui/views/view.h"
// A view wrapper class that owns the ExtensionsToolbarContainer. // A view wrapper class that owns the ExtensionsToolbarContainer.
...@@ -111,10 +112,7 @@ void ExtensionsMenuTestUtil::Press(int index) { ...@@ -111,10 +112,7 @@ void ExtensionsMenuTestUtil::Press(int index) {
ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), 0, 0); ui::EventTimeForNow(), 0, 0);
// ExtensionsMenuButton::ButtonPressed() is private; workaround by casting to views::test::ButtonTestApi(primary_button).NotifyClick(event);
// to a ButtonListener.
static_cast<views::ButtonListener*>(primary_button)
->ButtonPressed(primary_button, event);
} }
std::string ExtensionsMenuTestUtil::GetExtensionId(int index) { std::string ExtensionsMenuTestUtil::GetExtensionId(int index) {
......
...@@ -55,15 +55,6 @@ ExtensionsMenuItemView* GetAsMenuItemView(views::View* view) { ...@@ -55,15 +55,6 @@ ExtensionsMenuItemView* GetAsMenuItemView(views::View* view) {
} // namespace } // namespace
ExtensionsMenuView::ButtonListener::ButtonListener(Browser* browser)
: browser_(browser) {}
void ExtensionsMenuView::ButtonListener::ButtonPressed(views::Button* sender,
const ui::Event& event) {
DCHECK_EQ(sender->GetID(), EXTENSIONS_SETTINGS_ID);
chrome::ShowExtensions(browser_, std::string());
}
ExtensionsMenuView::ExtensionsMenuView( ExtensionsMenuView::ExtensionsMenuView(
views::View* anchor_view, views::View* anchor_view,
Browser* browser, Browser* browser,
...@@ -76,7 +67,6 @@ ExtensionsMenuView::ExtensionsMenuView( ...@@ -76,7 +67,6 @@ ExtensionsMenuView::ExtensionsMenuView(
allow_pinning_(allow_pinning), allow_pinning_(allow_pinning),
toolbar_model_(ToolbarActionsModel::Get(browser_->profile())), toolbar_model_(ToolbarActionsModel::Get(browser_->profile())),
toolbar_model_observer_(this), toolbar_model_observer_(this),
button_listener_(browser_),
cant_access_{nullptr, nullptr, cant_access_{nullptr, nullptr,
IDS_EXTENSIONS_MENU_CANT_ACCESS_SITE_DATA_SHORT, IDS_EXTENSIONS_MENU_CANT_ACCESS_SITE_DATA_SHORT,
IDS_EXTENSIONS_MENU_CANT_ACCESS_SITE_DATA, IDS_EXTENSIONS_MENU_CANT_ACCESS_SITE_DATA,
...@@ -154,7 +144,7 @@ void ExtensionsMenuView::Populate() { ...@@ -154,7 +144,7 @@ void ExtensionsMenuView::Populate() {
constexpr int kSettingsIconSize = 16; constexpr int kSettingsIconSize = 16;
auto footer = CreateBubbleMenuItem( auto footer = CreateBubbleMenuItem(
EXTENSIONS_SETTINGS_ID, l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION), EXTENSIONS_SETTINGS_ID, l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION),
&button_listener_); base::BindRepeating(&chrome::ShowExtensions, browser_, std::string()));
footer->SetImage( footer->SetImage(
views::Button::STATE_NORMAL, views::Button::STATE_NORMAL,
gfx::CreateVectorIcon(vector_icons::kSettingsIcon, kSettingsIconSize, gfx::CreateVectorIcon(vector_icons::kSettingsIcon, kSettingsIconSize,
......
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "chrome/browser/ui/toolbar/toolbar_actions_model.h" #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h" #include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/controls/button/button.h"
namespace views { namespace views {
class Button; class Button;
...@@ -104,18 +103,6 @@ class ExtensionsMenuView : public views::BubbleDialogDelegateView, ...@@ -104,18 +103,6 @@ class ExtensionsMenuView : public views::BubbleDialogDelegateView,
// the view directly is more friendly to unit test setups. // the view directly is more friendly to unit test setups.
static base::AutoReset<bool> AllowInstancesForTesting(); static base::AutoReset<bool> AllowInstancesForTesting();
private:
class ButtonListener : public views::ButtonListener {
public:
explicit ButtonListener(Browser* browser);
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
private:
Browser* const browser_;
};
// A "section" within the menu, based on the extension's current access to // A "section" within the menu, based on the extension's current access to
// the page. // the page.
struct Section { struct Section {
...@@ -178,7 +165,6 @@ class ExtensionsMenuView : public views::BubbleDialogDelegateView, ...@@ -178,7 +165,6 @@ class ExtensionsMenuView : public views::BubbleDialogDelegateView,
ToolbarActionsModel* const toolbar_model_; ToolbarActionsModel* const toolbar_model_;
ScopedObserver<ToolbarActionsModel, ToolbarActionsModel::Observer> ScopedObserver<ToolbarActionsModel, ToolbarActionsModel::Observer>
toolbar_model_observer_; toolbar_model_observer_;
ButtonListener button_listener_;
std::vector<ExtensionsMenuItemView*> extensions_menu_items_; std::vector<ExtensionsMenuItemView*> extensions_menu_items_;
views::Button* manage_extensions_button_for_testing_ = nullptr; views::Button* manage_extensions_button_for_testing_ = nullptr;
......
...@@ -23,12 +23,14 @@ ...@@ -23,12 +23,14 @@
ExtensionsToolbarButton::ExtensionsToolbarButton( ExtensionsToolbarButton::ExtensionsToolbarButton(
Browser* browser, Browser* browser,
ExtensionsToolbarContainer* extensions_container) ExtensionsToolbarContainer* extensions_container)
: ToolbarButton(this), : ToolbarButton(nullptr),
browser_(browser), browser_(browser),
extensions_container_(extensions_container) { extensions_container_(extensions_container) {
std::unique_ptr<views::MenuButtonController> menu_button_controller = std::unique_ptr<views::MenuButtonController> menu_button_controller =
std::make_unique<views::MenuButtonController>( std::make_unique<views::MenuButtonController>(
this, this, this,
base::BindRepeating(&ExtensionsToolbarButton::ButtonPressed,
base::Unretained(this)),
std::make_unique<views::Button::DefaultButtonControllerDelegate>( std::make_unique<views::Button::DefaultButtonControllerDelegate>(
this)); this));
menu_button_controller_ = menu_button_controller.get(); menu_button_controller_ = menu_button_controller.get();
...@@ -90,19 +92,6 @@ void ExtensionsToolbarButton::UpdateIcon() { ...@@ -90,19 +92,6 @@ void ExtensionsToolbarButton::UpdateIcon() {
extensions_container_->GetIconColor(), GetIconSize())); extensions_container_->GetIconColor(), GetIconSize()));
} }
void ExtensionsToolbarButton::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (ExtensionsMenuView::IsShowing()) {
ExtensionsMenuView::Hide();
return;
}
pressed_lock_ = menu_button_controller_->TakeLock();
base::RecordAction(base::UserMetricsAction("Extensions.Toolbar.MenuOpened"));
ExtensionsMenuView::ShowBubble(this, browser_, extensions_container_,
extensions_container_->CanShowIconInToolbar())
->AddObserver(this);
}
void ExtensionsToolbarButton::OnWidgetDestroying(views::Widget* widget) { void ExtensionsToolbarButton::OnWidgetDestroying(views::Widget* widget) {
widget->RemoveObserver(this); widget->RemoveObserver(this);
pressed_lock_.reset(); pressed_lock_.reset();
...@@ -113,3 +102,15 @@ int ExtensionsToolbarButton::GetIconSize() const { ...@@ -113,3 +102,15 @@ int ExtensionsToolbarButton::GetIconSize() const {
return (touch_ui && !browser_->app_controller()) ? kDefaultTouchableIconSize return (touch_ui && !browser_->app_controller()) ? kDefaultTouchableIconSize
: kDefaultIconSize; : kDefaultIconSize;
} }
void ExtensionsToolbarButton::ButtonPressed() {
if (ExtensionsMenuView::IsShowing()) {
ExtensionsMenuView::Hide();
return;
}
pressed_lock_ = menu_button_controller_->TakeLock();
base::RecordAction(base::UserMetricsAction("Extensions.Toolbar.MenuOpened"));
ExtensionsMenuView::ShowBubble(this, browser_, extensions_container_,
extensions_container_->CanShowIconInToolbar())
->AddObserver(this);
}
...@@ -18,7 +18,6 @@ class ExtensionsToolbarContainer; ...@@ -18,7 +18,6 @@ class ExtensionsToolbarContainer;
// Button in the toolbar that provides access to the corresponding extensions // Button in the toolbar that provides access to the corresponding extensions
// menu. // menu.
class ExtensionsToolbarButton : public ToolbarButton, class ExtensionsToolbarButton : public ToolbarButton,
public views::ButtonListener,
public views::WidgetObserver { public views::WidgetObserver {
public: public:
ExtensionsToolbarButton(Browser* browser, ExtensionsToolbarButton(Browser* browser,
...@@ -34,15 +33,14 @@ class ExtensionsToolbarButton : public ToolbarButton, ...@@ -34,15 +33,14 @@ class ExtensionsToolbarButton : public ToolbarButton,
const char* GetClassName() const override; const char* GetClassName() const override;
void UpdateIcon() override; void UpdateIcon() override;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
// views::WidgetObserver: // views::WidgetObserver:
void OnWidgetDestroying(views::Widget* widget) override; void OnWidgetDestroying(views::Widget* widget) override;
private: private:
int GetIconSize() const; int GetIconSize() const;
void ButtonPressed();
// A lock to keep the button pressed when a popup is visible. // A lock to keep the button pressed when a popup is visible.
std::unique_ptr<views::MenuButtonController::PressedLock> pressed_lock_; std::unique_ptr<views::MenuButtonController::PressedLock> pressed_lock_;
......
...@@ -62,13 +62,6 @@ void ScrollableView::Layout() { ...@@ -62,13 +62,6 @@ void ScrollableView::Layout() {
views::View::Layout(); views::View::Layout();
} }
std::unique_ptr<views::LabelButton> CreateAuxiliaryButton(
views::ButtonListener* listener,
const base::string16& label) {
return label.empty() ? nullptr
: std::make_unique<views::MdTextButton>(listener, label);
}
} // namespace } // namespace
MediaGalleriesDialogViews::MediaGalleriesDialogViews( MediaGalleriesDialogViews::MediaGalleriesDialogViews(
...@@ -85,8 +78,16 @@ MediaGalleriesDialogViews::MediaGalleriesDialogViews( ...@@ -85,8 +78,16 @@ MediaGalleriesDialogViews::MediaGalleriesDialogViews(
SetShowCloseButton(false); SetShowCloseButton(false);
SetTitle(controller_->GetHeader()); SetTitle(controller_->GetHeader());
auxiliary_button_ = SetExtraView( base::string16 label = controller_->GetAuxiliaryButtonText();
CreateAuxiliaryButton(this, controller_->GetAuxiliaryButtonText())); if (!label.empty()) {
auxiliary_button_ = SetExtraView(std::make_unique<views::MdTextButton>(
base::BindRepeating(
&MediaGalleriesDialogViews::ButtonPressed, base::Unretained(this),
base::BindRepeating(
&MediaGalleriesDialogController::DidClickAuxiliaryButton,
base::Unretained(controller_))),
label));
}
InitChildViews(); InitChildViews();
if (ControllerHasWebContents()) { if (ControllerHasWebContents()) {
...@@ -221,12 +222,19 @@ bool MediaGalleriesDialogViews::AddOrUpdateGallery( ...@@ -221,12 +222,19 @@ bool MediaGalleriesDialogViews::AddOrUpdateGallery(
return false; return false;
} }
MediaGalleryCheckboxView* gallery_view = new MediaGalleryCheckboxView( auto* gallery_view =
gallery.pref_info, trailing_vertical_space, this, this); container->AddChildView(std::make_unique<MediaGalleryCheckboxView>(
gallery.pref_info, trailing_vertical_space, this));
gallery_view->checkbox()->set_callback(base::BindRepeating(
&MediaGalleriesDialogViews::ButtonPressed, base::Unretained(this),
base::BindRepeating(
[](MediaGalleriesDialogController* controller,
MediaGalleryPrefId pref_id, views::Checkbox* checkbox) {
controller->DidToggleEntry(pref_id, checkbox->GetChecked());
},
controller_, gallery.pref_info.pref_id, gallery_view->checkbox())));
gallery_view->checkbox()->SetChecked(gallery.selected); gallery_view->checkbox()->SetChecked(gallery.selected);
container->AddChildView(gallery_view);
checkbox_map_[gallery.pref_info.pref_id] = gallery_view; checkbox_map_[gallery.pref_info.pref_id] = gallery_view;
return true; return true;
} }
...@@ -255,28 +263,6 @@ ui::ModalType MediaGalleriesDialogViews::GetModalType() const { ...@@ -255,28 +263,6 @@ ui::ModalType MediaGalleriesDialogViews::GetModalType() const {
return ui::MODAL_TYPE_CHILD; return ui::MODAL_TYPE_CHILD;
} }
void MediaGalleriesDialogViews::ButtonPressed(views::Button* sender,
const ui::Event& /* event */) {
confirm_available_ = true;
if (ControllerHasWebContents())
DialogModelChanged();
if (sender == auxiliary_button_) {
controller_->DidClickAuxiliaryButton();
return;
}
for (CheckboxMap::const_iterator iter = checkbox_map_.begin();
iter != checkbox_map_.end(); ++iter) {
if (sender == iter->second->checkbox()) {
controller_->DidToggleEntry(iter->first,
iter->second->checkbox()->GetChecked());
return;
}
}
}
void MediaGalleriesDialogViews::ShowContextMenuForViewImpl( void MediaGalleriesDialogViews::ShowContextMenuForViewImpl(
views::View* source, views::View* source,
const gfx::Point& point, const gfx::Point& point,
...@@ -309,6 +295,15 @@ bool MediaGalleriesDialogViews::ControllerHasWebContents() const { ...@@ -309,6 +295,15 @@ bool MediaGalleriesDialogViews::ControllerHasWebContents() const {
return controller_->WebContents() != nullptr; return controller_->WebContents() != nullptr;
} }
void MediaGalleriesDialogViews::ButtonPressed(base::RepeatingClosure closure) {
confirm_available_ = true;
if (ControllerHasWebContents())
DialogModelChanged();
closure.Run();
}
void MediaGalleriesDialogViews::OnMenuClosed() { void MediaGalleriesDialogViews::OnMenuClosed() {
context_menu_runner_.reset(); context_menu_runner_.reset();
} }
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/media_galleries/media_galleries_dialog_controller.h" #include "chrome/browser/media_galleries/media_galleries_dialog_controller.h"
#include "ui/views/context_menu_controller.h" #include "ui/views/context_menu_controller.h"
#include "ui/views/controls/button/button.h"
#include "ui/views/window/dialog_delegate.h" #include "ui/views/window/dialog_delegate.h"
namespace views { namespace views {
...@@ -27,7 +26,6 @@ class MediaGalleryCheckboxView; ...@@ -27,7 +26,6 @@ class MediaGalleryCheckboxView;
// The media galleries configuration view for Views. It will immediately show // The media galleries configuration view for Views. It will immediately show
// upon construction. // upon construction.
class MediaGalleriesDialogViews : public MediaGalleriesDialog, class MediaGalleriesDialogViews : public MediaGalleriesDialog,
public views::ButtonListener,
public views::ContextMenuController, public views::ContextMenuController,
public views::DialogDelegate { public views::DialogDelegate {
public: public:
...@@ -46,9 +44,6 @@ class MediaGalleriesDialogViews : public MediaGalleriesDialog, ...@@ -46,9 +44,6 @@ class MediaGalleriesDialogViews : public MediaGalleriesDialog,
bool IsDialogButtonEnabled(ui::DialogButton button) const override; bool IsDialogButtonEnabled(ui::DialogButton button) const override;
ui::ModalType GetModalType() const override; ui::ModalType GetModalType() const override;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
// views::ContextMenuController: // views::ContextMenuController:
void ShowContextMenuForViewImpl(views::View* source, void ShowContextMenuForViewImpl(views::View* source,
const gfx::Point& point, const gfx::Point& point,
...@@ -60,7 +55,7 @@ class MediaGalleriesDialogViews : public MediaGalleriesDialog, ...@@ -60,7 +55,7 @@ class MediaGalleriesDialogViews : public MediaGalleriesDialog,
FRIEND_TEST_ALL_PREFIXES(MediaGalleriesDialogTest, UpdateAdds); FRIEND_TEST_ALL_PREFIXES(MediaGalleriesDialogTest, UpdateAdds);
FRIEND_TEST_ALL_PREFIXES(MediaGalleriesDialogTest, ForgetDeletes); FRIEND_TEST_ALL_PREFIXES(MediaGalleriesDialogTest, ForgetDeletes);
typedef std::map<MediaGalleryPrefId, MediaGalleryCheckboxView*> CheckboxMap; using CheckboxMap = std::map<MediaGalleryPrefId, MediaGalleryCheckboxView*>;
// MediaGalleriesDialog: // MediaGalleriesDialog:
void AcceptDialogForTesting() override; void AcceptDialogForTesting() override;
...@@ -82,6 +77,10 @@ class MediaGalleriesDialogViews : public MediaGalleriesDialog, ...@@ -82,6 +77,10 @@ class MediaGalleriesDialogViews : public MediaGalleriesDialog,
// In unit tests, it may not. // In unit tests, it may not.
bool ControllerHasWebContents() const; bool ControllerHasWebContents() const;
// Called when a button is pressed; does common preamble, then runs the
// supplied closure to execute the specific details of the particular button.
void ButtonPressed(base::RepeatingClosure closure);
// Callback for MenuRunner. // Callback for MenuRunner.
void OnMenuClosed(); void OnMenuClosed();
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "components/storage_monitor/storage_info.h" #include "components/storage_monitor/storage_info.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/views/controls/button/checkbox.h" #include "ui/views/controls/button/checkbox.h"
#include "ui/views/test/button_test_api.h"
using ::testing::_; using ::testing::_;
using ::testing::AnyNumber; using ::testing::AnyNumber;
...@@ -100,14 +101,13 @@ TEST_F(MediaGalleriesDialogTest, ToggleCheckboxes) { ...@@ -100,14 +101,13 @@ TEST_F(MediaGalleriesDialogTest, ToggleCheckboxes) {
views::Checkbox* checkbox = dialog.checkbox_map_[1]->checkbox(); views::Checkbox* checkbox = dialog.checkbox_map_[1]->checkbox();
EXPECT_TRUE(checkbox->GetChecked()); EXPECT_TRUE(checkbox->GetChecked());
ui::KeyEvent dummy_event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
EXPECT_CALL(*controller(), DidToggleEntry(1, false)); EXPECT_CALL(*controller(), DidToggleEntry(1, false));
checkbox->SetChecked(false); views::test::ButtonTestApi test_api(checkbox);
dialog.ButtonPressed(checkbox, dummy_event); ui::KeyEvent dummy_event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
test_api.NotifyClick(dummy_event); // Toggles to unchecked before notifying.
EXPECT_CALL(*controller(), DidToggleEntry(1, true)); EXPECT_CALL(*controller(), DidToggleEntry(1, true));
checkbox->SetChecked(true); test_api.NotifyClick(dummy_event); // Toggles to checked before notifying.
dialog.ButtonPressed(checkbox, dummy_event);
} }
// Tests that UpdateGallery will add a new checkbox, but only if it refers to // Tests that UpdateGallery will add a new checkbox, but only if it refers to
......
...@@ -26,9 +26,7 @@ const SkColor kDeemphasizedTextColor = SkColorSetRGB(159, 159, 159); ...@@ -26,9 +26,7 @@ const SkColor kDeemphasizedTextColor = SkColorSetRGB(159, 159, 159);
MediaGalleryCheckboxView::MediaGalleryCheckboxView( MediaGalleryCheckboxView::MediaGalleryCheckboxView(
const MediaGalleryPrefInfo& pref_info, const MediaGalleryPrefInfo& pref_info,
int trailing_vertical_space, int trailing_vertical_space,
views::ButtonListener* button_listener,
views::ContextMenuController* menu_controller) { views::ContextMenuController* menu_controller) {
DCHECK(button_listener != NULL);
SetLayoutManager(std::make_unique<views::BoxLayout>( SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kHorizontal)); views::BoxLayout::Orientation::kHorizontal));
ChromeLayoutProvider* provider = ChromeLayoutProvider::Get(); ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
...@@ -39,8 +37,8 @@ MediaGalleryCheckboxView::MediaGalleryCheckboxView( ...@@ -39,8 +37,8 @@ MediaGalleryCheckboxView::MediaGalleryCheckboxView(
if (menu_controller) if (menu_controller)
set_context_menu_controller(menu_controller); set_context_menu_controller(menu_controller);
checkbox_ = checkbox_ = AddChildView(std::make_unique<views::Checkbox>(
new views::Checkbox(pref_info.GetGalleryDisplayName(), button_listener); pref_info.GetGalleryDisplayName(), views::Button::PressedCallback()));
if (menu_controller) if (menu_controller)
checkbox_->set_context_menu_controller(menu_controller); checkbox_->set_context_menu_controller(menu_controller);
checkbox_->SetElideBehavior(gfx::ELIDE_MIDDLE); checkbox_->SetElideBehavior(gfx::ELIDE_MIDDLE);
...@@ -48,7 +46,7 @@ MediaGalleryCheckboxView::MediaGalleryCheckboxView( ...@@ -48,7 +46,7 @@ MediaGalleryCheckboxView::MediaGalleryCheckboxView(
checkbox_->SetTooltipText(tooltip_text); checkbox_->SetTooltipText(tooltip_text);
base::string16 details = pref_info.GetGalleryAdditionalDetails(); base::string16 details = pref_info.GetGalleryAdditionalDetails();
secondary_text_ = new views::Label(details); secondary_text_ = AddChildView(std::make_unique<views::Label>(details));
if (menu_controller) if (menu_controller)
secondary_text_->set_context_menu_controller(menu_controller); secondary_text_->set_context_menu_controller(menu_controller);
secondary_text_->SetVisible(details.length() > 0); secondary_text_->SetVisible(details.length() > 0);
...@@ -58,12 +56,9 @@ MediaGalleryCheckboxView::MediaGalleryCheckboxView( ...@@ -58,12 +56,9 @@ MediaGalleryCheckboxView::MediaGalleryCheckboxView(
secondary_text_->SetBorder(views::CreateEmptyBorder( secondary_text_->SetBorder(views::CreateEmptyBorder(
0, provider->GetDistanceMetric(DISTANCE_RELATED_CONTROL_HORIZONTAL_SMALL), 0, provider->GetDistanceMetric(DISTANCE_RELATED_CONTROL_HORIZONTAL_SMALL),
0, 0)); 0, 0));
AddChildView(checkbox_);
AddChildView(secondary_text_);
} }
MediaGalleryCheckboxView::~MediaGalleryCheckboxView() {} MediaGalleryCheckboxView::~MediaGalleryCheckboxView() = default;
void MediaGalleryCheckboxView::Layout() { void MediaGalleryCheckboxView::Layout() {
views::View::Layout(); views::View::Layout();
......
...@@ -8,12 +8,12 @@ ...@@ -8,12 +8,12 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/views/controls/button/button.h"
#include "ui/views/view.h" #include "ui/views/view.h"
struct MediaGalleryPrefInfo; struct MediaGalleryPrefInfo;
namespace views { namespace views {
class ButtonListener;
class Checkbox; class Checkbox;
class ContextMenuController; class ContextMenuController;
class Label; class Label;
...@@ -26,16 +26,15 @@ class MediaGalleryCheckboxView : public views::View { ...@@ -26,16 +26,15 @@ class MediaGalleryCheckboxView : public views::View {
public: public:
MediaGalleryCheckboxView(const MediaGalleryPrefInfo& pref_info, MediaGalleryCheckboxView(const MediaGalleryPrefInfo& pref_info,
int trailing_vertical_space, int trailing_vertical_space,
views::ButtonListener* button_listener,
views::ContextMenuController* menu_controller); views::ContextMenuController* menu_controller);
~MediaGalleryCheckboxView() override; ~MediaGalleryCheckboxView() override;
// Overrides from views::View.
void Layout() override;
views::Checkbox* checkbox() { return checkbox_; } views::Checkbox* checkbox() { return checkbox_; }
views::Label* secondary_text() { return secondary_text_; } views::Label* secondary_text() { return secondary_text_; }
// views::View:
void Layout() override;
private: private:
// Owned by the parent class (views::View). // Owned by the parent class (views::View).
views::Checkbox* checkbox_; views::Checkbox* checkbox_;
......
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