Commit 30159b4f authored by Connie Wan's avatar Connie Wan Committed by Commit Bot

Trigger the Editor Bubble from context menu keys

The primary feature change is in the group header. See inline comments for an issues I had with the editor bubble: the text field, which automatically gets focus when the bubble opens, would also spawn a context menu. There isn't a single event (in the sense of a keyboard event or mouse event) that catches this, so I had to manually work around stopping the propagation.

Bug: 1056362
Change-Id: I8ca7f10bc4b05ece0ed1eb52be61a0341210e182
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079835
Commit-Queue: Connie Wan <connily@chromium.org>
Reviewed-by: default avatarCharlene Yan <cyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746847}
parent e4048c2e
...@@ -51,9 +51,11 @@ ...@@ -51,9 +51,11 @@
views::Widget* TabGroupEditorBubbleView::Show( views::Widget* TabGroupEditorBubbleView::Show(
const Browser* browser, const Browser* browser,
const tab_groups::TabGroupId& group, const tab_groups::TabGroupId& group,
TabGroupHeader* anchor_view) { TabGroupHeader* anchor_view,
bool stop_context_menu_propagation) {
views::Widget* const widget = BubbleDialogDelegateView::CreateBubble( views::Widget* const widget = BubbleDialogDelegateView::CreateBubble(
new TabGroupEditorBubbleView(browser, group, anchor_view, base::nullopt)); new TabGroupEditorBubbleView(browser, group, anchor_view, base::nullopt,
stop_context_menu_propagation));
widget->Show(); widget->Show();
return widget; return widget;
} }
...@@ -63,8 +65,9 @@ views::Widget* TabGroupEditorBubbleView::ShowWithRect( ...@@ -63,8 +65,9 @@ views::Widget* TabGroupEditorBubbleView::ShowWithRect(
const Browser* browser, const Browser* browser,
const tab_groups::TabGroupId& group, const tab_groups::TabGroupId& group,
gfx::Rect anchor_rect) { gfx::Rect anchor_rect) {
views::Widget* const widget = BubbleDialogDelegateView::CreateBubble( views::Widget* const widget =
new TabGroupEditorBubbleView(browser, group, nullptr, anchor_rect)); BubbleDialogDelegateView::CreateBubble(new TabGroupEditorBubbleView(
browser, group, nullptr, anchor_rect, false));
widget->Show(); widget->Show();
return widget; return widget;
} }
...@@ -87,7 +90,8 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView( ...@@ -87,7 +90,8 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
const Browser* browser, const Browser* browser,
const tab_groups::TabGroupId& group, const tab_groups::TabGroupId& group,
TabGroupHeader* anchor_view, TabGroupHeader* anchor_view,
base::Optional<gfx::Rect> anchor_rect) base::Optional<gfx::Rect> anchor_rect,
bool stop_context_menu_propagation)
: browser_(browser), : browser_(browser),
group_(group), group_(group),
title_field_controller_(this), title_field_controller_(this),
...@@ -146,8 +150,8 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView( ...@@ -146,8 +150,8 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
title_field_container->SetBorder(views::CreateEmptyBorder(gfx::Insets( title_field_container->SetBorder(views::CreateEmptyBorder(gfx::Insets(
0, color_element_insets.left(), vertical_dialog_content_spacing, 0, color_element_insets.left(), vertical_dialog_content_spacing,
color_element_insets.right()))); color_element_insets.right())));
title_field_ = title_field_ = title_field_container->AddChildView(
title_field_container->AddChildView(std::make_unique<views::Textfield>()); std::make_unique<TitleField>(stop_context_menu_propagation));
title_field_->SetText(title); title_field_->SetText(title);
title_field_->SetAccessibleName(base::ASCIIToUTF16("Group title")); title_field_->SetAccessibleName(base::ASCIIToUTF16("Group title"));
title_field_->SetPlaceholderText( title_field_->SetPlaceholderText(
...@@ -310,6 +314,21 @@ bool TabGroupEditorBubbleView::TitleFieldController::HandleKeyEvent( ...@@ -310,6 +314,21 @@ bool TabGroupEditorBubbleView::TitleFieldController::HandleKeyEvent(
return false; return false;
} }
void TabGroupEditorBubbleView::TitleField::ShowContextMenu(
const gfx::Point& p,
ui::MenuSourceType source_type) {
// There is no easy way to stop the propagation of a ShowContextMenu event,
// which is sometimes used to open the bubble itself. So when the bubble is
// opened this way, we manually hide the textfield's context menu the first
// time. Otherwise, the textfield, which is automatically focused, would show
// an extra context menu when the bubble first opens.
if (stop_context_menu_propagation_) {
stop_context_menu_propagation_ = false;
return;
}
views::Textfield::ShowContextMenu(p, source_type);
}
TabGroupEditorBubbleView::ButtonListener::ButtonListener( TabGroupEditorBubbleView::ButtonListener::ButtonListener(
const Browser* browser, const Browser* browser,
tab_groups::TabGroupId group, tab_groups::TabGroupId group,
......
...@@ -40,7 +40,8 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView { ...@@ -40,7 +40,8 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
// Returns an *unowned* pointer to the bubble's widget. // Returns an *unowned* pointer to the bubble's widget.
static views::Widget* Show(const Browser* browser, static views::Widget* Show(const Browser* browser,
const tab_groups::TabGroupId& group, const tab_groups::TabGroupId& group,
TabGroupHeader* anchor_view); TabGroupHeader* anchor_view,
bool stop_context_menu_propagation = false);
// Shows the editor for |group| using a rect as an anchor. Should only be used // Shows the editor for |group| using a rect as an anchor. Should only be used
// if the TabGroupHeader is not available as an anchor, e.g. in WebUI. Returns // if the TabGroupHeader is not available as an anchor, e.g. in WebUI. Returns
...@@ -58,7 +59,8 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView { ...@@ -58,7 +59,8 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
TabGroupEditorBubbleView(const Browser* browser, TabGroupEditorBubbleView(const Browser* browser,
const tab_groups::TabGroupId& group, const tab_groups::TabGroupId& group,
TabGroupHeader* anchor_view, TabGroupHeader* anchor_view,
base::Optional<gfx::Rect> anchor_rect); base::Optional<gfx::Rect> anchor_rect,
bool stop_context_menu_propagation);
~TabGroupEditorBubbleView() override; ~TabGroupEditorBubbleView() override;
void UpdateGroup(); void UpdateGroup();
...@@ -88,6 +90,26 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView { ...@@ -88,6 +90,26 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
TitleFieldController title_field_controller_; TitleFieldController title_field_controller_;
class TitleField : public views::Textfield {
public:
explicit TitleField(bool stop_context_menu_propagation)
: stop_context_menu_propagation_(stop_context_menu_propagation) {}
~TitleField() override = default;
// views::Textfield:
void ShowContextMenu(const gfx::Point& p,
ui::MenuSourceType source_type) override;
private:
// Whether the context menu should be hidden the first time it shows.
// Needed because there is no easy way to stop the propagation of a
// ShowContextMenu event, which is sometimes used to open the bubble
// itself.
bool stop_context_menu_propagation_;
};
TitleField* title_field_;
class ButtonListener : public views::ButtonListener { class ButtonListener : public views::ButtonListener {
public: public:
explicit ButtonListener(const Browser* browser, explicit ButtonListener(const Browser* browser,
...@@ -105,8 +127,6 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView { ...@@ -105,8 +127,6 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
ButtonListener button_listener_; ButtonListener button_listener_;
views::Textfield* title_field_;
std::vector<tab_groups::TabGroupColorId> color_ids_; std::vector<tab_groups::TabGroupColorId> color_ids_;
std::vector<std::pair<SkColor, base::string16>> colors_; std::vector<std::pair<SkColor, base::string16>> colors_;
ColorPickerView* color_selector_; ColorPickerView* color_selector_;
......
...@@ -78,6 +78,7 @@ TabGroupHeader::TabGroupHeader(TabStrip* tab_strip, ...@@ -78,6 +78,7 @@ TabGroupHeader::TabGroupHeader(TabStrip* tab_strip,
DCHECK(tab_strip); DCHECK(tab_strip);
set_group(group); set_group(group);
set_context_menu_controller(this);
// The size and color of the chip are set in VisualsChanged(). // The size and color of the chip are set in VisualsChanged().
title_chip_ = AddChildView(std::make_unique<views::View>()); title_chip_ = AddChildView(std::make_unique<views::View>());
...@@ -227,6 +228,17 @@ TabSizeInfo TabGroupHeader::GetTabSizeInfo() const { ...@@ -227,6 +228,17 @@ TabSizeInfo TabGroupHeader::GetTabSizeInfo() const {
return size_info; return size_info;
} }
void TabGroupHeader::ShowContextMenuForViewImpl(
views::View* source,
const gfx::Point& point,
ui::MenuSourceType source_type) {
if (editor_bubble_tracker_.is_open())
return;
editor_bubble_tracker_.Opened(TabGroupEditorBubbleView::Show(
tab_strip_->controller()->GetBrowser(), group().value(), this, true));
}
int TabGroupHeader::CalculateWidth() const { int TabGroupHeader::CalculateWidth() const {
// We don't want tabs to visually overlap group headers, so we add that space // We don't want tabs to visually overlap group headers, so we add that space
// to the width to compensate. We don't want to actually remove the overlap // to the width to compensate. We don't want to actually remove the overlap
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "chrome/browser/ui/views/tabs/tab_slot_view.h" #include "chrome/browser/ui/views/tabs/tab_slot_view.h"
#include "components/tab_groups/tab_group_id.h" #include "components/tab_groups/tab_group_id.h"
#include "ui/views/context_menu_controller.h"
#include "ui/views/controls/focus_ring.h" #include "ui/views/controls/focus_ring.h"
#include "ui/views/widget/widget_observer.h" #include "ui/views/widget/widget_observer.h"
...@@ -21,7 +22,7 @@ class View; ...@@ -21,7 +22,7 @@ class View;
// View for tab group headers in the tab strip, which are markers of group // View for tab group headers in the tab strip, which are markers of group
// boundaries. There is one header for each group, which is included in the tab // boundaries. There is one header for each group, which is included in the tab
// strip flow and positioned left of the leftmost tab in the group. // strip flow and positioned left of the leftmost tab in the group.
class TabGroupHeader : public TabSlotView { class TabGroupHeader : public TabSlotView, public views::ContextMenuController {
public: public:
TabGroupHeader(TabStrip* tab_strip, const tab_groups::TabGroupId& group); TabGroupHeader(TabStrip* tab_strip, const tab_groups::TabGroupId& group);
~TabGroupHeader() override; ~TabGroupHeader() override;
...@@ -39,6 +40,11 @@ class TabGroupHeader : public TabSlotView { ...@@ -39,6 +40,11 @@ class TabGroupHeader : public TabSlotView {
TabSlotView::ViewType GetTabSlotViewType() const override; TabSlotView::ViewType GetTabSlotViewType() const override;
TabSizeInfo GetTabSizeInfo() const override; TabSizeInfo GetTabSizeInfo() const override;
// views::ContextMenuController:
void ShowContextMenuForViewImpl(views::View* source,
const gfx::Point& point,
ui::MenuSourceType source_type) override;
// Updates our visual state according to the tab_groups::TabGroupVisualData // Updates our visual state according to the tab_groups::TabGroupVisualData
// for our group. // for our group.
void VisualsChanged(); void VisualsChanged();
......
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