Skip to content

Commit

Permalink
[responsive toolbar] Add menu item activation
Browse files Browse the repository at this point in the history 
Change-Id: I1c3c07b2c5f677ac84e398c8d651ceb3a3a9bc3e
Bug: 1469146
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4919933
Commit-Queue: pengchao Cai <pengchaocai@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1211694}
  • Loading branch information
Pengchao Cai authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent 3566b8f commit 483e694
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 77 deletions.
112 changes: 76 additions & 36 deletions chrome/browser/ui/views/toolbar/toolbar_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ ToolbarController::ToolbarController(
toolbar_container_view_(toolbar_container_view),
overflow_button_(overflow_button) {
for (ui::ElementIdentifier id : element_ids) {
views::View* toolbar_element = FindToolbarElementWithId(id);
auto* const toolbar_element =
FindToolbarElementWithId(toolbar_container_view_, id);
if (!toolbar_element) {
continue;
}
Expand All @@ -80,10 +81,15 @@ ToolbarController::ToolbarController(
->WithOrder(element_flex_order_start++);
toolbar_element->SetProperty(views::kFlexBehaviorKey, flex_spec);

// Create pop out state and pop out handlers to support pop out.
// Check `element_info_map` is constructed correctly i.e.
// 1. keys should be a super set of `element_ids_`,
// 2. ResponsiveElementInfo::activate_identifier is non-null.
auto it = element_info_map.find(id);
if (it != element_info_map.end() &&
it->second.observed_identifier.has_value()) {
CHECK(it != element_info_map.end());
CHECK(it->second.activate_identifier);

// Create pop out state and pop out handlers to support pop out.
if (it->second.observed_identifier.has_value()) {
auto state = std::make_unique<PopOutState>();
if (original_spec) {
state->original_spec =
Expand All @@ -109,22 +115,28 @@ ToolbarController::GetDefaultElementInfoMap() {
{IDS_OVERFLOW_MENU_ITEM_TEXT_EXTENSIONS,
kExtensionsMenuButtonElementId}},
{kToolbarSidePanelContainerElementId,
{IDS_OVERFLOW_MENU_ITEM_TEXT_SIDE_PANEL}},
{kToolbarHomeButtonElementId, {IDS_OVERFLOW_MENU_ITEM_TEXT_HOME}},
{IDS_OVERFLOW_MENU_ITEM_TEXT_SIDE_PANEL,
kToolbarSidePanelButtonElementId}},
{kToolbarHomeButtonElementId,
{IDS_OVERFLOW_MENU_ITEM_TEXT_HOME, kToolbarHomeButtonElementId}},
{kToolbarChromeLabsButtonElementId,
{IDS_OVERFLOW_MENU_ITEM_TEXT_LABS, kToolbarChromeLabsBubbleElementId}},
{IDS_OVERFLOW_MENU_ITEM_TEXT_LABS, kToolbarChromeLabsButtonElementId,
kToolbarChromeLabsBubbleElementId}},
{kToolbarMediaButtonElementId,
{IDS_OVERFLOW_MENU_ITEM_TEXT_MEDIA_CONTROLS,
kToolbarMediaBubbleElementId}},
kToolbarMediaButtonElementId, kToolbarMediaBubbleElementId}},
{kToolbarDownloadButtonElementId,
{IDS_OVERFLOW_MENU_ITEM_TEXT_DOWNLOADS,
{IDS_OVERFLOW_MENU_ITEM_TEXT_DOWNLOADS, kToolbarDownloadButtonElementId,
kToolbarDownloadBubbleElementId}},
{kToolbarForwardButtonElementId, {IDS_OVERFLOW_MENU_ITEM_TEXT_FORWARD}},
{kToolbarForwardButtonElementId,
{IDS_OVERFLOW_MENU_ITEM_TEXT_FORWARD, kToolbarForwardButtonElementId}},
});
}

bool ToolbarController::PopOut(ui::ElementIdentifier identifier) {
views::View* element = FindToolbarElementWithId(identifier);
auto* const element =
FindToolbarElementWithId(toolbar_container_view_, identifier);

if (!element) {
LOG(ERROR) << "Cannot find toolbar element id: " << identifier;
return false;
Expand Down Expand Up @@ -153,7 +165,9 @@ bool ToolbarController::PopOut(ui::ElementIdentifier identifier) {
}

bool ToolbarController::EndPopOut(ui::ElementIdentifier identifier) {
views::View* element = FindToolbarElementWithId(identifier);
auto* const element =
FindToolbarElementWithId(toolbar_container_view_, identifier);

if (!element) {
LOG(ERROR) << "Cannot find toolbar element id: " << identifier;
return false;
Expand All @@ -177,52 +191,78 @@ bool ToolbarController::EndPopOut(ui::ElementIdentifier identifier) {
bool ToolbarController::ShouldShowOverflowButton() {
// Once at least one button has been dropped by layout manager show overflow
// button.
return GetOverflowedElements().size() > 0;
for (ui::ElementIdentifier id : element_ids_) {
if (IsOverflowed(id)) {
return true;
}
}
return false;
}

std::u16string ToolbarController::GenerateMenuText(const views::View* element) {
ui::ElementIdentifier id = element->GetProperty(views::kElementIdentifierKey);
CHECK(id);

std::u16string ToolbarController::GetMenuText(ui::ElementIdentifier id) {
return l10n_util::GetStringUTF16(element_info_map_.at(id).menu_text_id);
}

const views::View* ToolbarController::FindToolbarElementWithId(
ui::ElementIdentifier id) const {
const views::View::Views toolbar_elements =
toolbar_container_view_->children();
for (const views::View* element : toolbar_elements) {
if (element->GetProperty(views::kElementIdentifierKey) == id) {
return element;
views::View* ToolbarController::FindToolbarElementWithId(
views::View* view,
ui::ElementIdentifier id) {
if (!view) {
return nullptr;
}
if (view->GetProperty(views::kElementIdentifierKey) == id) {
return view;
}
for (auto* child : view->children()) {
if (auto* result = FindToolbarElementWithId(child, id)) {
return result;
}
}
return nullptr;
}

std::vector<const views::View*> ToolbarController::GetOverflowedElements() {
std::vector<const views::View*> overflowed_buttons;
const views::FlexLayout* flex_layout = static_cast<views::FlexLayout*>(
toolbar_container_view_->GetLayoutManager());
std::vector<ui::ElementIdentifier> ToolbarController::GetOverflowedElements() {
std::vector<ui::ElementIdentifier> overflowed_buttons;
for (ui::ElementIdentifier id : element_ids_) {
const views::View* toolbar_element = FindToolbarElementWithId(id);
if (flex_layout->CanBeVisible(toolbar_element) &&
!toolbar_element->GetVisible()) {
overflowed_buttons.push_back(toolbar_element);
if (IsOverflowed(id)) {
overflowed_buttons.push_back(id);
}
}
return overflowed_buttons;
}

bool ToolbarController::IsOverflowed(ui::ElementIdentifier id) {
const auto* const toolbar_element =
FindToolbarElementWithId(toolbar_container_view_, id);
const views::FlexLayout* flex_layout = static_cast<views::FlexLayout*>(
toolbar_container_view_->GetLayoutManager());
return flex_layout->CanBeVisible(toolbar_element) &&
!toolbar_element->GetVisible();
}

std::unique_ptr<ui::SimpleMenuModel>
ToolbarController::CreateOverflowMenuModel() {
CHECK(overflow_button_->GetVisible());
auto menu_model = std::make_unique<ui::SimpleMenuModel>(this);
int menu_id_start = 0;
for (auto* toolbar_element : GetOverflowedElements()) {
menu_model->AddItem(menu_id_start++, GenerateMenuText(toolbar_element));
for (size_t i = 0; i < element_ids_.size(); ++i) {
if (IsOverflowed(element_ids_[i])) {
menu_model->AddItem(i, GetMenuText(element_ids_[i]));
}
}
return menu_model;
}

void ToolbarController::ExecuteCommand(int command_id, int event_flags) {}
ui::ElementIdentifier ToolbarController::GetHiddenElementOfCommandId(
int command_id) const {
return element_ids_.at(command_id);
}

void ToolbarController::ExecuteCommand(int command_id, int event_flags) {
ui::ElementIdentifier activate_identifier =
element_info_map_.at(GetHiddenElementOfCommandId(command_id))
.activate_identifier;
const auto* const element =
FindToolbarElementWithId(toolbar_container_view_, activate_identifier);
CHECK(element);
const auto* button = AsViewClass<views::Button>(element);
button->button_controller()->NotifyClick();
}
32 changes: 21 additions & 11 deletions chrome/browser/ui/views/toolbar/toolbar_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ class ToolbarController : public ui::SimpleMenuModel::Delegate {
// Menu text when the element is overflow to the overflow menu.
int menu_text_id;

// The toolbar button to be activated with menu text pressed. This is not
// necessarily the same as the element that overflows. E.g. when the
// overflowed element is kToolbarExtensionsContainerElementId the
// `activate_identifier` should be kExtensionsMenuButtonElementId.
ui::ElementIdentifier activate_identifier;

// Pop out button when `observed_identifier` is shown. End pop out when it's
// hidden.
absl::optional<ui::ElementIdentifier> observed_identifier;
Expand Down Expand Up @@ -120,29 +126,33 @@ class ToolbarController : public ui::SimpleMenuModel::Delegate {
std::unique_ptr<ui::SimpleMenuModel> CreateOverflowMenuModel();

// Generate menu text from the responsive element.
virtual std::u16string GenerateMenuText(const views::View* element);
virtual std::u16string GetMenuText(ui::ElementIdentifier id);

// Utility that recursively searches for a view with `id` from `view`.
static views::View* FindToolbarElementWithId(views::View* view,
ui::ElementIdentifier id);

private:
friend class ToolbarControllerInteractiveTest;
friend class ToolbarControllerUnitTest;

// Searches for a toolbar element from `toolbar_container_view_` with `id`.
views::View* FindToolbarElementWithId(ui::ElementIdentifier id) {
return const_cast<views::View*>(
std::as_const(*this).FindToolbarElementWithId(id));
}
const views::View* FindToolbarElementWithId(ui::ElementIdentifier id) const;

// Returns currently hidden elements.
std::vector<const views::View*> GetOverflowedElements();
std::vector<ui::ElementIdentifier> GetOverflowedElements();

// Check if element has overflowed.
bool IsOverflowed(ui::ElementIdentifier id);

// ui::SimpleMenuModel::Delegate:
void ExecuteCommand(int command_id, int event_flags) override;

// Returns the element in `element_ids_` at index `command_id`.
ui::ElementIdentifier GetHiddenElementOfCommandId(int command_id) const;

// The toolbar elements managed by this controller.
// Order matters as each will be assigned with a flex order that increments by
// 1 starting from `element_flex_order_start_`. So the last element drops out
// first once overflow starts.
// first once overflow starts. This also serves as a map that its indices are
// used as command ids in overflowed menu model.
const std::vector<ui::ElementIdentifier> element_ids_;

const ToolbarController::ResponsiveElementInfoMap element_info_map_;
Expand All @@ -151,7 +161,7 @@ class ToolbarController : public ui::SimpleMenuModel::Delegate {
const int element_flex_order_start_;

// Reference to ToolbarView::container_view_. Must outlive `this`.
const raw_ptr<const views::View> toolbar_container_view_;
const raw_ptr<views::View> toolbar_container_view_;

// The button with a chevron icon that indicates at least one element in
// `element_ids_` overflows. Owned by `toolbar_container_view_`.
Expand Down
90 changes: 79 additions & 11 deletions chrome/browser/ui/views/toolbar/toolbar_controller_interactive_uitest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/toolbar/chrome_labs_button.h"
#include "chrome/browser/ui/views/toolbar/toolbar_controller.h"
#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
#include "chrome/test/base/interactive_test_utils.h"
Expand All @@ -27,6 +28,8 @@ class ToolbarControllerInteractiveTest : public InteractiveBrowserTest {
}

void SetUpOnMainThread() override {
ASSERT_TRUE(embedded_test_server()->InitializeAndListen());
embedded_test_server()->StartAcceptingConnections();
InteractiveBrowserTest::SetUpOnMainThread();
browser_view_ = BrowserView::GetBrowserViewForBrowser(browser());
toolbar_controller_ = const_cast<ToolbarController*>(
Expand All @@ -46,6 +49,7 @@ class ToolbarControllerInteractiveTest : public InteractiveBrowserTest {
overflow_button_ = nullptr;
toolbar_controller_ = nullptr;
browser_view_ = nullptr;
EXPECT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());
InteractiveBrowserTest::TearDownOnMainThread();
}

Expand Down Expand Up @@ -78,12 +82,34 @@ class ToolbarControllerInteractiveTest : public InteractiveBrowserTest {
// width.
void MaybeAddDummyButtonsToToolbarView() {
while (GetOverflowThresholdWidth() <= kBrowserContentAllowedMinimumWidth) {
auto button = std::make_unique<ToolbarButton>();
button->SetPreferredSize(dummy_button_size_);
button->SetMinSize(dummy_button_size_);
button->SetAccessibleName(u"dummybutton");
button->SetVisible(true);
toolbar_container_view_->AddChildView(std::move(button));
toolbar_container_view_->AddChildView(CreateADummyButton());
}
}

std::unique_ptr<ToolbarButton> CreateADummyButton() {
auto button = std::make_unique<ToolbarButton>();
button->SetPreferredSize(dummy_button_size_);
button->SetMinSize(dummy_button_size_);
button->SetAccessibleName(u"dummybutton");
button->SetVisible(true);
return button;
}

// Forces `id` to overflow by filling toolbar with dummy buttons.
void AddDummyButtonsToToolbarTillElementOverflows(ui::ElementIdentifier id) {
// The element must be in toolbar.
const auto* element = toolbar_controller_->FindToolbarElementWithId(
toolbar_container_view_, id);
ASSERT_NE(element, nullptr);

// This element must have been managed by controller.
EXPECT_TRUE(std::find(element_ids_.begin(), element_ids_.end(), id) !=
element_ids_.end());

SetBrowserWidth(kBrowserContentAllowedMinimumWidth);
while (element->GetVisible()) {
toolbar_container_view_->AddChildView(CreateADummyButton());
toolbar_container_view_->parent()->Layout();
}
}

Expand All @@ -92,13 +118,13 @@ class ToolbarControllerInteractiveTest : public InteractiveBrowserTest {
auto CheckMenuMatchesOverflowedElements() {
return Steps(Check(base::BindLambdaForTesting([this]() {
const ui::SimpleMenuModel* menu = GetOverflowMenu();
std::vector<const views::View*> overflowed_elements =
std::vector<ui::ElementIdentifier> overflowed_elements =
GetOverflowedElements();
EXPECT_NE(menu, nullptr);
EXPECT_GT(menu->GetItemCount(), size_t(0));
EXPECT_EQ(menu->GetItemCount(), overflowed_elements.size());
for (size_t i = 0; i < menu->GetItemCount(); ++i) {
if (menu->GetLabelAt(i).compare(toolbar_controller_->GenerateMenuText(
if (menu->GetLabelAt(i).compare(toolbar_controller_->GetMenuText(
overflowed_elements[i])) != 0) {
return false;
}
Expand All @@ -107,12 +133,30 @@ class ToolbarControllerInteractiveTest : public InteractiveBrowserTest {
})));
}

auto ActivateMenuItemWithElementId(ui::ElementIdentifier id) {
return Do(base::BindLambdaForTesting([=]() {
int command_id = -1;
for (size_t i = 0; i < element_ids_.size(); ++i) {
if (element_ids_[i] == id) {
command_id = i;
break;
}
}
auto* menu = const_cast<ui::SimpleMenuModel*>(GetOverflowMenu());
auto index = menu->GetIndexOfCommandId(command_id);
EXPECT_TRUE(index.has_value());
menu->ActivatedAt(index.value());
}));
}

void SetBrowserWidth(int width) {
browser_view_->SetSize({width, browser_view_->size().height()});
browser_view_->SetContentsSize(
{width, browser_view_->GetContentsSize().height()});
}

const views::View* FindToolbarElementWithId(ui::ElementIdentifier id) const {
return toolbar_controller_->FindToolbarElementWithId(id);
return toolbar_controller_->FindToolbarElementWithId(
toolbar_container_view_, id);
}

const views::View* overflow_button() const { return overflow_button_; }
Expand All @@ -121,7 +165,7 @@ class ToolbarControllerInteractiveTest : public InteractiveBrowserTest {
return element_ids_;
}
int overflow_threshold_width() const { return overflow_threshold_width_; }
std::vector<const views::View*> GetOverflowedElements() {
std::vector<ui::ElementIdentifier> GetOverflowedElements() {
return toolbar_controller_->GetOverflowedElements();
}
const ui::SimpleMenuModel* GetOverflowMenu() {
Expand Down Expand Up @@ -233,3 +277,27 @@ IN_PROC_BROWSER_TEST_F(ToolbarControllerInteractiveTest,
WaitForActivate(kToolbarOverflowButtonElementId),
CheckMenuMatchesOverflowedElements());
}

IN_PROC_BROWSER_TEST_F(ToolbarControllerInteractiveTest,
ActivateActionElementFromMenu) {
DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kPrimaryTabPageElementId);
const auto back_url = embedded_test_server()->GetURL("/back");
const auto forward_url = embedded_test_server()->GetURL("/forward");
RunTestSequence(
InstrumentTab(kPrimaryTabPageElementId),
NavigateWebContents(kPrimaryTabPageElementId, back_url),
NavigateWebContents(kPrimaryTabPageElementId, forward_url),
PressButton(kToolbarBackButtonElementId),
WaitForWebContentsNavigation(kPrimaryTabPageElementId, back_url),
EnsurePresent(kToolbarForwardButtonElementId),
Do(base::BindLambdaForTesting([this]() {
AddDummyButtonsToToolbarTillElementOverflows(
kToolbarForwardButtonElementId);
})),
WaitForHide(kToolbarForwardButtonElementId),
PressButton(kToolbarOverflowButtonElementId),
ActivateMenuItemWithElementId(kToolbarForwardButtonElementId),

// Forward navigation is triggered after activating menu item.
WaitForWebContentsNavigation(kPrimaryTabPageElementId, forward_url));
}

0 comments on commit 483e694

Please sign in to comment.