CrOS MD Menu - Some more user row tweaks.

ash/common/system/user/user_view.cc

Index: ash/common/system/user/user_view.cc
diff --git a/ash/common/system/user/user_view.cc b/ash/common/system/user/user_view.cc
index b2f17ddd5299e839c1d577643fdcfcfbd56b67c0..29c7d6ef359522aac6b0d46f8a80c4901513a77b 100644
--- a/ash/common/system/user/user_view.cc
+++ b/ash/common/system/user/user_view.cc
@@ -16,6 +16,7 @@
 #include "ash/common/system/tray/system_tray_controller.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/system/tray/tray_constants.h"
+#include "ash/common/system/tray/tray_popup_item_style.h"
 #include "ash/common/system/tray/tray_popup_label_button.h"
 #include "ash/common/system/tray/tray_popup_label_button_border.h"
 #include "ash/common/system/tray/tray_popup_utils.h"
@@ -104,16 +105,16 @@ bool IsMultiProfileSupportedAndUserActive() {
 }
 
 // Creates the view shown in the user switcher popup ("AddUserMenuOption").
-views::View* CreateAddUserView(AddUserSessionPolicy policy) {
+views::View* CreateAddUserView(AddUserSessionPolicy policy,
+                               views::ButtonListener* listener) {
   DCHECK(UseMd());
   auto view = new views::View;
-  auto layout = new views::BoxLayout(
-      views::BoxLayout::kHorizontal, (kMenuButtonSize - kMenuIconSize) / 2,
-      kMenuSeparatorVerticalPadding, kTrayPopupPaddingBetweenItems);
+  const int icon_padding = (kMenuButtonSize - kMenuIconSize) / 2;
+  auto layout =
+      new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0,
+                           kTrayPopupLabelHorizontalPadding + icon_padding);
   layout->set_minimum_cross_axis_size(
-      policy == AddUserSessionPolicy::ALLOWED
-          ? GetTrayConstant(TRAY_POPUP_ITEM_MIN_HEIGHT)
-          : 56);
+      GetTrayConstant(TRAY_POPUP_ITEM_MIN_HEIGHT));
   view->SetLayoutManager(layout);
   view->set_background(
       views::Background::CreateSolidBackground(kBackgroundColor));
@@ -143,7 +144,29 @@ views::View* CreateAddUserView(AddUserSessionPolicy policy) {
   auto command_label = new views::Label(l10n_util::GetStringUTF16(message_id));
   command_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   command_label->SetMultiLine(true);
+
+  TrayPopupItemStyle label_style(
+      TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL);
+  int vertical_padding = kMenuSeparatorVerticalPadding;
+  if (policy != AddUserSessionPolicy::ALLOWED) {
+    label_style.set_font_style(TrayPopupItemStyle::FontStyle::CAPTION);
+    label_style.set_color_style(TrayPopupItemStyle::ColorStyle::INACTIVE);
+    vertical_padding += kMenuSeparatorVerticalPadding;

[tdanderson, 2016/11/28 20:55:30]

From the bug, Sebastien has specified "the entire additional row ("sign in
another user" or "All available...) need to be moved down 8dp" but here it looks
like you're giving 4dp of extra padding for "Sign in another user" but 8dp extra
padding to "All available...". Just wanted to double check this is intentional.

[Evan Stade, 2016/11/29 02:30:33]

I added 4 with add_user_padding (because that 4 is not supposed to be part of
the ripple). I added 4 here for "sign in another", making 8. I added 4 more top
and bottom for "all available" which is supposed to be 8dp taller (not
explicitly mentioned in the bug, but this is where the previous 56 came from).

+  }
+  label_style.SetupLabel(command_label);
   view->AddChildView(command_label);
+  view->SetBorder(views::CreateEmptyBorder(vertical_padding, icon_padding,
+                                           vertical_padding,
+                                           kTrayPopupLabelHorizontalPadding));
+  if (policy == AddUserSessionPolicy::ALLOWED) {
+    auto button =
+        new ButtonFromView(view, listener, TrayPopupInkDropStyle::INSET_BOUNDS,
+                           false, gfx::Insets());
+    button->SetAccessibleName(
+        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SIGN_IN_ANOTHER_ACCOUNT));
+    return button;

[tdanderson, 2016/11/28 20:55:30]

Thanks for making this a button only when it needs to be.

[Evan Stade, 2016/11/29 02:30:33]

Acknowledged.

+  }
+
   return view;
 }
 
@@ -609,15 +632,6 @@ void UserView::ToggleAddUserMenuOption() {
   add_user_enabled_ = add_user_policy == AddUserSessionPolicy::ALLOWED;
 
   if (UseMd()) {
-    ButtonFromView* button = new ButtonFromView(
-        CreateAddUserView(add_user_policy), add_user_enabled_ ? this : nullptr,
-        IsActiveUser() ? TrayPopupInkDropStyle::INSET_BOUNDS
-                       : TrayPopupInkDropStyle::FILL_BOUNDS,
-        false, gfx::Insets());
-    button->SetAccessibleName(
-        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SIGN_IN_ANOTHER_ACCOUNT));
-    button->ForceBorderVisible(true);
-
     // Position the widget on top of the user card view (which is still in the
     // system menu). The top half of the widget will be transparent to allow
     // the active user to show through.
@@ -631,8 +645,14 @@ void UserView::ToggleAddUserMenuOption() {
         views::CreateSolidSidedBorder(0, 0, 0, kSeparatorWidth,
                                       kBackgroundColor),
         gfx::Insets(row_height, 0, 0, 0)));
+    views::View* add_user_padding = new views::View();
+    add_user_padding->SetBorder(views::CreateSolidSidedBorder(
+        kMenuSeparatorVerticalPadding, 0, 0, 0, kBackgroundColor));
+    views::View* add_user_view = CreateAddUserView(add_user_policy, this);
+    add_user_padding->AddChildView(add_user_view);
+    add_user_padding->SetLayoutManager(new views::FillLayout());
+    container->AddChildView(add_user_padding);
     container->SetLayoutManager(new views::FillLayout());
-    container->AddChildView(button);
     add_menu_option_->SetContentsView(container);
 
     bounds.set_height(container->GetPreferredSize().height());
@@ -644,8 +664,8 @@ void UserView::ToggleAddUserMenuOption() {
 
     // We activate the entry automatically if invoked with focus.
     if (add_user_enabled_ && user_card_view_->HasFocus()) {
-      button->GetFocusManager()->SetFocusedView(button);
-      user_card_view_->GetFocusManager()->SetFocusedView(button);
+      add_user_view->GetFocusManager()->SetFocusedView(add_user_view);
+      user_card_view_->GetFocusManager()->SetFocusedView(add_user_view);
     }
   } else {
     AddUserView* add_user_view =