Fix Focus Ring for UserAvatarButton

Add Empty Border around UserAvatarButton to create space for focus ring
Change focus ring shape to same shape as Button
Compensate for extra space needed in TopShortcutsView and FeaturePodsContainer

Bug: 922423
Test: Manual Test on PixelBook
Change-Id: Ib5c069e9cfb4619faaa34f474e1a83ef5b765417
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1542498
Auto-Submit: Ahmed Mehfooz <amehfooz@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Tim Song <tengs@chromium.org>
Commit-Queue: Ahmed Mehfooz <amehfooz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649752}

ash/system/tray/tray_constants.h

@@ -183,10 +183,11 @@ constexpr int kUnifiedTrayCornerRadius = 20;
 constexpr int kUnifiedTrayContentPadding = 8;
 constexpr int kUnifiedTopShortcutSpacing = 16;
 constexpr int kUnifiedNotificationHiddenLineHeight = 20;
+constexpr int kUnifiedTopShortcutContainerTopPadding = 12;
 constexpr int kUnifiedNotificationMinimumHeight = 40;
 constexpr gfx::Insets kUnifiedTopShortcutPadding(0, 16);
 constexpr gfx::Insets kUnifiedNotificationHiddenPadding(6, 16);
-
+constexpr gfx::Insets kUnifiedCircularButtonFocusPadding(4);
 constexpr int kStackingNotificationCounterMax = 8;
 constexpr int kStackingNotificationCounterRadius = 2;
 constexpr int kStackingNotificationCounterStartX = 18;
@@ -220,6 +221,8 @@ constexpr int kUnifiedFeaturePodLabelWidth = 80;
 constexpr int kUnifiedFeaturePodSpacing = 6;
 constexpr int kUnifiedFeaturePodHoverRadius = 4;
 constexpr int kUnifiedFeaturePodVerticalPadding = 28;
+constexpr int kUnifiedFeaturePodTopPadding = 24;
+constexpr int kUnifiedFeaturePodBottomPadding = 24;
 constexpr int kUnifiedFeaturePodHorizontalSidePadding = 12;
 constexpr int kUnifiedFeaturePodHorizontalMiddlePadding = 0;
 constexpr int kUnifiedFeaturePodCollapsedVerticalPadding = 16;

ash/system/unified/feature_pods_container_view.cc

@@ -39,7 +39,7 @@ int FeaturePodsContainerView::GetExpandedHeight() const {
   // floor(visible_count / kUnifiedFeaturePodItemsInRow)
   int number_of_lines = (visible_count + kUnifiedFeaturePodItemsInRow - 1) /
                         kUnifiedFeaturePodItemsInRow;
-  return kUnifiedFeaturePodVerticalPadding +
+  return kUnifiedFeaturePodBottomPadding +
          (kUnifiedFeaturePodVerticalPadding + kUnifiedFeaturePodSize.height()) *
              number_of_lines;
 }
@@ -145,10 +145,9 @@ gfx::Point FeaturePodsContainerView::GetButtonPosition(
           (kUnifiedFeaturePodSize.width() +
            kUnifiedFeaturePodHorizontalMiddlePadding) *
               column;
-  int y =
-      kUnifiedFeaturePodVerticalPadding +
-      (kUnifiedFeaturePodSize.height() + kUnifiedFeaturePodVerticalPadding) *
-          row;
+  int y = kUnifiedFeaturePodTopPadding + (kUnifiedFeaturePodSize.height() +
+                                          kUnifiedFeaturePodVerticalPadding) *
+                                             row;
 
   // When fully expanded, or below the second row, always return the same
   // position.

ash/system/unified/top_shortcuts_view.cc

@@ -21,7 +21,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
-
+#include "ui/views/view_class_properties.h"
 namespace ash {
 
 namespace {
@@ -38,11 +38,19 @@ class UserAvatarButton : public views::Button {
 UserAvatarButton::UserAvatarButton(views::ButtonListener* listener)
     : Button(listener) {
   SetLayoutManager(std::make_unique<views::FillLayout>());
+  SetBorder(views::CreateEmptyBorder(kUnifiedCircularButtonFocusPadding));
   AddChildView(CreateUserAvatarView(0 /* user_index */));
 
   SetTooltipText(GetUserItemAccessibleString(0 /* user_index */));
-  SetFocusPainter(TrayPopupUtils::CreateFocusPainter());
+  SetInstallFocusRingOnFocus(true);
   SetFocusForPlatform();
+
+  int focus_ring_radius =
+      kTrayItemSize + kUnifiedCircularButtonFocusPadding.width();
+  auto path = std::make_unique<SkPath>();
+  path->addOval(gfx::RectToSkRect(
+      gfx::Rect(gfx::Size(focus_ring_radius, focus_ring_radius))));
+  SetProperty(views::kHighlightPathKey, path.release());
 }
 
 }  // namespace
@@ -89,15 +97,36 @@ void TopShortcutButtonContainer::Layout() {
   }
 
   int horizontal_position = child_area.x();
-  for (int i = 0; i < child_count(); i++) {
+
+  if (user_avatar_button_ && user_avatar_button_->visible()) {
+    int vertical_position =
+        child_area.y() + kUnifiedTopShortcutContainerTopPadding;
+    horizontal_position -= kUnifiedCircularButtonFocusPadding.left();
+
+    gfx::Size size = user_avatar_button_->GetPreferredSize();
+    gfx::Rect user_avatar_bounds(horizontal_position, vertical_position,
+                                 size.width(), size.height());
+    user_avatar_button_->SetBoundsRect(user_avatar_bounds);
+
+    horizontal_position += user_avatar_bounds.size().width() + spacing -
+                           kUnifiedCircularButtonFocusPadding.right();
+  }
+
+  int vertical_position = child_area.y() +
+                          kUnifiedTopShortcutContainerTopPadding +
+                          kUnifiedCircularButtonFocusPadding.bottom();
+  for (int i = 1; i < child_count(); i++) {
     views::View* child = child_at(i);
     if (!child->visible())
       continue;
     gfx::Rect bounds(child_area);
     bounds.set_x(horizontal_position);
+    bounds.set_y(vertical_position);
+
     int width = (child == sign_out_button_) ? sign_out_button_width
                                             : child->GetPreferredSize().width();
     bounds.set_width(width);
+    bounds.set_height(child->GetHeightForWidth(width));
     child->SetBoundsRect(bounds);
     horizontal_position += width + spacing;
   }
@@ -105,7 +134,6 @@ void TopShortcutButtonContainer::Layout() {
 
 gfx::Size TopShortcutButtonContainer::CalculatePreferredSize() const {
   int total_horizontal_size = 0;
-  int max_height = 0;
   int num_visible = 0;
   for (int i = 0; i < child_count(); i++) {
     const views::View* child = child_at(i);
@@ -116,14 +144,21 @@ gfx::Size TopShortcutButtonContainer::CalculatePreferredSize() const {
       continue;
     total_horizontal_size += child_horizontal_size;
     num_visible++;
-    max_height = std::max(child->GetPreferredSize().height(), max_height);
   }
   int width =
       (num_visible == 0)
           ? 0
           : total_horizontal_size +
                 (num_visible - 1) * kUnifiedTopShortcutButtonDefaultSpacing;
-  return gfx::Size(width, max_height);
+  int height = kTrayItemSize + kUnifiedCircularButtonFocusPadding.height() +
+               kUnifiedTopShortcutContainerTopPadding;
+  return gfx::Size(width, height);
+}
+
+void TopShortcutButtonContainer::AddUserAvatarButton(
+    views::View* user_avatar_button) {
+  AddChildView(user_avatar_button);
+  user_avatar_button_ = user_avatar_button;
 }
 
 void TopShortcutButtonContainer::AddSignOutButton(
@@ -139,7 +174,8 @@ TopShortcutsView::TopShortcutsView(UnifiedSystemTrayController* controller)
   auto* layout = SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::kHorizontal, kUnifiedTopShortcutPadding,
       kUnifiedTopShortcutSpacing));
-  layout->set_cross_axis_alignment(views::BoxLayout::CROSS_AXIS_ALIGNMENT_END);
+  layout->set_cross_axis_alignment(
+      views::BoxLayout::CROSS_AXIS_ALIGNMENT_START);
   container_ = new TopShortcutButtonContainer();
   AddChildView(container_);
 
@@ -147,7 +183,7 @@ TopShortcutsView::TopShortcutsView(UnifiedSystemTrayController* controller)
       LoginStatus::NOT_LOGGED_IN) {
     user_avatar_button_ = new UserAvatarButton(this);
     user_avatar_button_->SetEnabled(controller->IsUserChooserEnabled());
-    container_->AddChildView(user_avatar_button_);
+    container_->AddUserAvatarButton(user_avatar_button_);
   }
 
   // Show the buttons in this row as disabled if the user is at the login

ash/system/unified/top_shortcuts_view.h

@@ -30,10 +30,12 @@ class TopShortcutButtonContainer : public views::View {
   void Layout() override;
   gfx::Size CalculatePreferredSize() const override;
 
+  void AddUserAvatarButton(views::View* user_avatar_button);
   // Add the sign-out button, which can be resized upon layout.
   void AddSignOutButton(views::View* sign_out_button);
 
  private:
+  views::View* user_avatar_button_ = nullptr;
   views::View* sign_out_button_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(TopShortcutButtonContainer);

ash/system/unified/user_chooser_view.cc

@@ -149,6 +149,9 @@ views::View* CreateUserAvatarView(int user_index) {
     gfx::ImageSkia icon =
         gfx::CreateVectorIcon(kSystemMenuGuestIcon, kMenuIconColor);
     image_view->SetImage(icon, icon.size());
+    // make sure icon height stays same for guest icon
+    image_view->SetBorder(views::CreateEmptyBorder(
+        gfx::Insets((kTrayItemSize - icon.size().height()) / 2, 0)));
   } else {
     image_view->SetImage(user_session->user_info->avatar->image,
                          gfx::Size(kTrayItemSize, kTrayItemSize));