Commit 86706c8d authored by Connie Wan's avatar Connie Wan Committed by Commit Bot

Refactor Editor Bubble to take the Browser (Editor Bubble refactor Part 1)

WebUI has access to the Browser, so this should allow interfacing with Mohnstrudel. It also allows us to clean up a good portion of TabStripController that was only being used by the Editor Bubble.

Still left to do: make it so that the dialog doesn't necesesarily need to take a TabGroupHeader.

Bug: 1055537
Change-Id: Ia341453a092a8083075d7a31b6e39705d883fbfc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2072365
Commit-Queue: Connie Wan <connily@chromium.org>
Reviewed-by: default avatarCharlene Yan <cyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745309}
parent d1bfe22f
......@@ -306,8 +306,7 @@ bool BrowserTabStripController::BeforeCloseTab(int model_index,
return result == Browser::WarnBeforeClosingResult::kOkToClose;
}
void BrowserTabStripController::CloseTab(int model_index,
CloseTabSource source) {
void BrowserTabStripController::CloseTab(int model_index) {
// Cancel any pending tab transition.
hover_tab_selector_.CancelTabTransition();
......@@ -316,26 +315,6 @@ void BrowserTabStripController::CloseTab(int model_index,
TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
}
void BrowserTabStripController::CloseAllTabsInGroup(
const tab_groups::TabGroupId& group) {
tabstrip_->UpdateHoverCard(nullptr);
std::vector<int> tabs = ListTabsInGroup(group);
for (int i = tabs.size() - 1; i >= 0; --i)
CloseTab(tabs[i], CLOSE_TAB_FROM_MOUSE);
}
void BrowserTabStripController::UngroupAllTabsInGroup(
const tab_groups::TabGroupId& group) {
model_->RemoveFromGroup(ListTabsInGroup(group));
}
void BrowserTabStripController::AddNewTabInGroup(
const tab_groups::TabGroupId& group) {
const std::vector<int> tabs = ListTabsInGroup(group);
model_->delegate()->AddTabAt(GURL(), tabs.back() + 1, true, group);
}
void BrowserTabStripController::AddTabToGroup(
int model_index,
const tab_groups::TabGroupId& group) {
......
......@@ -63,10 +63,7 @@ class BrowserTabStripController : public TabStripController,
void ToggleSelected(int model_index) override;
void AddSelectionFromAnchorTo(int model_index) override;
bool BeforeCloseTab(int model_index, CloseTabSource source) override;
void CloseTab(int model_index, CloseTabSource source) override;
void CloseAllTabsInGroup(const tab_groups::TabGroupId& group) override;
void UngroupAllTabsInGroup(const tab_groups::TabGroupId& group) override;
void AddNewTabInGroup(const tab_groups::TabGroupId& group) override;
void CloseTab(int model_index) override;
void AddTabToGroup(int model_index,
const tab_groups::TabGroupId& group) override;
void RemoveTabFromGroup(int model_index) override;
......
......@@ -88,15 +88,6 @@ void FakeBaseTabStripController::SetVisualDataForGroup(
fake_group_data_ = visual_data;
}
void FakeBaseTabStripController::CloseAllTabsInGroup(
const tab_groups::TabGroupId& group) {}
void FakeBaseTabStripController::UngroupAllTabsInGroup(
const tab_groups::TabGroupId& group) {}
void FakeBaseTabStripController::AddNewTabInGroup(
const tab_groups::TabGroupId& group) {}
void FakeBaseTabStripController::AddTabToGroup(
int model_index,
const tab_groups::TabGroupId& group) {
......@@ -196,7 +187,7 @@ bool FakeBaseTabStripController::BeforeCloseTab(int index,
return true;
}
void FakeBaseTabStripController::CloseTab(int index, CloseTabSource source) {
void FakeBaseTabStripController::CloseTab(int index) {
RemoveTab(index);
}
......
......@@ -45,7 +45,7 @@ class FakeBaseTabStripController : public TabStripController {
void ToggleSelected(int index) override;
void AddSelectionFromAnchorTo(int index) override;
bool BeforeCloseTab(int index, CloseTabSource source) override;
void CloseTab(int index, CloseTabSource source) override;
void CloseTab(int index) override;
void MoveTab(int from_index, int to_index) override;
void MoveGroup(const tab_groups::TabGroupId&, int to_index) override;
void ShowContextMenuForTab(Tab* tab,
......@@ -70,9 +70,6 @@ class FakeBaseTabStripController : public TabStripController {
const tab_groups::TabGroupVisualData& visual_data) override;
std::vector<int> ListTabsInGroup(
const tab_groups::TabGroupId& group) const override;
void CloseAllTabsInGroup(const tab_groups::TabGroupId& group) override;
void UngroupAllTabsInGroup(const tab_groups::TabGroupId& group) override;
void AddNewTabInGroup(const tab_groups::TabGroupId& group) override;
void AddTabToGroup(int model_index,
const tab_groups::TabGroupId& group) override;
void RemoveTabFromGroup(int model_index) override;
......
......@@ -17,12 +17,16 @@
#include "base/no_destructor.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/chrome_pages.h"
#include "chrome/browser/ui/tabs/tab_group.h"
#include "chrome/browser/ui/tabs/tab_group_model.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
#include "chrome/browser/ui/views/bubble_menu_item_factory.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h"
#include "chrome/browser/ui/views/chrome_typography.h"
#include "chrome/browser/ui/views/tabs/color_picker_view.h"
#include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
#include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
#include "chrome/grit/generated_resources.h"
#include "components/tab_groups/tab_group_color.h"
......@@ -45,10 +49,10 @@
// static
views::Widget* TabGroupEditorBubbleView::Show(
TabGroupHeader* anchor_view,
TabStripController* tab_strip_controller,
const Browser* browser,
const tab_groups::TabGroupId& group) {
views::Widget* const widget = BubbleDialogDelegateView::CreateBubble(
new TabGroupEditorBubbleView(anchor_view, tab_strip_controller, group));
new TabGroupEditorBubbleView(anchor_view, browser, group));
widget->Show();
return widget;
}
......@@ -69,18 +73,22 @@ views::View* TabGroupEditorBubbleView::GetInitiallyFocusedView() {
TabGroupEditorBubbleView::TabGroupEditorBubbleView(
TabGroupHeader* anchor_view,
TabStripController* tab_strip_controller,
const Browser* browser,
const tab_groups::TabGroupId& group)
: tab_strip_controller_(tab_strip_controller),
: browser_(browser),
group_(group),
title_field_controller_(this),
button_listener_(tab_strip_controller, anchor_view, group) {
button_listener_(browser, anchor_view, group) {
SetAnchorView(anchor_view);
set_margins(gfx::Insets());
DialogDelegate::set_buttons(ui::DIALOG_BUTTON_NONE);
const base::string16 title = tab_strip_controller_->GetGroupTitle(group_);
const base::string16 title = browser_->tab_strip_model()
->group_model()
->GetTabGroup(group_)
->visual_data()
->title();
title_at_opening_ = title;
DialogDelegate::set_close_callback(base::BindOnce(
&TabGroupEditorBubbleView::OnBubbleClose, base::Unretained(this)));
......@@ -200,7 +208,11 @@ SkColor TabGroupEditorBubbleView::InitColorSet() {
// Keep track of the current group's color, to be returned as the initial
// selected value.
const tab_groups::TabGroupColorId initial_color_id =
tab_strip_controller_->GetGroupColorId(group_);
browser_->tab_strip_model()
->group_model()
->GetTabGroup(group_)
->visual_data()
->color();
SkColor initial_color;
color_ids_.reserve(all_colors.size());
......@@ -221,8 +233,11 @@ SkColor TabGroupEditorBubbleView::InitColorSet() {
void TabGroupEditorBubbleView::UpdateGroup() {
base::Optional<int> selected_element = color_selector_->GetSelectedElement();
TabGroup* tab_group =
browser_->tab_strip_model()->group_model()->GetTabGroup(group_);
const tab_groups::TabGroupColorId current_color =
tab_strip_controller_->GetGroupColorId(group_);
tab_group->visual_data()->color();
const tab_groups::TabGroupColorId updated_color =
selected_element.has_value() ? color_ids_[selected_element.value()]
: current_color;
......@@ -234,7 +249,7 @@ void TabGroupEditorBubbleView::UpdateGroup() {
tab_groups::TabGroupVisualData new_data(title_field_->GetText(),
updated_color);
tab_strip_controller_->SetVisualDataForGroup(group_, new_data);
tab_group->SetVisualData(new_data);
}
void TabGroupEditorBubbleView::OnBubbleClose() {
......@@ -276,39 +291,44 @@ bool TabGroupEditorBubbleView::TitleFieldController::HandleKeyEvent(
}
TabGroupEditorBubbleView::ButtonListener::ButtonListener(
TabStripController* tab_strip_controller,
const Browser* browser,
TabGroupHeader* anchor_view,
tab_groups::TabGroupId group)
: tab_strip_controller_(tab_strip_controller),
anchor_view_(anchor_view),
group_(group) {}
: browser_(browser), anchor_view_(anchor_view), group_(group) {}
void TabGroupEditorBubbleView::ButtonListener::ButtonPressed(
views::Button* sender,
const ui::Event& event) {
TabStripModel* model = browser_->tab_strip_model();
const std::vector<int> tabs_in_group =
model->group_model()->GetTabGroup(group_)->ListTabs();
switch (sender->GetID()) {
case TAB_GROUP_HEADER_CXMENU_NEW_TAB_IN_GROUP:
base::RecordAction(
base::UserMetricsAction("TabGroups_TabGroupBubble_NewTabInGroup"));
tab_strip_controller_->AddNewTabInGroup(group_);
model->delegate()->AddTabAt(GURL(), tabs_in_group.back() + 1, true,
group_);
break;
case TAB_GROUP_HEADER_CXMENU_UNGROUP:
base::RecordAction(
base::UserMetricsAction("TabGroups_TabGroupBubble_Ungroup"));
anchor_view_->RemoveObserverFromWidget(sender->GetWidget());
tab_strip_controller_->UngroupAllTabsInGroup(group_);
model->RemoveFromGroup(tabs_in_group);
break;
case TAB_GROUP_HEADER_CXMENU_CLOSE_GROUP:
base::RecordAction(
base::UserMetricsAction("TabGroups_TabGroupBubble_CloseGroup"));
tab_strip_controller_->CloseAllTabsInGroup(group_);
for (int i = tabs_in_group.size() - 1; i >= 0; --i) {
model->CloseWebContentsAt(
tabs_in_group[i], TabStripModel::CLOSE_USER_GESTURE |
TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
}
break;
case TAB_GROUP_HEADER_CXMENU_FEEDBACK: {
base::RecordAction(
base::UserMetricsAction("TabGroups_TabGroupBubble_SendFeedback"));
const Browser* browser = tab_strip_controller_->GetBrowser();
chrome::ShowFeedbackPage(
browser, chrome::FeedbackSource::kFeedbackSourceDesktopTabGroups,
browser_, chrome::FeedbackSource::kFeedbackSourceDesktopTabGroups,
std::string() /* description_template */,
std::string() /* description_placeholder_text */,
std::string("DESKTOP_TAB_GROUPS") /* category_tag */,
......
......@@ -12,7 +12,7 @@
#include "ui/views/controls/textfield/textfield.h"
#include "ui/views/controls/textfield/textfield_controller.h"
class TabStripController;
class Browser;
namespace gfx {
class Size;
......@@ -37,7 +37,7 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
// Shows the editor for |group|. Returns an *unowned* pointer to the
// bubble's widget.
static views::Widget* Show(TabGroupHeader* anchor_view,
TabStripController* tab_strip_controller,
const Browser* browser,
const tab_groups::TabGroupId& group);
// views::BubbleDialogDelegateView:
......@@ -47,7 +47,7 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
private:
TabGroupEditorBubbleView(TabGroupHeader* anchor_view,
TabStripController* tab_strip_controller,
const Browser* browser,
const tab_groups::TabGroupId& group);
~TabGroupEditorBubbleView() override;
......@@ -57,7 +57,7 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
void OnBubbleClose();
TabStripController* const tab_strip_controller_;
const Browser* const browser_;
const tab_groups::TabGroupId group_;
class TitleFieldController : public views::TextfieldController {
......@@ -80,7 +80,7 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
class ButtonListener : public views::ButtonListener {
public:
explicit ButtonListener(TabStripController* tab_strip_controller,
explicit ButtonListener(const Browser* browser,
TabGroupHeader* anchor_view,
tab_groups::TabGroupId group);
......@@ -88,7 +88,7 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
private:
TabStripController* const tab_strip_controller_;
const Browser* const browser_;
TabGroupHeader* anchor_view_;
const tab_groups::TabGroupId group_;
};
......
......@@ -106,7 +106,7 @@ bool TabGroupHeader::OnKeyPressed(const ui::KeyEvent& event) {
event.key_code() == ui::VKEY_RETURN) &&
!editor_bubble_tracker_.is_open()) {
editor_bubble_tracker_.Opened(TabGroupEditorBubbleView::Show(
this, tab_strip_->controller(), group().value()));
this, tab_strip_->controller()->GetBrowser(), group().value()));
return true;
}
......@@ -155,7 +155,7 @@ bool TabGroupHeader::OnMouseDragged(const ui::MouseEvent& event) {
void TabGroupHeader::OnMouseReleased(const ui::MouseEvent& event) {
if (!dragging()) {
editor_bubble_tracker_.Opened(TabGroupEditorBubbleView::Show(
this, tab_strip_->controller(), group().value()));
this, tab_strip_->controller()->GetBrowser(), group().value()));
}
tab_strip_->EndDrag(END_DRAG_COMPLETE);
}
......@@ -175,7 +175,7 @@ void TabGroupHeader::OnGestureEvent(ui::GestureEvent* event) {
switch (event->type()) {
case ui::ET_GESTURE_TAP: {
editor_bubble_tracker_.Opened(TabGroupEditorBubbleView::Show(
this, tab_strip_->controller(), group().value()));
this, tab_strip_->controller()->GetBrowser(), group().value()));
break;
}
......
......@@ -1641,7 +1641,7 @@ void TabStrip::CloseTab(Tab* tab, CloseTabSource source) {
UpdateHoverCard(nullptr);
if (tab->group().has_value())
base::RecordAction(base::UserMetricsAction("CloseGroupedTab"));
controller_->CloseTab(model_index, source);
controller_->CloseTab(model_index);
}
void TabStrip::ShiftTabLeft(Tab* tab) {
......
......@@ -80,16 +80,7 @@ class TabStripController {
virtual bool BeforeCloseTab(int index, CloseTabSource source) = 0;
// Closes the tab at the specified index in the model.
virtual void CloseTab(int index, CloseTabSource source) = 0;
// Closes all tabs belonging in the given |group|.
virtual void CloseAllTabsInGroup(const tab_groups::TabGroupId& group) = 0;
// Ungroups the tabs at the specified index in the model.
virtual void UngroupAllTabsInGroup(const tab_groups::TabGroupId& group) = 0;
// Adds a new tab to end of the tab group.
virtual void AddNewTabInGroup(const tab_groups::TabGroupId& group) = 0;
virtual void CloseTab(int index) = 0;
// Adds a tab to an existing tab group.
virtual void AddTabToGroup(int model_index,
......
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