Update apps grid horizontal margins calculation

Updates how apps grid horizontal margins are calculated with
kScalableAppList feature enabled.

Adds some tests that verify the apps grid layout in fullscreen state.

Bug: 999273
Change-Id: I63f36f6b946afc7b55b66568a8113bf19ce381b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776715
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692214}

ash/app_list/views/app_list_view_unittest.cc

@@ -25,6 +25,7 @@
 #include "ash/app_list/views/contents_view.h"
 #include "ash/app_list/views/expand_arrow_view.h"
 #include "ash/app_list/views/folder_header_view.h"
+#include "ash/app_list/views/page_switcher.h"
 #include "ash/app_list/views/result_selection_controller.h"
 #include "ash/app_list/views/search_box_view.h"
 #include "ash/app_list/views/search_result_answer_card_view.h"
@@ -70,6 +71,30 @@ namespace {
 
 constexpr int kInitialItems = 34;
 
+// Constants used for for testing app list layout in fullscreen state:
+// The expected bottom of suggestion chip view. The top is expected to be 96,
+// and height 32.
+constexpr int kFullscreenSuggestionChipBottom = 96 + 32;
+
+// The app list grid top inset - the height of the view fadeout area.
+constexpr int kGridTopInset = 24;
+
+// The horizontal spacing between apps grid view and the page switcher.
+constexpr int kPageSwitcherSpacing = 8;
+
+// The maximum allowed margin between items in apps item grid.
+constexpr int kMaxItemMargin = 96;
+
+int GridItemSizeWithMargins(int grid_size, int item_size, int item_count) {
+  int margin = (grid_size - item_size * item_count) / (2 * (item_count - 1));
+  return item_size + 2 * margin;
+}
+
+// Calculates the apps item grid size with maximum allowed margin between items.
+int GetItemGridSizeWithMaxItemMargins(int item_size, int item_count) {
+  return item_size * item_count + (item_count - 1) * kMaxItemMargin;
+}
+
 template <class T>
 size_t GetVisibleViews(const std::vector<T*>& tiles) {
   size_t count = 0;
@@ -216,6 +241,10 @@ class AppListViewTest : public views::ViewsTestBase,
     return contents_view()->GetAppsContainerView()->apps_grid_view();
   }
 
+  PageSwitcher* page_switcher_view() {
+    return contents_view()->GetAppsContainerView()->page_switcher();
+  }
+
   gfx::Point GetPointBetweenTwoApps() {
     const views::ViewModelT<AppListItemView>* view_model =
         apps_grid_view()->view_model();
@@ -230,6 +259,77 @@ class AppListViewTest : public views::ViewsTestBase,
     return delegate_->show_wallpaper_context_menu_count();
   }
 
+  // Verifies fullscreen apps grid bounds and layout with
+  // app_list_features::kScalableAppList feature enabled.
+  void VerifyAppsGridLayout(const gfx::Size& container_size,
+                            int row_count,
+                            int column_count,
+                            int expected_horizontal_margin,
+                            int expected_item_size,
+                            bool expect_max_grid_height) {
+    const int kShelfHeight = AppListConfig::instance().shelf_height();
+    // The apps grid height including insets and margins.
+    const int kExpectedTotalGridHeight = container_size.height() -
+                                         kFullscreenSuggestionChipBottom -
+                                         kShelfHeight;
+    const int kMaxGridHeight =
+        GetItemGridSizeWithMaxItemMargins(expected_item_size, column_count) +
+        2 * kGridTopInset;
+    const int kExpectedGridVerticalMargin =
+        expect_max_grid_height ? (kExpectedTotalGridHeight - kMaxGridHeight) / 2
+                               : kExpectedTotalGridHeight / 16 - kGridTopInset;
+    const int kExpectedGridHeight =
+        kExpectedTotalGridHeight - 2 * kExpectedGridVerticalMargin;
+    const int kExpectedGridTop =
+        kFullscreenSuggestionChipBottom + kExpectedGridVerticalMargin;
+    const int kExpectedGridWidth =
+        container_size.width() - 2 * expected_horizontal_margin;
+
+    EXPECT_EQ(
+        gfx::Rect(expected_horizontal_margin, kExpectedGridTop,
+                  kExpectedGridWidth,
+                  kExpectedTotalGridHeight - 2 * kExpectedGridVerticalMargin),
+        apps_grid_view()->bounds());
+    EXPECT_EQ(gfx::Rect(kExpectedGridWidth + expected_horizontal_margin +
+                            kPageSwitcherSpacing,
+                        kFullscreenSuggestionChipBottom,
+                        PageSwitcher::kPreferredButtonStripWidth,
+                        container_size.height() -
+                            kFullscreenSuggestionChipBottom - kShelfHeight),
+              page_switcher_view()->bounds());
+
+    // Horizontal offset between app list item views.
+    const int kHorizontalOffset = GridItemSizeWithMargins(
+        kExpectedGridWidth, expected_item_size, row_count);
+
+    // Verify expected bounds for the first row:
+    for (int i = 0; i < row_count; ++i) {
+      EXPECT_EQ(gfx::Rect(i * kHorizontalOffset, kGridTopInset,
+                          expected_item_size, expected_item_size),
+                test_api_->GetItemTileRectAtVisualIndex(0, i))
+          << "Item " << i << " bounds";
+    }
+
+    // Vertical offset between app list item views.
+    const int kVerticalOffset =
+        GridItemSizeWithMargins(kExpectedGridHeight - 2 * kGridTopInset,
+                                expected_item_size, column_count);
+
+    // Verify expected bounds for the first column:
+    for (int j = 1; j < column_count; ++j) {
+      EXPECT_EQ(gfx::Rect(0, kGridTopInset + j * kVerticalOffset,
+                          expected_item_size, expected_item_size),
+                test_api_->GetItemTileRectAtVisualIndex(0, j * row_count))
+          << "Item " << j * row_count << " bounds";
+    }
+
+    // The last item in the page (bottom right):
+    EXPECT_EQ(gfx::Rect((row_count - 1) * kHorizontalOffset,
+                        kGridTopInset + (column_count - 1) * kVerticalOffset,
+                        expected_item_size, expected_item_size),
+              test_api_->GetItemTileRectAtVisualIndex(0, 19));
+  }
+
   AppListView* view_ = nullptr;  // Owned by native widget.
   std::unique_ptr<AppListTestViewDelegate> delegate_;
   std::unique_ptr<AppsGridViewTestApi> test_api_;
@@ -2589,5 +2689,193 @@ TEST_F(AppListViewTest, ExpandArrowNotVisibleInEmbeddedAssistantUI) {
   EXPECT_TRUE(contents_view()->expand_arrow_view()->layer()->opacity() == 0.0f);
 }
 
+// Tests fullscreen apps grid sizing and layout for small screens (width < 960)
+// in landscape layout.
+TEST_F(AppListViewTest, AppListViewLayoutForSmallLandscapeScreen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(800, 600);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  VerifyAppsGridLayout(window_size, 5 /*row_count*/, 4 /*column_count*/,
+                       window_size.width() / 16 /*expected_horizontal_margin*/,
+                       80 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Tests fullscreen apps grid sizing and layout for small screens (width < 600)
+// in portrait layout.
+TEST_F(AppListViewTest, AppListViewLayoutForSmallPortraitScreen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(500, 800);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  VerifyAppsGridLayout(window_size, 4 /*row_count*/, 5 /*column_count*/,
+                       window_size.width() / 12 /*expected_horizontal_margin*/,
+                       80 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Tests fullscreen apps grid sizing and layout for medium sized screens
+// (width < 1200) in lanscape layout.
+TEST_F(AppListViewTest, AppListViewLayoutForMediumLandscapeScreen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(960, 800);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  // Horizontal margin should be set so apps grid doesn't go over the max size.
+  const int expected_horizontal_margin =
+      (window_size.width() - GetItemGridSizeWithMaxItemMargins(88, 5)) / 2;
+  VerifyAppsGridLayout(window_size, 5 /*row_count*/, 4 /*column_count*/,
+                       expected_horizontal_margin, 88 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Tests fullscreen apps grid sizing and layout for medium sized screens
+// (width < 768) in portrait layout.
+TEST_F(AppListViewTest, AppListViewLayoutForMediumPortraitScreen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(700, 800);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  VerifyAppsGridLayout(window_size, 4 /*row_count*/, 5 /*column_count*/,
+                       window_size.width() / 16 /*expected_horizontal_margin*/,
+                       88 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Tests fullscreen apps grid sizing and layout for large screens
+// (width >= 1200) in landscape layout.
+TEST_F(AppListViewTest, AppListViewLayoutForLargeLandscapeScreen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(1200, 960);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  // Horizontal margin should be set so apps grid doesn't go over the max size.
+  const int expected_horizontal_margin =
+      (window_size.width() - GetItemGridSizeWithMaxItemMargins(120, 5)) / 2;
+
+  VerifyAppsGridLayout(window_size, 5 /*row_count*/, 4 /*column_count*/,
+                       expected_horizontal_margin, 120 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Tests fullscreen apps grid sizing and layout for large screens (width >= 768)
+// in portrait layout.
+TEST_F(AppListViewTest, AppListViewLayoutForLargePortraitScreen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(800, 1200);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  VerifyAppsGridLayout(window_size, 4 /*row_count*/, 5 /*column_count*/,
+                       window_size.width() / 16 /*expected_horizontal_margin*/,
+                       120 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Tests that apps grid horizontal margin have minimum that ensures the page
+// switcher view can fit next to the apps grid.
+TEST_F(AppListViewTest, EnsurePageSwitcherFitsAppsGridMargin) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(600, 800);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  // The horizontal margin is selected so the page switcher fits the margin
+  // space (note that 600 / 16, which is how the margin is normally calculated
+  // is smaller than the width required by page switcher).
+  VerifyAppsGridLayout(window_size, 4 /*row_count*/, 5 /*column_count*/,
+                       40 /*expected_horizontal_margin*/,
+                       88 /*expected_item_size*/,
+                       false /*expect_max_grid_height*/);
+}
+
+// Verifies that the vertical spacing between items in apps grid has an upper
+// limit, and that the apps grid is centered in the available space if item
+// spacing hits that limit.
+TEST_F(AppListViewTest, VerticalAppsGridItemSpacingIsBounded) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures({app_list_features::kScalableAppList},
+                                       {});
+
+  const gfx::Size window_size = gfx::Size(960, 1600);
+  gfx::NativeView parent = GetContext();
+  parent->SetBounds(gfx::Rect(window_size));
+
+  Initialize(false /*is_tablet_mode*/);
+  delegate_->GetTestModel()->PopulateApps(kInitialItems);
+  Show();
+  view_->SetState(ash::AppListViewState::kFullscreenAllApps);
+
+  // Horizontal margin should be set so apps grid doesn't go over the max size.
+  const int expected_horizontal_margin =
+      (window_size.width() - GetItemGridSizeWithMaxItemMargins(120, 4)) / 2;
+
+  VerifyAppsGridLayout(window_size, 4 /*row_count*/, 5 /*column_count*/,
+                       expected_horizontal_margin, 120 /*expected_item_size*/,
+                       true /*expect_max_grid_height*/);
+}
+
 }  // namespace test
 }  // namespace app_list

ash/app_list/views/apps_container_view.cc

@@ -19,6 +19,7 @@
 #include "ash/app_list/views/search_box_view.h"
 #include "ash/app_list/views/suggestion_chip_container_view.h"
 #include "ash/public/cpp/app_list/app_list_config.h"
+#include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/app_list/app_list_switches.h"
 #include "base/command_line.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -44,6 +45,11 @@ constexpr int kSuggestionChipFullscreenY = 96;
 
 // The ratio of allowed bounds for apps grid view to its maximum margin.
 constexpr int kAppsGridMarginRatio = 16;
+constexpr int kAppsGridMarginRatioForSmallWidth = 12;
+
+// The width threshold under which kAppsGridMarginRatioForSmallWidth should be
+// used to calculate apps grid horizontal margins.
+constexpr int kAppsGridMarginSmallWidthThreshold = 600;
 
 // The minimum margin of apps grid view.
 constexpr int kAppsGridMinimumMargin = 8;
@@ -62,8 +68,15 @@ constexpr float kSuggestionChipOpacityEndProgress = 1;
 gfx::Size AppsContainerView::GetNonAppsGridSize() {
   gfx::Size size;
 
+  // If ScalableAppList feature is enabled, there is no extra horizontal margin
+  // between grid view and the page switcher.
+  const int min_grid_horizontal_margin =
+      app_list_features::IsScalableAppListEnabled()
+          ? 0
+          : kAppsGridMinimumMargin * 2;
+
   // Enlarge with the apps grid view insets and margin.
-  size.Enlarge(kAppsGridMinimumMargin * 2,
+  size.Enlarge(min_grid_horizontal_margin,
                AppsGridView::kFadeoutZoneHeight * 2);
 
   // Enlarge with suggestion chips.
@@ -269,8 +282,14 @@ void AppsContainerView::Layout() {
       rect.set_y(chip_container_rect.bottom());
       rect.set_height(rect.height() - kSuggestionChipFullscreenY -
                       kSuggestionChipContainerHeight);
+
       const int page_switcher_width =
           page_switcher_->GetPreferredSize().width();
+
+      // With scalable app list feature enabled, the margins are calculated from
+      // the edge of the apps container, instead of container bounds inset by
+      // page switcher area.
+      if (!app_list_features::IsScalableAppListEnabled())
         rect.Inset(kAppsGridPageSwitcherSpacing + page_switcher_width, 0);
 
       // Layout apps grid.
@@ -287,8 +306,13 @@ void AppsContainerView::Layout() {
       apps_grid_view_->SetLayout(cols, rows);
 
       // Calculate the maximum margin of apps grid.
+      const int horizontal_margin_ratio =
+          (app_list_features::IsScalableAppListEnabled() &&
+           rect.width() < kAppsGridMarginSmallWidthThreshold)
+              ? kAppsGridMarginRatioForSmallWidth
+              : kAppsGridMarginRatio;
       const int max_horizontal_margin =
-          grid_rect.width() / kAppsGridMarginRatio;
+          grid_rect.width() / horizontal_margin_ratio;
       const int max_vertical_margin = grid_rect.height() / kAppsGridMarginRatio;
 
       // Calculate the minimum size of apps grid.
@@ -299,22 +323,35 @@ void AppsContainerView::Layout() {
       // keep maximum margin if apps grid can maintain at least
       // |min_grid_size|; Otherwise, always keep at least
       // |kAppsGridMinimumMargin|.
-      const int horizontal_margin =
-          max_horizontal_margin * 2 <= grid_rect.width() - min_grid_size.width()
-              ? max_horizontal_margin
-              : std::max(kAppsGridMinimumMargin,
+      int horizontal_margin = max_horizontal_margin;
+      if (app_list_features::IsScalableAppListEnabled()) {
+        // For scalable app list margins, ensure the max margin allow space for
+        // the page switcher (note that this is larger than
+        // |kAppsGridMinimumMargin|).
+        horizontal_margin =
+            std::max(max_horizontal_margin,
+                     kAppsGridPageSwitcherSpacing + page_switcher_width);
+      } else if (max_horizontal_margin * 2 >
+                 grid_rect.width() - min_grid_size.width()) {
+        horizontal_margin =
+            std::max(kAppsGridMinimumMargin,
                      (grid_rect.width() - min_grid_size.width()) / 2);
-      const int vertical_margin =
-          max_vertical_margin * 2 <= grid_rect.height() - min_grid_size.height()
+      }
+
+      int vertical_margin =
+          (max_vertical_margin * 2 <=
+           grid_rect.height() - min_grid_size.height())
               ? max_vertical_margin
               : std::max(kAppsGridMinimumMargin,
                          (grid_rect.height() - min_grid_size.height()) / 2);
+
       grid_rect.Inset(
           horizontal_margin,
           std::max(apps_grid_view_->GetInsets().top(), vertical_margin));
       grid_rect.ClampToCenteredSize(
           apps_grid_view_->GetMaximumTileGridSize(cols, rows));
       grid_rect.Inset(-apps_grid_view_->GetInsets());
+
       apps_grid_view_->SetBoundsRect(grid_rect);
 
       // Record the distance of y position between suggestion chip container

ash/app_list/views/apps_container_view.h

@@ -102,6 +102,7 @@ class APP_LIST_EXPORT AppsContainerView : public HorizontalPage {
     return folder_background_view_;
   }
   AppListFolderView* app_list_folder_view() { return app_list_folder_view_; }
+  PageSwitcher* page_switcher() { return page_switcher_; }
 
   // Updates suggestion chips from app list model.
   void UpdateSuggestionChips();