Revert "Update QS bubble width and fix page indicator visibility"

This reverts commit c920e62.

Reason for revert: crbug.com/1411566

Original change's description:
> Update QS bubble width and fix page indicator visibility
>
> - Updated QS Revamp bubble and Feature Tile widths to spec.
> See go/cros-quick-settings-spec for details.
>
> - Fixed a padding issue by making the PageIndicatorView visible only
> when we have multiple pages.
>
> - Added page indicator visibility test in new quick_settings_unittest.cc
> file and moved some existing unified_system_tray tests to this file.
>
> - Added feature tiles container test to switch page with focus.
>
> screenshots
> single page: https://screenshot.googleplex.com/3hN2KssXLAZ6Q7H
> multiple pages: https://screenshot.googleplex.com/5JiibovN5NC6aHe
>
> b: 260119632, b:266601219, b:264469449, b:266627837
> Change-Id: I3600ce520291d9785e526542b79cc51d10f3a3f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195122
> Reviewed-by: Alex Newcomer <newcomer@chromium.org>
> Commit-Queue: Kevin Radtke <kradtke@chromium.org>
> Reviewed-by: Jiaming Cheng <jiamingc@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1099054}

Change-Id: I570a75434093c99abb12865c37f692cf049c883e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4207269
Auto-Submit: Keishi Hattori <keishi@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099090}

ash/BUILD.gn

@@ -3225,7 +3225,6 @@ test("ash_unittests") {
     "system/unified/power_button_unittest.cc",
     "system/unified/quick_settings_footer_unittest.cc",
     "system/unified/quick_settings_header_unittest.cc",
-    "system/unified/quick_settings_view_unittest.cc",
     "system/unified/quiet_mode_feature_pod_controller_unittest.cc",
     "system/unified/top_shortcuts_view_unittest.cc",
     "system/unified/unified_system_info_view_unittest.cc",

ash/system/tray/tray_constants.h

@@ -28,7 +28,7 @@ constexpr float kTrayItemCornerRadius = kTrayItemSize / 2.f;
 constexpr int kTrayMenuWidth = 360;
 
 // The width of the revamped tray menu.
-constexpr int kRevampedTrayMenuWidth = 400;
+constexpr int kRevampedTrayMenuWidth = 440;
 
 // TODO(b/258072559): Update this height once we have finalized UX specs for
 // tray height.

ash/system/unified/feature_tile.cc

@@ -31,13 +31,13 @@ constexpr int kButtonRadius = 16;
 
 // Primary tile constants
 constexpr int kPrimarySubtitleLineHeight = 18;
-constexpr gfx::Size kDefaultSize(180, kFeatureTileHeight);
+constexpr gfx::Size kDefaultSize(200, kFeatureTileHeight);
 constexpr gfx::Size kIconContainerSize(48, kFeatureTileHeight);
-constexpr gfx::Size kTitlesContainerSize(92, kFeatureTileHeight);
+constexpr gfx::Size kTitlesContainerSize(112, kFeatureTileHeight);
 constexpr gfx::Size kDrillContainerSize(40, kFeatureTileHeight);
 
 // Compact tile constants
-constexpr int kCompactWidth = 86;
+constexpr int kCompactWidth = 96;
 constexpr int kCompactTitleLineHeight = 14;
 constexpr gfx::Size kCompactSize(kCompactWidth, kFeatureTileHeight);
 constexpr gfx::Size kCompactIconContainerSize(kCompactWidth, 30);

ash/system/unified/feature_tiles_container_view.cc

@@ -97,7 +97,8 @@ FeatureTilesContainerView::FeatureTilesContainerView(
   DCHECK(controller_);
   pagination_model_->AddObserver(this);
   SetLayoutManager(std::make_unique<views::FlexLayout>())
-      ->SetOrientation(views::LayoutOrientation::kHorizontal);
+      ->SetOrientation(views::LayoutOrientation::kHorizontal)
+      .SetDefault(views::kMarginsKey, kRowContainerMargins);
 }
 
 FeatureTilesContainerView::~FeatureTilesContainerView() {
@@ -169,7 +170,6 @@ void FeatureTilesContainerView::RelayoutTiles() {
   // Re-add tiles to container.
   AddTiles(std::move(tiles));
 
-  // Update bubble height in case number of rows changed.
   controller_->UpdateBubble();
 }
 

ash/system/unified/feature_tiles_container_view.h

@@ -75,7 +75,6 @@ class ASH_EXPORT FeatureTilesContainerView : public views::View,
   class RowContainer;
 
   friend class FeatureTilesContainerViewTest;
-  friend class QuickSettingsViewTest;
 
   // Calculates the number of rows based on the available `height`.
   int CalculateRowsFromHeight(int height);

ash/system/unified/feature_tiles_container_view_unittest.cc

@@ -7,11 +7,11 @@
 #include "ash/constants/ash_features.h"
 #include "ash/constants/quick_settings_catalogs.h"
 #include "ash/public/cpp/pagination/pagination_model.h"
-#include "ash/shell.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/unified/feature_pod_button.h"
 #include "ash/system/unified/feature_pod_controller_base.h"
 #include "ash/system/unified/feature_tile.h"
+#include "ash/system/unified/page_indicator_view.h"
 #include "ash/system/unified/unified_system_tray.h"
 #include "ash/system/unified/unified_system_tray_bubble.h"
 #include "ash/system/unified/unified_system_tray_controller.h"
@@ -78,39 +78,35 @@ class FeatureTilesContainerViewTest : public AshTestBase,
   // AshTestBase:
   void SetUp() override {
     AshTestBase::SetUp();
-
-    tray_model_ =
-        base::MakeRefCounted<UnifiedSystemTrayModel>(/*shelf=*/nullptr);
-    tray_controller_ =
-        std::make_unique<UnifiedSystemTrayController>(tray_model_.get());
-    widget_ = CreateFramelessTestWidget();
-    widget_->SetFullscreen(true);
-
-    container_ = widget_->SetContentsView(
-        std::make_unique<FeatureTilesContainerView>(tray_controller_.get()));
+    GetPrimaryUnifiedSystemTray()->ShowBubble();
+    container_ = std::make_unique<FeatureTilesContainerView>(
+        GetPrimaryUnifiedSystemTray()
+            ->bubble()
+            ->unified_system_tray_controller());
     container_->AddObserver(this);
   }
 
   void TearDown() override {
     container_->RemoveObserver(this);
-    tray_controller_.reset();
-    tray_model_.reset();
-    widget_.reset();
-
+    container_.reset();
+    GetPrimaryUnifiedSystemTray()->CloseBubble();
     AshTestBase::TearDown();
   }
 
-  void PressTab() {
-    ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
-    generator.PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_NONE);
+  FeatureTilesContainerView* container() { return container_.get(); }
+
+  PageIndicatorView* GetPageIndicatorView() {
+    return GetPrimaryUnifiedSystemTray()
+        ->bubble()
+        ->quick_settings_view()
+        ->page_indicator_view_for_test();
   }
 
-  void PressShiftTab() {
-    ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
-    generator.PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_SHIFT_DOWN);
+  std::vector<views::View*> GetPageIndicatorButtons() {
+    return GetPageIndicatorView()->buttons_container()->children();
   }
 
-  FeatureTilesContainerView* container() { return container_; }
+  int GetPageIndicatorButtonCount() { return GetPageIndicatorButtons().size(); }
 
   PaginationModel* pagination_model() { return container()->pagination_model_; }
 
@@ -130,8 +126,6 @@ class FeatureTilesContainerViewTest : public AshTestBase,
 
   int GetPageCount() { return container()->page_count(); }
 
-  // Fills the container with a number of `pages` given the max amount of
-  // displayable primary tiles per page.
   void FillContainerWithPrimaryTiles(int pages) {
     auto mock_controller = std::make_unique<MockFeaturePodController>();
     std::vector<std::unique_ptr<FeatureTile>> tiles;
@@ -146,14 +140,12 @@ class FeatureTilesContainerViewTest : public AshTestBase,
 
     EXPECT_EQ(pages, GetPageCount());
     EXPECT_EQ(pages, pagination_model()->total_pages());
+    EXPECT_EQ(pages, GetPageIndicatorButtonCount());
   }
 
  private:
   base::test::ScopedFeatureList feature_list_;
-  std::unique_ptr<views::Widget> widget_;
-  std::unique_ptr<UnifiedSystemTrayController> tray_controller_;
-  scoped_refptr<UnifiedSystemTrayModel> tray_model_;
-  FeatureTilesContainerView* container_;
+  std::unique_ptr<FeatureTilesContainerView> container_;
 };
 
 // Tests `CalculateRowsFromHeight()` which returns the number of max displayable
@@ -417,20 +409,31 @@ TEST_F(FeatureTilesContainerViewTest, PaginationMouseWheel) {
   }
 }
 
-TEST_F(FeatureTilesContainerViewTest, SwitchPageWithFocus) {
-  FillContainerWithPrimaryTiles(/*pages=*/2);
-
-  // View starts at page with index zero.
-  EXPECT_EQ(0, pagination_model()->selected_page());
+TEST_F(FeatureTilesContainerViewTest, PaginationDots) {
+  constexpr int kNumberOfPages = 4;
+  FillContainerWithPrimaryTiles(kNumberOfPages);
 
-  // Tab until the `selected_page` index changes to 1.
-  while (pagination_model()->selected_page() == 0) {
-    PressTab();
+  // Expect the current_page to increase with each pagination dot click.
+  int current_page = pagination_model()->selected_page();
+  for (auto* button : GetPageIndicatorButtons()) {
+    LeftClickOn(button);
+    pagination_model()->FinishAnimation();
+    EXPECT_EQ(current_page++, pagination_model()->selected_page());
   }
-  EXPECT_EQ(1, pagination_model()->selected_page());
+}
+
+TEST_F(FeatureTilesContainerViewTest, ResetPagination) {
+  constexpr int kNumberOfPages = 4;
+  FillContainerWithPrimaryTiles(kNumberOfPages);
+
+  // Expect page with index 2 to be selected after clicking its dot.
+  LeftClickOn(GetPageIndicatorButtons()[2]);
+  pagination_model()->FinishAnimation();
+  EXPECT_EQ(2, pagination_model()->selected_page());
 
-  // Pressing shift tab returns to the previous page.
-  PressShiftTab();
+  // Expect page reset after closing and opening bubble.
+  GetPrimaryUnifiedSystemTray()->CloseBubble();
+  GetPrimaryUnifiedSystemTray()->ShowBubble();
   EXPECT_EQ(0, pagination_model()->selected_page());
 }
 

ash/system/unified/page_indicator_view.h

@@ -42,6 +42,7 @@ class ASH_EXPORT PageIndicatorView : public views::View,
 
  private:
   friend class PageIndicatorViewTest;
+  friend class FeatureTilesContainerViewTest;
 
   class PageIndicatorButton;
 

ash/system/unified/quick_settings_view.cc

@@ -120,7 +120,6 @@ QuickSettingsView::QuickSettingsView(UnifiedSystemTrayController* controller)
       interacted_by_tap_recorder_(
           std::make_unique<InteractedByTapRecorder>(this)) {
   DCHECK(controller_);
-  controller_->model()->pagination_model()->AddObserver(this);
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
@@ -139,9 +138,8 @@ QuickSettingsView::QuickSettingsView(UnifiedSystemTrayController* controller)
       std::make_unique<QuickSettingsHeader>(controller_));
   feature_tiles_container_ = system_tray_container_->AddChildView(
       std::make_unique<FeatureTilesContainerView>(controller_));
-  page_indicator_view_ =
-      system_tray_container_->AddChildView(std::make_unique<PageIndicatorView>(
-          controller_, /*initially_expanded=*/false));
+  page_indicator_view_ = system_tray_container_->AddChildView(
+      std::make_unique<PageIndicatorView>(controller_, true));
 
   if (base::FeatureList::IsEnabled(media::kGlobalMediaControlsForChromeOS)) {
     media_controls_container_ = system_tray_container_->AddChildView(
@@ -164,9 +162,7 @@ QuickSettingsView::QuickSettingsView(UnifiedSystemTrayController* controller)
       std::make_unique<AccessibilityFocusHelperView>(controller_));
 }
 
-QuickSettingsView::~QuickSettingsView() {
-  controller_->model()->pagination_model()->RemoveObserver(this);
-}
+QuickSettingsView::~QuickSettingsView() = default;
 
 void QuickSettingsView::SetMaxHeight(int max_height) {
   max_height_ = max_height;
@@ -269,11 +265,6 @@ bool QuickSettingsView::IsDetailedViewShown() const {
   return detailed_view_container_->GetVisible();
 }
 
-void QuickSettingsView::TotalPagesChanged(int previous_page_count,
-                                          int new_page_count) {
-  page_indicator_view_->SetVisible(new_page_count > 1);
-}
-
 void QuickSettingsView::OnGestureEvent(ui::GestureEvent* event) {
   if (event->type() == ui::ET_SCROLL_FLING_START) {
     controller_->Fling(event->details().velocity_y());

ash/system/unified/quick_settings_view.h

@@ -8,7 +8,6 @@
 #include <memory>
 
 #include "ash/ash_export.h"
-#include "ash/public/cpp/pagination/pagination_model_observer.h"
 #include "ash/system/brightness/unified_brightness_view.h"
 #include "ui/base/metadata/metadata_header_macros.h"
 #include "ui/views/view.h"
@@ -30,8 +29,7 @@ class UnifiedSystemTrayController;
 // View class of the bubble in status area tray.
 //
 // The `QuickSettingsView` contains the quick settings controls
-class ASH_EXPORT QuickSettingsView : public views::View,
-                                     public PaginationModelObserver {
+class ASH_EXPORT QuickSettingsView : public views::View {
  public:
   METADATA_HEADER(QuickSettingsView);
 
@@ -84,9 +82,6 @@ class ASH_EXPORT QuickSettingsView : public views::View,
   // Shows media controls view.
   void ShowMediaControls();
 
-  // PaginationModelObserver:
-  void TotalPagesChanged(int previous_page_count, int new_page_count) override;
-
   // views::View:
   void OnGestureEvent(ui::GestureEvent* event) override;