bento_button: Make desk button use less space from shelf

This makes the desk button use preferred width, i.e. the exact width
needed for all UI elements for the button, including the padding,
between child spacing, the profile avatar icon, and the desk name label.

Bug: b/329122533
Test: ash_unittests
Change-Id: I6b3397e7eeb0f8d5c2cbe5c0ce9daa81b0906b3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5362416
Reviewed-by: Daniel Andersson <dandersson@chromium.org>
Commit-Queue: Yongshun Liu <yongshun@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1273241}

ash/shelf/desk_button_widget.cc

@@ -108,7 +108,8 @@ void DeskButtonWidget::DelegateView::Layout(PassKey) {
   }
 
   // Update the desk button container.
-  desk_button_container_->set_zero_state(desk_button_widget_->zero_state());
+  desk_button_container_->set_zero_state(
+      !desk_button_widget_->IsHorizontalShelf());
   desk_button_container_->UpdateUi(DesksController::Get()->active_desk());
 
   // Calculate bounds of the desk button container.
@@ -150,9 +151,9 @@ DeskButtonWidget::DeskButtonWidget(Shelf* shelf) : shelf_(shelf) {
 DeskButtonWidget::~DeskButtonWidget() = default;
 
 // static
-int DeskButtonWidget::GetMaxLength(bool horizontal_shelf, bool zero_state) {
+int DeskButtonWidget::GetMaxLength(bool horizontal_shelf) {
   const int container_len =
-      DeskButtonContainer::GetMaxLength(horizontal_shelf, zero_state);
+      DeskButtonContainer::GetMaxLength(!horizontal_shelf);
   return container_len + (horizontal_shelf
                               ? kDeskButtonWidgetInsetsHorizontal.width()
                               : kDeskButtonWidgetInsetsVertical.height());
@@ -193,7 +194,7 @@ void DeskButtonWidget::CalculateTargetBounds() {
       shelf_->hotseat_widget()
           ->scrollable_shelf_view()
           ->CalculateMirroredEdgePadding(/*use_target_bounds=*/true);
-  const int max_length = GetMaxLength(IsHorizontalShelf(), zero_state_);
+  const int max_length = GetMaxLength(IsHorizontalShelf());
 
   if (IsHorizontalShelf()) {
     widget_origin = gfx::Point(
@@ -309,10 +310,6 @@ bool DeskButtonWidget::IsHorizontalShelf() const {
   return shelf_->IsHorizontalAlignment();
 }
 
-bool DeskButtonWidget::IsForcedZeroState() const {
-  return delegate_view_->desk_button_container()->IsForcedZeroState();
-}
-
 void DeskButtonWidget::SetDefaultChildToFocus(
     views::View* default_child_to_focus) {
   CHECK(!default_child_to_focus || (default_child_to_focus->GetVisible() &&

ash/shelf/desk_button_widget.h

@@ -59,17 +59,13 @@ class ASH_EXPORT DeskButtonWidget : public ShelfComponent,
   DeskButtonWidget& operator=(const DeskButtonWidget&) = delete;
   ~DeskButtonWidget() override;
 
-  // Returns the max length for the widget assuming provided `horizontal_shelf`
-  // and `zero_state` values.
-  static int GetMaxLength(bool horizontal_shelf, bool zero_state);
+  // Returns the max length for the widget for the horizontal or vertical shelf.
+  static int GetMaxLength(bool horizontal_shelf);
 
   DelegateView* delegate_view() const { return delegate_view_; }
 
   Shelf* shelf() const { return shelf_; }
 
-  bool zero_state() const { return zero_state_; }
-  void set_zero_state(bool zero_state) { zero_state_ = zero_state; }
-
   // Indicates if the shelf should reserve some space for this widget.
   bool ShouldReserveSpaceFromShelf() const;
 
@@ -93,10 +89,9 @@ class ASH_EXPORT DeskButtonWidget : public ShelfComponent,
 
   DeskButtonContainer* GetDeskButtonContainer() const;
 
+  // Returns true if this widget belongs to a horizontal shelf.
   bool IsHorizontalShelf() const;
 
-  bool IsForcedZeroState() const;
-
   void SetDefaultChildToFocus(views::View* default_child_to_focus);
 
   // Stores the current focused view for desk button widget.
@@ -121,13 +116,6 @@ class ASH_EXPORT DeskButtonWidget : public ShelfComponent,
 
   raw_ptr<Shelf> const shelf_;
 
-  // Indicates if the widget is in zero state. It means the desk button UI takes
-  // less space from shelf. It can happen for any of the following conditions:
-  //   1) The display width is under a certain threshold, i.e.
-  //   `kDeskButtonLargeDisplayThreshold`.
-  //   2) The current shelf is overflown.
-  bool zero_state_ = false;
-
   // Default child view to focus when `OnNativeWidgetActivationChanged()`
   // occurs. When it's not null, it should point to the desk button, the
   // previous desk button, or the next desk button.

ash/shelf/scrollable_shelf_view_unittest.cc

@@ -1578,48 +1578,62 @@ class ScrollableShelfViewDeskButtonTest : public ScrollableShelfViewTest {
 
 }  // namespace
 
-// Verify that adding an app to overflow the shelf will cause the desk button to
-// shrink.
+// Verify that desk button behavior before and after shelf is overflown.
 TEST_F(ScrollableShelfViewDeskButtonTest, ButtonRespondsToOverflowStateChange) {
-  // Desk button will be forced to be zero state for display that is narrower
-  // than 1280.
-  UpdateDisplay("1280x720");
-
   SetShelfAnimationDuration(base::Milliseconds(1));
-  GetPrimaryShelf()->SetAlignment(ShelfAlignment::kBottom);
 
-  auto* scrollable_shelf_view =
-      GetPrimaryShelf()->hotseat_widget()->scrollable_shelf_view();
+  auto* shelf = GetPrimaryShelf();
+  auto* hotseat_widget = shelf->hotseat_widget();
+  auto* scrollable_shelf_view = hotseat_widget->scrollable_shelf_view();
+  auto* desk_button_widget = shelf->desk_button_widget();
+  shelf->SetAlignment(ShelfAlignment::kBottom);
+  EXPECT_FALSE(hotseat_widget->CalculateShelfOverflow(true));
   EXPECT_EQ(ScrollableShelfView::LayoutStrategy::kNotShowArrowButtons,
             scrollable_shelf_view->layout_strategy_for_test());
-  auto* desk_button_widget = GetPrimaryShelf()->desk_button_widget();
-  EXPECT_FALSE(desk_button_widget->zero_state());
-  EXPECT_EQ(218, desk_button_widget->GetTargetBounds().width());
 
-  // Keep adding apps until the desk button shrinks, and track the ID of the
-  // last added app so that we can remove it later.
-  // Set the upper limit on number of apps to avoid infinite loop if the desk
-  // button does not shrink.
+  // Keep adding apps until the shelf overflows. The desk button should remain
+  // expanded.
   ShelfID last_app_id;
-  size_t number_of_apps = 0u;
-  while (!desk_button_widget->zero_state()) {
+  gfx::Rect last_desk_button_bounds;
+  for (int i = 0; i < 50 && !hotseat_widget->CalculateShelfOverflow(true);
+       i++) {
+    last_desk_button_bounds = desk_button_widget->GetTargetBounds();
     last_app_id = AddAppShortcut();
     WaitForShelfAnimation();
-    ++number_of_apps;
-    ASSERT_LT(number_of_apps, 50u);
+    ASSERT_NE(last_desk_button_bounds, desk_button_widget->GetTargetBounds());
   }
-
-  EXPECT_EQ(ScrollableShelfView::LayoutStrategy::kNotShowArrowButtons,
+  EXPECT_TRUE(hotseat_widget->CalculateShelfOverflow(true));
+  EXPECT_EQ(ScrollableShelfView::LayoutStrategy::kShowRightArrowButton,
             scrollable_shelf_view->layout_strategy_for_test());
-  EXPECT_EQ(118, desk_button_widget->GetTargetBounds().width());
 
+  // Add one more app, desk button does not change its bounds. The desk button
+  // remains at the same bounds.
   auto* shelf_model = ShelfModel::Get();
+  last_desk_button_bounds = desk_button_widget->GetTargetBounds();
+  ShelfID new_app_id = AddAppShortcut();
+  WaitForShelfAnimation();
+  EXPECT_EQ(last_desk_button_bounds, desk_button_widget->GetTargetBounds());
+  EXPECT_TRUE(hotseat_widget->CalculateShelfOverflow(true));
+  EXPECT_EQ(ScrollableShelfView::LayoutStrategy::kShowRightArrowButton,
+            scrollable_shelf_view->layout_strategy_for_test());
+
+  // Remove the new app, desk button does not change its bounds. The desk button
+  // remains at the same bounds.
+  shelf_model->RemoveItemAt(shelf_model->ItemIndexByID(new_app_id));
+  WaitForShelfAnimation();
+  EXPECT_EQ(last_desk_button_bounds, desk_button_widget->GetTargetBounds());
+  EXPECT_TRUE(hotseat_widget->CalculateShelfOverflow(true));
+  EXPECT_EQ(ScrollableShelfView::LayoutStrategy::kShowRightArrowButton,
+            scrollable_shelf_view->layout_strategy_for_test());
+
+  // Remove the last app icon so that the shelf does not overflow. The desk
+  // button changes its bounds.
   shelf_model->RemoveItemAt(shelf_model->ItemIndexByID(last_app_id));
   WaitForShelfAnimation();
+  EXPECT_NE(last_desk_button_bounds, desk_button_widget->GetTargetBounds());
+  EXPECT_FALSE(hotseat_widget->CalculateShelfOverflow(true));
   EXPECT_EQ(ScrollableShelfView::LayoutStrategy::kNotShowArrowButtons,
             scrollable_shelf_view->layout_strategy_for_test());
-  EXPECT_FALSE(desk_button_widget->zero_state());
-  EXPECT_EQ(218, desk_button_widget->GetTargetBounds().width());
 }
 
 }  // namespace ash

ash/shelf/shelf_layout_manager.cc

@@ -1995,20 +1995,11 @@ void ShelfLayoutManager::UpdateWorkAreaInsetsAndNotifyObservers(
 }
 
 void ShelfLayoutManager::HandleScrollableShelfContainerBoundsChange() {
+  // Update desk button widget layout with animation.
   DeskButtonWidget* desk_button = shelf_widget_->desk_button_widget();
   if (desk_button && desk_button->ShouldReserveSpaceFromShelf()) {
-    // If desk button widget changes its state, update hotseat widget layout.
-    const bool old_zero_state = desk_button->zero_state();
     CalculateDeskButtonAndHotseatTargetBounds();
-    if (desk_button->zero_state() != old_zero_state) {
-      // Update hotseat widget layout with animation.
-      shelf_->hotseat_widget()->UpdateLayout(/*animate=*/true);
-      // Update desk button widget layout without animation.
-      shelf_->desk_button_widget()->UpdateLayout(/*animate=*/false);
-    } else {
-      // Update desk button widget layout with animation.
-      shelf_->desk_button_widget()->UpdateLayout(/*animate=*/true);
-    }
+    desk_button->UpdateLayout(/*animate=*/true);
   }
 }
 
@@ -3189,9 +3180,8 @@ void ShelfLayoutManager::CalculateDeskButtonAndHotseatTargetBounds() {
         shelf_->desk_button_widget()->ShouldReserveSpaceFromShelf());
 
   auto reserve_space_for_desk_button_widget = [](Shelf* shelf) {
-    const int length_needed = DeskButtonWidget::GetMaxLength(
-        shelf->IsHorizontalAlignment(),
-        shelf->desk_button_widget()->zero_state());
+    const int length_needed =
+        DeskButtonWidget::GetMaxLength(shelf->IsHorizontalAlignment());
     shelf->hotseat_widget()->ReserveSpaceForAdjacentWidgets(
         shelf->IsHorizontalAlignment()
             ? (base::i18n::IsRTL() ? gfx::Insets::TLBR(0, 0, 0, length_needed)
@@ -3199,27 +3189,10 @@ void ShelfLayoutManager::CalculateDeskButtonAndHotseatTargetBounds() {
             : gfx::Insets::TLBR(length_needed, 0, 0, 0));
   };
 
-  // First calculate target bounds of the hotseat widget.
-  if (shelf_->IsHorizontalAlignment() &&
-      !shelf_->desk_button_widget()->IsForcedZeroState()) {
-    // Try to reserve length for expanded desk button widget first.
-    shelf_->desk_button_widget()->set_zero_state(false);
-    reserve_space_for_desk_button_widget(shelf_);
-    shelf_->hotseat_widget()->CalculateTargetBounds();
-
-    // If hotseat overflows, reserve length for zero state desk button widget
-    // to save shelf space.
-    if (shelf_->hotseat_widget()->CalculateShelfOverflow(
-            /*use_target_bounds=*/true)) {
-      shelf_->desk_button_widget()->set_zero_state(true);
-      reserve_space_for_desk_button_widget(shelf_);
-      shelf_->hotseat_widget()->CalculateTargetBounds();
-    }
-  } else {
-    shelf_->desk_button_widget()->set_zero_state(true);
-    reserve_space_for_desk_button_widget(shelf_);
-    shelf_->hotseat_widget()->CalculateTargetBounds();
-  }
+  // First reserve space for the desk button widget and calculate target bounds
+  // of the hotseat widget.
+  reserve_space_for_desk_button_widget(shelf_);
+  shelf_->hotseat_widget()->CalculateTargetBounds();
 
   // Then calculate target bounds of the desk button widget.
   shelf_->desk_button_widget()->CalculateTargetBounds();

ash/shelf/shelf_view_unittest.cc

@@ -3900,24 +3900,6 @@ TEST_F(ShelfViewDeskButtonTest, TabletModeVisibility) {
   EXPECT_TRUE(desk_button_widget()->GetLayer()->GetTargetVisibility());
 }
 
-// Verify that the desk button is 218px wide if the screen width is greater than
-// or equal to 1280px, 118px if the screen width is less than 1280px. We also
-// test that the button is 48x50 in vertical alignment.
-TEST_F(ShelfViewDeskButtonTest, Position) {
-  SetShowDeskButtonInShelfPref(prefs_, true);
-  GetPrimaryShelf()->SetAlignment(ShelfAlignment::kBottom);
-  UpdateDisplay("1281x400");
-  EXPECT_EQ(218, desk_button_widget()->GetTargetBounds().width());
-  UpdateDisplay("200x1281");
-  EXPECT_EQ(118, desk_button_widget()->GetTargetBounds().width());
-  UpdateDisplay("1280x400");
-  EXPECT_EQ(218, desk_button_widget()->GetTargetBounds().width());
-
-  GetPrimaryShelf()->SetAlignment(ShelfAlignment::kLeft);
-  EXPECT_EQ(48, desk_button_widget()->GetTargetBounds().width());
-  EXPECT_EQ(50, desk_button_widget()->GetTargetBounds().height());
-}
-
 // Verify that the desk button does not appear by default, appears when the user
 // has more than 1 desk, and stays even if they go back to having just one desk.
 TEST_F(ShelfViewDeskButtonTest, NewDeskVisibility) {

ash/wm/desks/desk_button/desk_button.cc

@@ -63,20 +63,21 @@ DeskButton::DeskButton()
 DeskButton::~DeskButton() {}
 
 gfx::Size DeskButton::CalculatePreferredSize() const {
-  int height;
-  int width;
-  if (desk_button_container_->IsHorizontalShelf()) {
-    if (desk_button_container_->ShouldShowDeskProfilesUi()) {
-      width = zero_state_ ? kDeskButtonWidthHorizontalZeroWithAvatar
-                          : kDeskButtonWidthHorizontalExpandedWithAvatar;
-    } else {
-      width = zero_state_ ? kDeskButtonWidthHorizontalZeroNoAvatar
-                          : kDeskButtonWidthHorizontalExpandedNoAvatar;
-    }
-    height = kDeskButtonHeightHorizontal;
+  if (zero_state_) {
+    return {kDeskButtonWidthVertical, kDeskButtonHeightVertical};
+  }
+
+  int height = kDeskButtonHeightHorizontal;
+  int width =
+      GetButtonInsets().width() + desk_name_label_->GetPreferredSize().width();
+  if (desk_button_container_->ShouldShowDeskProfilesUi()) {
+    width += desk_avatar_view_->GetPreferredSize().width() +
+             kDeskButtonChildSpacingHorizontalExpanded;
+    width = std::clamp(width, kDeskButtonWidthHorizontalZeroWithAvatar,
+                       kDeskButtonWidthHorizontalExpandedWithAvatar);
   } else {
-    width = kDeskButtonWidthVertical;
-    height = kDeskButtonHeightVertical;
+    width = std::clamp(width, kDeskButtonWidthHorizontalZeroNoAvatar,
+                       kDeskButtonWidthHorizontalExpandedNoAvatar);
   }
 
   return {width, height};
@@ -89,21 +90,18 @@ void DeskButton::Layout(PassKey) {
 
   LayoutSuperclass<views::Button>(this);
 
-  gfx::Rect available_bounds = gfx::Rect(size());
-
   // Layout when it's vertical shelf.
-  if (!desk_button_container_->IsHorizontalShelf()) {
-    available_bounds.Inset(kDeskButtonInsectVerticalNoAvatar);
+  gfx::Rect available_bounds = gfx::Rect(size());
+  if (desk_button_container_->zero_state()) {
+    available_bounds.Inset(GetButtonInsets());
     desk_name_label_->SetBoundsRect(available_bounds);
     return;
   }
 
-  if (desk_avatar_view_ && desk_avatar_view_->GetVisible()) {
+  if (IsShowingAvatar()) {
     if (base::i18n::IsRTL()) {
       // Layout when the desk avatar is visible and it's rtl.
-      gfx::Insets desk_button_insets =
-          zero_state_ ? kDeskButtonInsectHorizontalZeroWithAvatar
-                      : kDeskButtonInsectHorizontalExpandedWithAvatar;
+      gfx::Insets desk_button_insets = GetButtonInsets();
       desk_button_insets.set_left_right(desk_button_insets.right(),
                                         desk_button_insets.left());
       available_bounds.Inset(desk_button_insets);
@@ -113,41 +111,34 @@ void DeskButton::Layout(PassKey) {
                (available_bounds.height() - kDeskButtonAvatarSize.height()) /
                    2},
           kDeskButtonAvatarSize);
-      gfx::Insets insets_taken = gfx::Insets::TLBR(
-          0, 0, 0,
-          kDeskButtonAvatarSize.width() +
-              (zero_state_ ? kDeskButtonChildSpacingHorizontalZero
-                           : kDeskButtonChildSpacingHorizontalExpanded));
+      gfx::Insets insets_taken =
+          gfx::Insets::TLBR(0, 0, 0,
+                            kDeskButtonAvatarSize.width() +
+                                kDeskButtonChildSpacingHorizontalExpanded);
       desk_avatar_view_->SetBoundsRect(desk_avatar_view_bounds);
       available_bounds.Inset(insets_taken);
       desk_name_label_->SetBoundsRect(available_bounds);
     } else {
       // Layout when the desk avatar is visible and it's *not* rtl.
-      gfx::Insets desk_button_insets =
-          zero_state_ ? kDeskButtonInsectHorizontalZeroWithAvatar
-                      : kDeskButtonInsectHorizontalExpandedWithAvatar;
-      available_bounds.Inset(desk_button_insets);
+      available_bounds.Inset(GetButtonInsets());
       gfx::Rect desk_avatar_view_bounds = gfx::Rect(
           {available_bounds.x(),
            available_bounds.y() +
                (available_bounds.height() - kDeskButtonAvatarSize.height()) /
                    2},
           kDeskButtonAvatarSize);
-      gfx::Insets insets_taken = gfx::Insets::TLBR(
-          0,
-          kDeskButtonAvatarSize.width() +
-              (zero_state_ ? kDeskButtonChildSpacingHorizontalZero
-                           : kDeskButtonChildSpacingHorizontalExpanded),
-          0, 0);
+      gfx::Insets insets_taken =
+          gfx::Insets::TLBR(0,
+                            kDeskButtonAvatarSize.width() +
+                                kDeskButtonChildSpacingHorizontalExpanded,
+                            0, 0);
       desk_avatar_view_->SetBoundsRect(desk_avatar_view_bounds);
       available_bounds.Inset(insets_taken);
       desk_name_label_->SetBoundsRect(available_bounds);
     }
   } else {
     // Layout when the desk avatar is *not* visible.
-    available_bounds.Inset(zero_state_
-                               ? kDeskButtonInsectHorizontalZeroNoAvatar
-                               : kDeskButtonInsectHorizontalExpandedNoAvatar);
+    available_bounds.Inset(GetButtonInsets());
     desk_name_label_->SetBoundsRect(available_bounds);
   }
 }
@@ -258,17 +249,33 @@ std::u16string DeskButton::GetTitle() const {
   return DesksController::Get()->active_desk()->name();
 }
 
+gfx::Insets DeskButton::GetButtonInsets() const {
+  if (desk_button_container_->zero_state()) {
+    return kDeskButtonInsectVerticalNoAvatar;
+  }
+
+  if (IsShowingAvatar()) {
+    return kDeskButtonInsectHorizontalExpandedWithAvatar;
+  }
+
+  return kDeskButtonInsectHorizontalExpandedNoAvatar;
+}
+
 void DeskButton::UpdateUi(const Desk* active_desk) {
   UpdateAvatar(active_desk);
   UpdateLocaleSpecificSettings();
 }
 
+bool DeskButton::IsShowingAvatar() const {
+  return desk_avatar_view_ && desk_avatar_view_->GetVisible();
+}
+
 void DeskButton::UpdateAvatar(const Desk* active_desk) {
   if (!desk_avatar_view_) {
     return;
   }
 
-  if (desk_button_container_->IsHorizontalShelf() &&
+  if (!desk_button_container_->zero_state() &&
       desk_button_container_->ShouldShowDeskProfilesUi()) {
     if (auto* desk_profiles_delegate =
             Shell::Get()->GetDeskProfilesDelegate()) {
@@ -295,7 +302,7 @@ void DeskButton::UpdateLocaleSpecificSettings() {
   // Update the accessible name.
   DesksController* desk_controller = DesksController::Get();
   const Desk* active_desk = desk_controller->active_desk();
-  if (desk_avatar_view_ && desk_avatar_view_->GetVisible()) {
+  if (IsShowingAvatar()) {
     SetAccessibleName(l10n_util::GetStringFUTF16(
         IDS_SHELF_DESK_BUTTON_TITLE_WITH_PROFILE_AVATAR, active_desk->name(),
         profile_.name, profile_.email,

ash/wm/desks/desk_button/desk_button.h

@@ -61,9 +61,15 @@ class ASH_EXPORT DeskButton : public views::Button {
 
   std::u16string GetTitle() const;
 
+  // Returns the button insets given current button state.
+  gfx::Insets GetButtonInsets() const;
+
   // Updates UI status without re-layout.
   void UpdateUi(const Desk* active_desk);
 
+  // Returns true if it is currently showing the desk profile avatar.
+  bool IsShowingAvatar() const;
+
   void UpdateAvatar(const Desk* active_desk);
 
   // Updates locale-specific settings.

ash/wm/desks/desk_button/desk_button_container.cc

@@ -39,16 +39,14 @@ bool DeskButtonContainer::ShouldShowDeskProfilesUi() {
 }
 
 // static
-int DeskButtonContainer::GetMaxLength(bool horizontal_shelf, bool zero_state) {
-  if (horizontal_shelf) {
-    if (ShouldShowDeskProfilesUi()) {
-      return zero_state ? kDeskButtonContainerWidthHorizontalZeroWithAvatar
-                        : kDeskButtonContainerWidthHorizontalExpandedWithAvatar;
-    }
-    return zero_state ? kDeskButtonContainerWidthHorizontalZeroNoAvatar
-                      : kDeskButtonContainerWidthHorizontalExpandedNoAvatar;
+int DeskButtonContainer::GetMaxLength(bool zero_state) {
+  if (zero_state) {
+    return kDeskButtonContainerHeightVertical;
   }
-  return kDeskButtonContainerHeightVertical;
+  if (ShouldShowDeskProfilesUi()) {
+    return kDeskButtonContainerWidthHorizontalExpandedWithAvatar;
+  }
+  return kDeskButtonContainerWidthHorizontalExpandedNoAvatar;
 }
 
 void DeskButtonContainer::OnProfileUpsert(const LacrosProfileSummary& summary) {
@@ -69,10 +67,10 @@ void DeskButtonContainer::OnFirstSessionStarted() {
 }
 
 gfx::Size DeskButtonContainer::CalculatePreferredSize() const {
-  if (IsHorizontalShelf()) {
-    return {GetPreferredLength(), kDeskButtonContainerHeightHorizontal};
+  if (zero_state_) {
+    return {kDeskButtonContainerWidthVertical, GetPreferredLength()};
   }
-  return {kDeskButtonContainerWidthVertical, GetPreferredLength()};
+  return {GetPreferredLength(), kDeskButtonContainerHeightHorizontal};
 }
 
 void DeskButtonContainer::Layout(PassKey) {
@@ -80,7 +78,12 @@ void DeskButtonContainer::Layout(PassKey) {
     return;
   }
 
-  if (IsHorizontalShelf()) {
+  if (zero_state_) {
+    desk_button_->SetBoundsRect(
+        gfx::Rect({kDeskButtonContainerInsetsVertical.left(),
+                   kDeskButtonContainerInsetsVertical.top()},
+                  desk_button_->GetPreferredSize()));
+  } else {
     auto get_spacing = [&](views::View* view1, views::View* view2) {
       if ((view1 == prev_desk_button_ && view2 == next_desk_button_) ||
           (view1 == next_desk_button_ && view2 == prev_desk_button_)) {
@@ -110,11 +113,6 @@ void DeskButtonContainer::Layout(PassKey) {
           gfx::Rect({x, y}, views_to_layout[i]->GetPreferredSize()));
       x += views_to_layout[i]->GetPreferredSize().width();
     }
-  } else {
-    desk_button_->SetBoundsRect(
-        gfx::Rect({kDeskButtonContainerInsetsVertical.left(),
-                   kDeskButtonContainerInsetsVertical.top()},
-                  desk_button_->GetPreferredSize()));
   }
 }
 
@@ -151,7 +149,10 @@ void DeskButtonContainer::PrepareForAlignmentChange() {
 int DeskButtonContainer::GetPreferredLength() const {
   int len = 0;
 
-  if (IsHorizontalShelf()) {
+  if (zero_state_) {
+    len += kDeskButtonContainerInsetsVertical.height() +
+           desk_button_->GetPreferredSize().height();
+  } else {
     len += kDeskButtonContainerInsetsHorizontal.left() +
            desk_button_->GetPreferredSize().width();
     if (prev_desk_button_->GetVisible() && next_desk_button_->GetVisible()) {
@@ -167,9 +168,6 @@ int DeskButtonContainer::GetPreferredLength() const {
              next_desk_button_->GetPreferredSize().width();
     }
     len += kDeskButtonContainerInsetsHorizontal.right();
-  } else {
-    len += kDeskButtonContainerInsetsVertical.height() +
-           desk_button_->GetPreferredSize().height();
   }
 
   return len;
@@ -200,14 +198,6 @@ std::u16string DeskButtonContainer::GetTitleForView(
   NOTREACHED_NORETURN();
 }
 
-bool DeskButtonContainer::IsHorizontalShelf() const {
-  return shelf_->IsHorizontalAlignment();
-}
-
-bool DeskButtonContainer::IsForcedZeroState() const {
-  return false;
-}
-
 void DeskButtonContainer::Init(DeskButtonWidget* desk_button_widget) {
   CHECK(desk_button_widget);
   desk_button_widget_ = desk_button_widget;
@@ -215,15 +205,12 @@ void DeskButtonContainer::Init(DeskButtonWidget* desk_button_widget) {
   shelf_ = desk_button_widget_->shelf();
   CHECK(shelf_);
 
+  zero_state_ = !shelf_->IsHorizontalAlignment();
+
   SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
   SetFlipCanvasOnPaintForRTLUI(false);
 
-  SetBackground(IsHorizontalShelf() ? views::CreateThemedRoundedRectBackground(
-                                          cros_tokens::kCrosSysSystemOnBase,
-                                          kDeskButtonContainerCornerRadius)
-                                    : nullptr);
-
   AddChildView(views::Builder<DeskButton>()
                    .CopyAddressTo(&desk_button_)
                    .Init(/*desk_button_container=*/this)
@@ -232,13 +219,13 @@ void DeskButtonContainer::Init(DeskButtonWidget* desk_button_widget) {
       views::Builder<DeskSwitchButton>()
           .CopyAddressTo(&prev_desk_button_)
           .Init(/*desk_button_container=*/this, DeskSwitchButton::Type::kPrev)
-          .SetVisible(IsHorizontalShelf())
+          .SetVisible(!zero_state_)
           .Build());
   AddChildView(
       views::Builder<DeskSwitchButton>()
           .CopyAddressTo(&next_desk_button_)
           .Init(/*desk_button_container=*/this, DeskSwitchButton::Type::kNext)
-          .SetVisible(IsHorizontalShelf())
+          .SetVisible(!zero_state_)
           .Build());
 
   desks_observation_.Observe(DesksController::Get());
@@ -246,6 +233,10 @@ void DeskButtonContainer::Init(DeskButtonWidget* desk_button_widget) {
 }
 
 void DeskButtonContainer::UpdateUi(const Desk* active_desk) {
+  SetBackground(zero_state_ ? nullptr
+                            : views::CreateThemedRoundedRectBackground(
+                                  cros_tokens::kCrosSysSystemOnBase,
+                                  kDeskButtonContainerCornerRadius));
   desk_button_->set_zero_state(zero_state_);
   desk_button_->UpdateUi(active_desk);
   prev_desk_button_->UpdateUi(active_desk);

ash/wm/desks/desk_button/desk_button_container.h

@@ -41,7 +41,7 @@ class ASH_EXPORT DeskButtonContainer : public DeskProfilesDelegate::Observer,
 
   static bool ShouldShowDeskProfilesUi();
 
-  static int GetMaxLength(bool horizontal_shelf, bool zero_state);
+  static int GetMaxLength(bool zero_state);
 
   bool zero_state() const { return zero_state_; }
   void set_zero_state(bool zero_state) { zero_state_ = zero_state; }
@@ -86,10 +86,6 @@ class ASH_EXPORT DeskButtonContainer : public DeskProfilesDelegate::Observer,
 
   std::u16string GetTitleForView(const views::View* view) const;
 
-  bool IsHorizontalShelf() const;
-
-  bool IsForcedZeroState() const;
-
   // Initializes the view. Must be called before any meaningful UIs can be laid
   // out.
   void Init(DeskButtonWidget* desk_button_widget);

ash/wm/desks/desk_button/desk_switch_button.cc

@@ -35,9 +35,9 @@ DeskSwitchButton::~DeskSwitchButton() = default;
 
 gfx::Size DeskSwitchButton::CalculatePreferredSize() const {
   return gfx::Size(kDeskButtonSwitchButtonWidth,
-                   desk_button_container_->IsHorizontalShelf()
-                       ? kDeskButtonSwitchButtonHeightHorizontal
-                       : kDeskButtonSwitchButtonHeightVertical);
+                   desk_button_container_->zero_state()
+                       ? kDeskButtonSwitchButtonHeightVertical
+                       : kDeskButtonSwitchButtonHeightHorizontal);
 }
 
 void DeskSwitchButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
@@ -125,11 +125,11 @@ void DeskSwitchButton::UpdateUi(const Desk* active_desk) {
   auto* desk_controller = DesksController::Get();
   int active_desk_index = desk_controller->GetDeskIndex(active_desk);
   if (type_ == Type::kPrev) {
-    SetVisible(desk_button_container_->IsHorizontalShelf() &&
+    SetVisible(!desk_button_container_->zero_state() &&
                active_desk_index - 1 >= 0);
   } else {
     const int desk_count = desk_controller->GetNumberOfDesks();
-    SetVisible(desk_count > 1 && desk_button_container_->IsHorizontalShelf());
+    SetVisible(desk_count > 1 && !desk_button_container_->zero_state());
     SetEnabled(active_desk_index + 1 < desk_count);
   }
 

ash/wm/desks/desks_unittests.cc

@@ -11274,14 +11274,14 @@ TEST_P(DeskButtonTest, ValidateDeskButtonPosition) {
         GetParam().alignment == ShelfAlignment::kBottom && i < desk_count - 1;
 
     // Check the desk button and both desk switch buttons.
-    EXPECT_EQ(desk_button->bounds(),
-              GetParam().alignment == ShelfAlignment::kBottom
-                  ? gfx::Rect(4, 4, 128, 28)
-                  : gfx::Rect(0, 0, 36, 36));
-    EXPECT_EQ(desk_name_label->bounds(),
-              GetParam().alignment == ShelfAlignment::kBottom
-                  ? gfx::Rect(12, 0, 104, 28)
-                  : gfx::Rect(4, 4, 28, 28));
+    if (GetParam().alignment == ShelfAlignment::kBottom) {
+      EXPECT_TRUE(gfx::Rect(4, 4, 128, 28).Contains(desk_button->bounds()));
+      EXPECT_TRUE(
+          gfx::Rect(12, 0, 104, 28).Contains(desk_name_label->bounds()));
+    } else {
+      EXPECT_EQ(desk_button->bounds(), gfx::Rect(0, 0, 36, 36));
+      EXPECT_EQ(desk_name_label->bounds(), gfx::Rect(4, 4, 28, 28));
+    }
     EXPECT_EQ(prev_desk_button->GetVisible(), should_show_prev_desk_button);
     if (prev_desk_button->GetVisible()) {
       EXPECT_TRUE(prev_desk_button->GetEnabled());
@@ -11346,7 +11346,7 @@ TEST_P(DeskButtonTest, LayoutInRTL) {
   switch (GetParam().alignment) {
     case ShelfAlignment::kBottom:
     case ShelfAlignment::kBottomLocked:
-      EXPECT_EQ(gfx::Rect(698, 682, 128, 28), desk_button_bounds);
+      EXPECT_TRUE(gfx::Rect(698, 682, 128, 28).Contains(desk_button_bounds));
       EXPECT_LT(app_icon_bounds.x(), desk_button_bounds.x());
       break;
     case ShelfAlignment::kLeft: