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}

ash/BUILD.gn

@@ -3225,6 +3225,7 @@ 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 = 440;
+constexpr int kRevampedTrayMenuWidth = 400;
 
 // 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(200, kFeatureTileHeight);
+constexpr gfx::Size kDefaultSize(180, kFeatureTileHeight);
 constexpr gfx::Size kIconContainerSize(48, kFeatureTileHeight);
-constexpr gfx::Size kTitlesContainerSize(112, kFeatureTileHeight);
+constexpr gfx::Size kTitlesContainerSize(92, kFeatureTileHeight);
 constexpr gfx::Size kDrillContainerSize(40, kFeatureTileHeight);
 
 // Compact tile constants
-constexpr int kCompactWidth = 96;
+constexpr int kCompactWidth = 86;
 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,8 +97,7 @@ FeatureTilesContainerView::FeatureTilesContainerView(
   DCHECK(controller_);
   pagination_model_->AddObserver(this);
   SetLayoutManager(std::make_unique<views::FlexLayout>())
-      ->SetOrientation(views::LayoutOrientation::kHorizontal)
-      .SetDefault(views::kMarginsKey, kRowContainerMargins);
+      ->SetOrientation(views::LayoutOrientation::kHorizontal);
 }
 
 FeatureTilesContainerView::~FeatureTilesContainerView() {
@@ -170,6 +169,7 @@ 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,6 +75,7 @@ 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,35 +78,39 @@ class FeatureTilesContainerViewTest : public AshTestBase,
   // AshTestBase:
   void SetUp() override {
     AshTestBase::SetUp();
-    GetPrimaryUnifiedSystemTray()->ShowBubble();
-    container_ = std::make_unique<FeatureTilesContainerView>(
-        GetPrimaryUnifiedSystemTray()
-            ->bubble()
-            ->unified_system_tray_controller());
+
+    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()));
     container_->AddObserver(this);
   }
 
   void TearDown() override {
     container_->RemoveObserver(this);
-    container_.reset();
-    GetPrimaryUnifiedSystemTray()->CloseBubble();
+    tray_controller_.reset();
+    tray_model_.reset();
+    widget_.reset();
+
     AshTestBase::TearDown();
   }
 
-  FeatureTilesContainerView* container() { return container_.get(); }
-
-  PageIndicatorView* GetPageIndicatorView() {
-    return GetPrimaryUnifiedSystemTray()
-        ->bubble()
-        ->quick_settings_view()
-        ->page_indicator_view_for_test();
+  void PressTab() {
+    ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
+    generator.PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_NONE);
   }
 
-  std::vector<views::View*> GetPageIndicatorButtons() {
-    return GetPageIndicatorView()->buttons_container()->children();
+  void PressShiftTab() {
+    ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
+    generator.PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_SHIFT_DOWN);
   }
 
-  int GetPageIndicatorButtonCount() { return GetPageIndicatorButtons().size(); }
+  FeatureTilesContainerView* container() { return container_; }
 
   PaginationModel* pagination_model() { return container()->pagination_model_; }
 
@@ -126,6 +130,8 @@ 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;
@@ -140,12 +146,14 @@ 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<FeatureTilesContainerView> container_;
+  std::unique_ptr<views::Widget> widget_;
+  std::unique_ptr<UnifiedSystemTrayController> tray_controller_;
+  scoped_refptr<UnifiedSystemTrayModel> tray_model_;
+  FeatureTilesContainerView* container_;
 };
 
 // Tests `CalculateRowsFromHeight()` which returns the number of max displayable
@@ -409,31 +417,20 @@ TEST_F(FeatureTilesContainerViewTest, PaginationMouseWheel) {
   }
 }
 
-TEST_F(FeatureTilesContainerViewTest, PaginationDots) {
-  constexpr int kNumberOfPages = 4;
-  FillContainerWithPrimaryTiles(kNumberOfPages);
-
-  // 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());
-  }
-}
+TEST_F(FeatureTilesContainerViewTest, SwitchPageWithFocus) {
+  FillContainerWithPrimaryTiles(/*pages=*/2);
 
-TEST_F(FeatureTilesContainerViewTest, ResetPagination) {
-  constexpr int kNumberOfPages = 4;
-  FillContainerWithPrimaryTiles(kNumberOfPages);
+  // View starts at page with index zero.
+  EXPECT_EQ(0, pagination_model()->selected_page());
 
-  // 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());
+  // Tab until the `selected_page` index changes to 1.
+  while (pagination_model()->selected_page() == 0) {
+    PressTab();
+  }
+  EXPECT_EQ(1, pagination_model()->selected_page());
 
-  // Expect page reset after closing and opening bubble.
-  GetPrimaryUnifiedSystemTray()->CloseBubble();
-  GetPrimaryUnifiedSystemTray()->ShowBubble();
+  // Pressing shift tab returns to the previous page.
+  PressShiftTab();
   EXPECT_EQ(0, pagination_model()->selected_page());
 }
 

ash/system/unified/page_indicator_view.h

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

ash/system/unified/quick_settings_view.cc

@@ -120,6 +120,7 @@ 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>());
 
@@ -138,8 +139,9 @@ 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_, true));
+  page_indicator_view_ =
+      system_tray_container_->AddChildView(std::make_unique<PageIndicatorView>(
+          controller_, /*initially_expanded=*/false));
 
   if (base::FeatureList::IsEnabled(media::kGlobalMediaControlsForChromeOS)) {
     media_controls_container_ = system_tray_container_->AddChildView(
@@ -162,7 +164,9 @@ QuickSettingsView::QuickSettingsView(UnifiedSystemTrayController* controller)
       std::make_unique<AccessibilityFocusHelperView>(controller_));
 }
 
-QuickSettingsView::~QuickSettingsView() = default;
+QuickSettingsView::~QuickSettingsView() {
+  controller_->model()->pagination_model()->RemoveObserver(this);
+}
 
 void QuickSettingsView::SetMaxHeight(int max_height) {
   max_height_ = max_height;
@@ -265,6 +269,11 @@ 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,6 +8,7 @@
 #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"
@@ -29,7 +30,8 @@ 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 {
+class ASH_EXPORT QuickSettingsView : public views::View,
+                                     public PaginationModelObserver {
  public:
   METADATA_HEADER(QuickSettingsView);
 
@@ -82,6 +84,9 @@ 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;