Skip to content

Commit

Permalink
Add tooltips to Extensions menu
Browse files Browse the repository at this point in the history
This change adds the same tooltips that are already present in the
toolbar.

Bonus cleanups are in here to make sure that the ExtensionMenuButton
HoverButton gets initialized with empty strings and icons. This makes
sure that the ::UpdateState() method can properly populate the button.

This also makes the menu taller as every item's icon is increased to
28dp to accommodate CTS badging and shadows.

Bug: chromium:943702
Change-Id: I596eaa8305eb80bf6d6bed0da505c30c553a70a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1569520
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#651363}
  • Loading branch information
pbos authored and Commit Bot committed Apr 16, 2019
1 parent 9bf18b2 commit b30594b
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 32 deletions.
40 changes: 11 additions & 29 deletions chrome/browser/ui/views/extensions/extensions_menu_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,23 @@
#include "chrome/browser/ui/views/extensions/extensions_menu_button.h"

#include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
#include "chrome/browser/ui/views/extensions/extensions_menu_view.h"
#include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"

const char ExtensionsMenuButton::kClassName[] = "ExtensionsMenuButton";

namespace {

content::WebContents* GetActiveWebContents(Browser* browser) {
return browser->tab_strip_model()->GetActiveWebContents();
}

void SetIconViewImage(views::ImageView* image_view,
Browser* browser,
ToolbarActionViewController* controller) {
image_view->SetImage(
controller->GetIcon(GetActiveWebContents(browser), gfx::Size(16, 16))
.AsImageSkia());
}

std::unique_ptr<views::View> CreateIconView(
Browser* browser,
ToolbarActionViewController* controller) {
auto image_view = std::make_unique<views::ImageView>();
SetIconViewImage(image_view.get(), browser, controller);
return image_view;
}

} // namespace

ExtensionsMenuButton::ExtensionsMenuButton(
Browser* browser,
std::unique_ptr<ToolbarActionViewController> controller)
: HoverButton(this,
CreateIconView(browser, controller.get()),
controller->GetActionName(),
ExtensionsMenuView::CreateFixedSizeIconView(),
base::string16(),
base::string16()),
browser_(browser),
controller_(std::move(controller)) {
set_auto_compute_tooltip(false);
set_context_menu_controller(this);
controller_->SetDelegate(this);
UpdateState();
Expand Down Expand Up @@ -76,15 +54,19 @@ views::View* ExtensionsMenuButton::GetReferenceViewForPopup() {
}

content::WebContents* ExtensionsMenuButton::GetCurrentWebContents() const {
return GetActiveWebContents(browser_);
return browser_->tab_strip_model()->GetActiveWebContents();
}

void ExtensionsMenuButton::UpdateState() {
DCHECK_EQ(views::ImageView::kViewClassName, icon_view()->GetClassName());
SetIconViewImage(static_cast<views::ImageView*>(icon_view()), browser_,
controller_.get());
static_cast<views::ImageView*>(icon_view())
->SetImage(controller_
->GetIcon(GetCurrentWebContents(),
icon_view()->GetPreferredSize())
.AsImageSkia());
SetTitleTextWithHintRange(controller_->GetActionName(),
gfx::Range::InvalidRange());
SetTooltipText(controller_->GetTooltip(GetCurrentWebContents()));
}

bool ExtensionsMenuButton::IsMenuRunning() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
protected:
ExtensionsMenuButtonTest()
: initial_extension_name_(base::ASCIIToUTF16("Initial Extension Name")) {}
: initial_extension_name_(base::ASCIIToUTF16("Initial Extension Name")),
initial_tooltip_(base::ASCIIToUTF16("Initial tooltip")) {}
void SetUp() override {
BrowserWithTestWindowTest::SetUp();

Expand All @@ -41,6 +42,7 @@ class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
std::make_unique<TestToolbarActionViewController>("hello");
controller_ = controller.get();
controller_->SetActionName(initial_extension_name_);
controller_->SetTooltip(initial_tooltip_);
button_ = std::make_unique<ExtensionsMenuButton>(browser(),
std::move(controller));
button_->set_owned_by_client();
Expand All @@ -63,6 +65,7 @@ class ExtensionsMenuButtonTest : public BrowserWithTestWindowTest {
}

const base::string16 initial_extension_name_;
const base::string16 initial_tooltip_;
std::unique_ptr<views::Widget> widget_;
std::unique_ptr<ExtensionsMenuButton> button_;
TestToolbarActionViewController* controller_ = nullptr;
Expand All @@ -82,3 +85,12 @@ TEST_F(ExtensionsMenuButtonTest, NotifyClickExecutesAction) {
TriggerNotifyClick();
EXPECT_EQ(1, controller_->execute_action_count());
}

TEST_F(ExtensionsMenuButtonTest, UpdatesToDisplayTooltip) {
EXPECT_EQ(button_->GetTooltipText(gfx::Point()), initial_tooltip_);

base::string16 tooltip = base::ASCIIToUTF16("New Tooltip");
controller_->SetTooltip(tooltip);

EXPECT_EQ(button_->GetTooltipText(gfx::Point()), tooltip);
}
16 changes: 14 additions & 2 deletions chrome/browser/ui/views/extensions/extensions_menu_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,11 @@ void ExtensionsMenuView::Repopulate() {
}

AddChildView(std::make_unique<views::Separator>());
auto icon_view = CreateFixedSizeIconView();
icon_view->SetImage(CreateVectorIcon(kSettingsIcon));
auto footer = std::make_unique<HoverButton>(
this, CreateVectorIcon(kSettingsIcon),
l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION));
this, std::move(icon_view),
l10n_util::GetStringUTF16(IDS_MANAGE_EXTENSION), base::string16());
footer->set_id(EXTENSIONS_SETTINGS_ID);
manage_extensions_button_for_testing_ = footer.get();
AddChildView(std::move(footer));
Expand Down Expand Up @@ -167,6 +169,16 @@ void ExtensionsMenuView::Hide() {
g_extensions_dialog->GetWidget()->Close();
}

// static
std::unique_ptr<views::ImageView>
ExtensionsMenuView::CreateFixedSizeIconView() {
// Note that this size is larger than the 16dp extension icons as it needs to
// accommodate 24dp click-to-script badging and surrounding shadows.
auto image_view = std::make_unique<views::ImageView>();
image_view->SetPreferredSize(gfx::Size(28, 28));
return image_view;
}

ExtensionsMenuView* ExtensionsMenuView::GetExtensionsMenuViewForTesting() {
return g_extensions_dialog;
}
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/extensions/extensions_menu_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace views {
class Button;
class ImageView;
} // namespace views

class ToolbarActionsBar;
Expand All @@ -35,6 +36,7 @@ class ExtensionsMenuView : public views::ButtonListener,
static bool IsShowing();
static void Hide();
static ExtensionsMenuView* GetExtensionsMenuViewForTesting();
static std::unique_ptr<views::ImageView> CreateFixedSizeIconView();

// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
Expand Down

0 comments on commit b30594b

Please sign in to comment.