WebUI Tab Strip: Show edit dialog for groups

This CL adds support for a user clicking on a tab group chip, which then
triggers the edit bubble dialog to pop open. This CL does not yet add
support for keyboard access to the chip and focus management.

Bug: 1027373, 1055537
Change-Id: I781e0761727f2115a9af2bf3387ee9842c0043e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083927Reviewed-by: Collin Baker <collinbaker@chromium.org>
Commit-Queue: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746948}

chrome/browser/resources/tab_strip/BUILD.gn

@@ -77,6 +77,7 @@ js_library("tab") {
 js_library("tab_group") {
   deps = [
     ":custom_element",
+    ":tab_strip_embedder_proxy",
     ":tabs_api_proxy",
   ]
 }

chrome/browser/resources/tab_strip/tab_group.js

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import {CustomElement} from './custom_element.js';
+import {TabStripEmbedderProxy} from './tab_strip_embedder_proxy.js';
 import {TabGroupVisualData} from './tabs_api_proxy.js';
 
 export class TabGroupElement extends CustomElement {
@@ -10,11 +11,32 @@ export class TabGroupElement extends CustomElement {
     return `{__html_template__}`;
   }
 
+  constructor() {
+    super();
+
+    /** @private @const {!TabStripEmbedderProxy} */
+    this.embedderApi_ = TabStripEmbedderProxy.getInstance();
+
+    this.$('#chip').addEventListener('click', () => this.onClickChip_());
+  }
+
   /** @return {!HTMLElement} */
   getDragImage() {
     return /** @type {!HTMLElement} */ (this.$('#dragImage'));
   }
 
+  /** @private */
+  onClickChip_() {
+    if (!this.dataset.groupId) {
+      return;
+    }
+
+    const boundingBox = this.$('#chip').getBoundingClientRect();
+    this.embedderApi_.showEditDialogForGroup(
+        this.dataset.groupId, boundingBox.left, boundingBox.top,
+        boundingBox.width, boundingBox.height);
+  }
+
   /** @param {boolean} enabled */
   setDragging(enabled) {
     // Since the draggable target is the #chip, if the #chip moves and is no

chrome/browser/resources/tab_strip/tab_strip_embedder_proxy.js

@@ -30,6 +30,19 @@ export class TabStripEmbedderProxy {
     chrome.send('observeThemeChanges');
   }
 
+  /**
+   * @param {string} groupId
+   * @param {number} locationX
+   * @param {number} locationY
+   * @param {number} width
+   * @param {number} height
+   */
+  showEditDialogForGroup(groupId, locationX, locationY, width, height) {
+    chrome.send(
+        'showEditDialogForGroup',
+        [groupId, locationX, locationY, width, height]);
+  }
+
   /**
    * @param {number} tabId
    * @param {number} locationX

chrome/browser/ui/views/frame/webui_tab_strip_container_view.cc

@@ -37,6 +37,7 @@
 #include "chrome/browser/ui/views/feature_promos/feature_promo_bubble_view.h"
 #include "chrome/browser/ui/views/feature_promos/feature_promo_colors.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/tabs/tab_group_editor_bubble_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
 #include "chrome/browser/ui/views/toolbar/webui_tab_counter_button.h"
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui.h"
@@ -552,6 +553,15 @@ void WebUITabStripContainerView::ShowContextMenuAtPoint(
       views::MenuAnchorPosition::kTopLeft, ui::MENU_SOURCE_MOUSE);
 }
 
+void WebUITabStripContainerView::ShowEditDialogForGroupAtPoint(
+    gfx::Point point,
+    gfx::Rect rect,
+    tab_groups::TabGroupId group) {
+  ConvertPointToScreen(this, &point);
+  rect.set_origin(point);
+  TabGroupEditorBubbleView::ShowWithRect(browser_, group, rect);
+}
+
 TabStripUILayout WebUITabStripContainerView::GetLayout() {
   DCHECK(tab_contents_container_);
   return TabStripUILayout::CalculateForWebViewportSize(

chrome/browser/ui/views/frame/webui_tab_strip_container_view.h

@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui.h"
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui_embedder.h"
 #include "chrome/common/buildflags.h"
+#include "components/tab_groups/tab_group_id.h"
 #include "ui/events/event_handler.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/views/accessible_pane_view.h"
@@ -112,6 +113,9 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
   void ShowContextMenuAtPoint(
       gfx::Point point,
       std::unique_ptr<ui::MenuModel> menu_model) override;
+  void ShowEditDialogForGroupAtPoint(gfx::Point point,
+                                     gfx::Rect rect,
+                                     tab_groups::TabGroupId group) override;
   TabStripUILayout GetLayout() override;
   SkColor GetColor(int id) const override;
 

chrome/browser/ui/webui/tab_strip/tab_strip_ui_browsertest.cc

@@ -20,10 +20,12 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/tab_groups/tab_group_id.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
+#include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/base/accelerators/accelerator.h"
@@ -42,6 +44,8 @@ class MockTabStripUIEmbedder : public TabStripUIEmbedder {
   MOCK_METHOD0(CloseContainer, void());
   MOCK_METHOD2(ShowContextMenuAtPoint,
                void(gfx::Point, std::unique_ptr<ui::MenuModel>));
+  MOCK_METHOD3(ShowEditDialogForGroupAtPoint,
+               void(gfx::Point, gfx::Rect, tab_groups::TabGroupId));
   MOCK_METHOD0(GetLayout, TabStripUILayout());
   MOCK_CONST_METHOD1(GetColor, SkColor(int));
 };
@@ -124,3 +128,44 @@ IN_PROC_BROWSER_TEST_F(TabStripUIBrowserTest,
                               content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
                               ISOLATED_WORLD_ID_CHROME_INTERNAL));
 }
+
+IN_PROC_BROWSER_TEST_F(TabStripUIBrowserTest, InvokesEditDialogForGroups) {
+  using ::testing::_;
+
+  tab_groups::TabGroupId group_id =
+      browser()->tab_strip_model()->AddToNewGroup({0});
+
+  const std::string get_chip_js =
+      "const chip = document.querySelector('tabstrip-tab-list')"
+      "    .shadowRoot.querySelector('tabstrip-tab-group')"
+      "    .shadowRoot.querySelector('#chip');"
+      "const chipRect = chip.getBoundingClientRect();";
+  int left =
+      content::EvalJs(webui_contents_.get(), get_chip_js + "chipRect.left",
+                      content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
+                      ISOLATED_WORLD_ID_CHROME_INTERNAL)
+          .ExtractInt();
+  int top = content::EvalJs(webui_contents_.get(), get_chip_js + "chipRect.top",
+                            content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
+                            ISOLATED_WORLD_ID_CHROME_INTERNAL)
+                .ExtractInt();
+  int width =
+      content::EvalJs(webui_contents_.get(), get_chip_js + "chipRect.width",
+                      content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
+                      ISOLATED_WORLD_ID_CHROME_INTERNAL)
+          .ExtractInt();
+  int height =
+      content::EvalJs(webui_contents_.get(), get_chip_js + "chipRect.height",
+                      content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
+                      ISOLATED_WORLD_ID_CHROME_INTERNAL)
+          .ExtractInt();
+
+  EXPECT_CALL(mock_embedder_,
+              ShowEditDialogForGroupAtPoint(gfx::Point(left, top),
+                                            gfx::Rect(width, height), group_id))
+      .Times(1);
+  ASSERT_TRUE(content::ExecJs(webui_contents_.get(),
+                              get_chip_js + "chip.click();",
+                              content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
+                              ISOLATED_WORLD_ID_CHROME_INTERNAL));
+}

chrome/browser/ui/webui/tab_strip/tab_strip_ui_embedder.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_WEBUI_TAB_STRIP_TAB_STRIP_UI_EMBEDDER_H_
 
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui_layout.h"
+#include "components/tab_groups/tab_group_id.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/gfx/geometry/point.h"
@@ -25,6 +26,10 @@ class TabStripUIEmbedder {
       gfx::Point point,
       std::unique_ptr<ui::MenuModel> menu_model) = 0;
 
+  virtual void ShowEditDialogForGroupAtPoint(gfx::Point point,
+                                             gfx::Rect rect,
+                                             tab_groups::TabGroupId group) = 0;
+
   virtual TabStripUILayout GetLayout() = 0;
 
   virtual SkColor GetColor(int id) const = 0;

chrome/browser/ui/webui/tab_strip/tab_strip_ui_handler.cc

@@ -37,6 +37,7 @@
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/geometry/point_conversions.h"
+#include "ui/gfx/geometry/rect.h"
 
 namespace {
 
@@ -375,6 +376,10 @@ void TabStripUIHandler::RegisterMessages() {
       "showBackgroundContextMenu",
       base::Bind(&TabStripUIHandler::HandleShowBackgroundContextMenu,
                  base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "showEditDialogForGroup",
+      base::Bind(&TabStripUIHandler::HandleShowEditDialogForGroup,
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "showTabContextMenu",
       base::Bind(&TabStripUIHandler::HandleShowTabContextMenu,
@@ -692,6 +697,34 @@ void TabStripUIHandler::HandleShowBackgroundContextMenu(
           browser_, embedder_->GetAcceleratorProvider()));
 }
 
+void TabStripUIHandler::HandleShowEditDialogForGroup(
+    const base::ListValue* args) {
+  const std::string group_id_string = args->GetList()[0].GetString();
+  base::Optional<tab_groups::TabGroupId> group_id =
+      tab_strip_ui::GetTabGroupIdFromString(
+          browser_->tab_strip_model()->group_model(), group_id_string);
+  if (!group_id.has_value()) {
+    return;
+  }
+
+  gfx::Point point;
+  {
+    double x = args->GetList()[1].GetDouble();
+    double y = args->GetList()[2].GetDouble();
+    point = gfx::Point(x, y);
+  }
+
+  gfx::Rect rect;
+  {
+    double width = args->GetList()[3].GetDouble();
+    double height = args->GetList()[4].GetDouble();
+    rect = gfx::Rect(width, height);
+  }
+
+  DCHECK(embedder_);
+  embedder_->ShowEditDialogForGroupAtPoint(point, rect, group_id.value());
+}
+
 void TabStripUIHandler::HandleShowTabContextMenu(const base::ListValue* args) {
   int tab_id = args->GetList()[0].GetInt();
 

chrome/browser/ui/webui/tab_strip/tab_strip_ui_handler.h

@@ -67,6 +67,7 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
   void HandleGetThemeColors(const base::ListValue* args);
   void HandleCloseContainer(const base::ListValue* args);
   void HandleShowBackgroundContextMenu(const base::ListValue* args);
+  void HandleShowEditDialogForGroup(const base::ListValue* args);
   void HandleShowTabContextMenu(const base::ListValue* args);
   void HandleGetLayout(const base::ListValue* args);
   void HandleGroupTab(const base::ListValue* args);

chrome/browser/ui/webui/tab_strip/tab_strip_ui_handler_unittest.cc

@@ -47,6 +47,10 @@ class StubTabStripUIEmbedder : public TabStripUIEmbedder {
   void ShowContextMenuAtPoint(
       gfx::Point point,
       std::unique_ptr<ui::MenuModel> menu_model) override {}
+  void ShowEditDialogForGroupAtPoint(gfx::Point point,
+                                     gfx::Rect rect,
+                                     tab_groups::TabGroupId group_id) override {
+  }
   TabStripUILayout GetLayout() override { return TabStripUILayout(); }
   SkColor GetColor(int id) const override { return SK_ColorWHITE; }
 };

chrome/test/data/webui/tab_strip/tab_group_test.js

@@ -5,12 +5,22 @@
 import 'chrome://tab-strip/tab.js';
 import 'chrome://tab-strip/tab_group.js';
 
+import {TabStripEmbedderProxy} from 'chrome://tab-strip/tab_strip_embedder_proxy.js';
+import {TestTabStripEmbedderProxy} from './test_tab_strip_embedder_proxy.js';
+
 suite('TabGroup', () => {
+  const groupId = 'my-group-id';
+
   let tabGroupElement;
+  let testTabStripEmbedderProxy;
 
   setup(() => {
+    testTabStripEmbedderProxy = new TestTabStripEmbedderProxy();
+    TabStripEmbedderProxy.instance_ = testTabStripEmbedderProxy;
+
     document.body.innerHTML = '';
     tabGroupElement = document.createElement('tabstrip-tab-group');
+    tabGroupElement.dataset.groupId = groupId;
     tabGroupElement.appendChild(document.createElement('tabstrip-tab'));
     document.body.appendChild(tabGroupElement);
   });
@@ -55,4 +65,17 @@ suite('TabGroup', () => {
     assertEquals(originalChipRect.right, newChipRect.right);
     assertEquals(originalChipRect.bottom, newChipRect.bottom);
   });
+
+  test('ChipOpensEditDialog', async () => {
+    const chip = tabGroupElement.$('#chip');
+    const chipRect = chip.getBoundingClientRect();
+    chip.click();
+    const [calledGroupId, locationX, locationY, width, height] =
+        await testTabStripEmbedderProxy.whenCalled('showEditDialogForGroup');
+    assertEquals(groupId, calledGroupId);
+    assertEquals(chipRect.left, locationX);
+    assertEquals(chipRect.top, locationY);
+    assertEquals(chipRect.width, width);
+    assertEquals(chipRect.height, height);
+  });
 });

chrome/test/data/webui/tab_strip/test_tab_strip_embedder_proxy.js

@@ -13,6 +13,7 @@ export class TestTabStripEmbedderProxy extends TestBrowserProxy {
       'isVisible',
       'observeThemeChanges',
       'showBackgroundContextMenu',
+      'showEditDialogForGroup',
       'showTabContextMenu',
       'reportTabActivationDuration',
       'reportTabDataReceivedDuration',
@@ -64,6 +65,12 @@ export class TestTabStripEmbedderProxy extends TestBrowserProxy {
     this.methodCalled('showBackgroundContextMenu', [locationX, locationY]);
   }
 
+  showEditDialogForGroup(groupId, locationX, locationY, width, height) {
+    this.methodCalled(
+        'showEditDialogForGroup',
+        [groupId, locationX, locationY, width, height]);
+  }
+
   showTabContextMenu(tabId, locationX, locationY) {
     this.methodCalled('showTabContextMenu', [tabId, locationX, locationY]);
   }