[Tab Groups] Visual update for the tab group editor bubble.

Bug: 1015634
Change-Id: I48e2c78071619827843f45c74852c0a127f75c59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879649
Commit-Queue: Charlene Yan <cyan@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710538}

chrome/browser/ui/views/chrome_layout_provider.cc

@@ -17,6 +17,7 @@ namespace {
 // respected (there's 3 * unit / 4 in use to express 12).
 // The Harmony layout unit. All distances are in terms of this unit.
 constexpr int kHarmonyLayoutUnit = 16;
+constexpr int kExtraSmallBubbleSize = 240;
 constexpr int kSmallSnapPoint = 320;
 constexpr int kMediumSnapPoint = 448;
 constexpr int kLargeSnapPoint = 512;
@@ -160,6 +161,8 @@ int ChromeLayoutProvider::GetDistanceMetric(int metric) const {
       return kHarmonyLayoutUnit;
     case DISTANCE_UNRELATED_CONTROL_VERTICAL_LARGE:
       return kHarmonyLayoutUnit;
+    case DISTANCE_BUBBLE_TABSTRIP_PREFERRED_WIDTH:
+      return kExtraSmallBubbleSize;
     case DISTANCE_BUBBLE_PREFERRED_WIDTH:
       return kSmallSnapPoint;
     case DISTANCE_MODAL_DIALOG_PREFERRED_WIDTH:

chrome/browser/ui/views/chrome_layout_provider.h

@@ -69,6 +69,8 @@ enum ChromeDistanceMetric {
   DISTANCE_MODAL_DIALOG_PREFERRED_WIDTH,
   // Width of larger modal dialogs that require extra width.
   DISTANCE_LARGE_MODAL_DIALOG_PREFERRED_WIDTH,
+  // Width of a bubble attached to the tabstrip.
+  DISTANCE_BUBBLE_TABSTRIP_PREFERRED_WIDTH,
   // Width of a bubble unless the content is too wide to make that
   // feasible.
   DISTANCE_BUBBLE_PREFERRED_WIDTH,

chrome/browser/ui/views/tabs/color_picker_view.cc

@@ -15,6 +15,7 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
+#include "ui/gfx/favicon_size.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/highlight_path_generator.h"
@@ -108,7 +109,7 @@ class ColorPickerElementView : public views::Button,
 
   gfx::Size CalculatePreferredSize() const override {
     const gfx::Insets insets = GetInsets();
-    gfx::Size size(24, 24);
+    gfx::Size size(gfx::kFaviconSize, gfx::kFaviconSize);
     size.Enlarge(insets.width(), insets.height());
     return size;
   }

chrome/browser/ui/views/tabs/tab_group_editor_bubble_view.cc

@@ -70,16 +70,10 @@ views::Widget* TabGroupEditorBubbleView::Show(TabGroupHeader* anchor_view,
 
 gfx::Size TabGroupEditorBubbleView::CalculatePreferredSize() const {
   const int width = ChromeLayoutProvider::Get()->GetDistanceMetric(
-                        DISTANCE_BUBBLE_PREFERRED_WIDTH) -
-                    margins().width();
+      DISTANCE_BUBBLE_TABSTRIP_PREFERRED_WIDTH);
   return gfx::Size(width, GetHeightForWidth(width));
 }
 
-base::string16 TabGroupEditorBubbleView::GetWindowTitle() const {
-  // TODO(crbug.com/989174): remove this and other hardcoded ASCII strings.
-  return base::ASCIIToUTF16("Customize tab group visuals");
-}
-
 ui::ModalType TabGroupEditorBubbleView::GetModalType() const {
   return ui::MODAL_TYPE_NONE;
 }
@@ -97,50 +91,33 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
       title_field_controller_(this),
       menu_button_listener_(tab_controller, anchor_view, group) {
   SetAnchorView(anchor_view);
-
-  // Title insets assume there is content (and thus have no bottom padding). Use
-  // dialog insets to get the bottom margin back.
-  set_title_margins(
-      ChromeLayoutProvider::Get()->GetInsetsMetric(views::INSETS_DIALOG));
   set_margins(gfx::Insets());
 
   const auto* layout_provider = ChromeLayoutProvider::Get();
   const TabGroupVisualData* current_data =
       tab_controller_->GetVisualDataForGroup(group_);
-
-  // Layout vertically with margin collapsing. This allows us to use spacer
-  // views with |DISTANCE_UNRELATED_CONTROL_VERTICAL| margins without worrying
-  // about the default |DISTANCE_RELATED_CONTROL_VERTICAL| spacing.
-  auto container_layout = std::make_unique<views::BoxLayout>(
-      views::BoxLayout::Orientation::kVertical, gfx::Insets(),
-      layout_provider->GetDistanceMetric(
-          views::DISTANCE_RELATED_CONTROL_VERTICAL),
-      true /* collapse_margins_spacing */);
-  container_layout->set_cross_axis_alignment(
-      views::BoxLayout::CrossAxisAlignment::kStretch);
+  const int horizontal_spacing = layout_provider->GetDistanceMetric(
+      views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
+  const int vertical_spacing = layout_provider->GetDistanceMetric(
+      views::DISTANCE_RELATED_CONTROL_VERTICAL);
 
   views::View* group_modifier_container =
       AddChildView(std::make_unique<views::View>());
   group_modifier_container->SetBorder(
-      views::CreateEmptyBorder(gfx::Insets(layout_provider->GetDistanceMetric(
-          views::DISTANCE_RELATED_CONTROL_VERTICAL))));
-  group_modifier_container->SetLayoutManager(std::move(container_layout));
+      views::CreateEmptyBorder(gfx::Insets(horizontal_spacing)));
+  views::FlexLayout* container_layout =
+      group_modifier_container->SetLayoutManager(
+          std::make_unique<views::FlexLayout>());
+  container_layout->SetOrientation(views::LayoutOrientation::kVertical)
+      .SetIgnoreDefaultMainAxisMargins(true);
 
   // Add the text field for editing the title.
   title_field_ = group_modifier_container->AddChildView(
       std::make_unique<views::Textfield>());
-  title_field_->SetDefaultWidthInChars(15);
   title_field_->SetText(current_data->title());
   title_field_->SetAccessibleName(base::ASCIIToUTF16("Group title"));
   title_field_->set_controller(&title_field_controller_);
 
-  title_field_->SetProperty(
-      views::kMarginsKey,
-      gfx::Insets(0, 0,
-                  layout_provider->GetDistanceMetric(
-                      views::DISTANCE_UNRELATED_CONTROL_VERTICAL),
-                  0));
-
   color_selector_ =
       group_modifier_container->AddChildView(std::make_unique<ColorPickerView>(
           GetColorPickerList(),
@@ -149,28 +126,44 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
 
   AddChildView(std::make_unique<views::Separator>());
 
+  views::View* menu_items_container =
+      AddChildView(std::make_unique<views::View>());
+  menu_items_container->SetBorder(
+      views::CreateEmptyBorder(gfx::Insets(vertical_spacing, 0)));
+  views::FlexLayout* layout_manager_ = menu_items_container->SetLayoutManager(
+      std::make_unique<views::FlexLayout>());
+  layout_manager_->SetOrientation(views::LayoutOrientation::kVertical)
+      .SetIgnoreDefaultMainAxisMargins(true);
+
+  gfx::Insets menu_item_border_inset =
+      gfx::Insets(vertical_spacing, horizontal_spacing);
+
   std::unique_ptr<views::LabelButton> new_tab_menu_item = CreateBubbleMenuItem(
       TAB_GROUP_HEADER_CXMENU_NEW_TAB_IN_GROUP,
       l10n_util::GetStringUTF16(IDS_TAB_GROUP_HEADER_CXMENU_NEW_TAB_IN_GROUP),
       &menu_button_listener_);
-  AddChildView(std::move(new_tab_menu_item));
+  new_tab_menu_item->SetBorder(
+      views::CreateEmptyBorder(menu_item_border_inset));
+  menu_items_container->AddChildView(std::move(new_tab_menu_item));
 
   std::unique_ptr<views::LabelButton> ungroup_menu_item = CreateBubbleMenuItem(
       TAB_GROUP_HEADER_CXMENU_UNGROUP,
       l10n_util::GetStringUTF16(IDS_TAB_GROUP_HEADER_CXMENU_UNGROUP),
       &menu_button_listener_);
-  AddChildView(std::move(ungroup_menu_item));
+  ungroup_menu_item->SetBorder(
+      views::CreateEmptyBorder(menu_item_border_inset));
+  menu_items_container->AddChildView(std::move(ungroup_menu_item));
 
   std::unique_ptr<views::LabelButton> close_menu_item = CreateBubbleMenuItem(
       TAB_GROUP_HEADER_CXMENU_CLOSE_GROUP,
       l10n_util::GetStringUTF16(IDS_TAB_GROUP_HEADER_CXMENU_CLOSE_GROUP),
       &menu_button_listener_);
-  AddChildView(std::move(close_menu_item));
+  close_menu_item->SetBorder(views::CreateEmptyBorder(menu_item_border_inset));
+  menu_items_container->AddChildView(std::move(close_menu_item));
 
-  views::FlexLayout* layout_manager_ =
+  views::FlexLayout* menu_layout_manager_ =
       SetLayoutManager(std::make_unique<views::FlexLayout>());
-  layout_manager_->SetOrientation(views::LayoutOrientation::kVertical)
-      .SetIgnoreDefaultMainAxisMargins(true);
+  menu_layout_manager_->SetOrientation(views::LayoutOrientation::kVertical);
 }
 
 TabGroupEditorBubbleView::~TabGroupEditorBubbleView() = default;

chrome/browser/ui/views/tabs/tab_group_editor_bubble_view.h

@@ -22,9 +22,6 @@ class Size;
 class ColorPickerView;
 
 // A dialog for changing a tab group's visual parameters.
-//
-// TODO(crbug.com/989174): polish this UI. It is currently sufficient for
-// testing, but it is not ready to be launched.
 class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
  public:
   // Shows the editor for |group|. Returns an *unowned* pointer to the
@@ -35,7 +32,6 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
 
   // views::BubbleDialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;
-  base::string16 GetWindowTitle() const override;
   ui::ModalType GetModalType() const override;
   int GetDialogButtons() const override;