views::Separator cleanup.

ash/common/system/tray/tray_popup_utils.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/common/system/tray/tray_popup_utils.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "ash/common/ash_constants.h"
@@ skipping 277 matching lines @@
       views::CreateEmptyBorder(gfx::Insets(kMenuSeparatorVerticalPadding, 0)));
   view->SetPaintToLayer();
   view->layer()->SetFillsBoundsOpaquely(false);
 }
 
 void TrayPopupUtils::ShowStickyHeaderSeparator(views::View* view,
                                                bool show_separator) {
   if (show_separator) {
     view->SetBorder(views::CreatePaddedBorder(
         views::CreateSolidSidedBorder(0, 0, kSeparatorWidth, 0,
-                                      kHorizontalSeparatorColor),
+                                      kMenuSeparatorColor),
         gfx::Insets(kMenuSeparatorVerticalPadding, 0,
                     kMenuSeparatorVerticalPadding - kSeparatorWidth, 0)));
   } else {
     view->SetBorder(views::CreateEmptyBorder(
         gfx::Insets(kMenuSeparatorVerticalPadding, 0)));
   }
   view->SchedulePaint();
 }
 
 void TrayPopupUtils::ConfigureContainer(TriView::Container container,
@@ skipping 14 matching lines @@
   if (!MaterialDesignController::IsSystemTrayMenuMaterial())
     return CreateTrayPopupBorderlessButton(listener, text);
 
   auto* button = views::MdTextButton::Create(listener, text);
   button->SetProminent(true);
   return button;
 }
 
 views::Separator* TrayPopupUtils::CreateVerticalSeparator() {
   views::Separator* separator =
-      new views::Separator(views::Separator::HORIZONTAL);

[Evan Stade, 2017/02/03 20:38:19]

This was actually creating a horizontal separator (i.e. --------) that was 24px
tall and 1 px long.

There seems to be confusion about the meaning of "horizontal", whether that's a
horizontal line or something that separates horizontal elements, but it's easy
to get it backwards and still have things work ok, especially for separators of
a default size (1x1 preferred size and gets laid out by a layout manager than
ignores one preferred dimension).

-  separator->SetPreferredSize(kHorizontalSeparatorHeight);
-  separator->SetColor(kHorizontalSeparatorColor);
+      new views::Separator(views::Separator::VERTICAL);
+  separator->SetPreferredLength(24);
+  separator->SetColor(kMenuSeparatorColor);
   return separator;
 }
 
 std::unique_ptr<views::InkDrop> TrayPopupUtils::CreateInkDrop(
     TrayPopupInkDropStyle ink_drop_style,
     views::InkDropHostView* host) {
   std::unique_ptr<views::InkDropImpl> ink_drop =
       base::MakeUnique<views::InkDropImpl>(host, host->size());
   ink_drop->SetAutoHighlightMode(
       views::InkDropImpl::AutoHighlightMode::SHOW_ON_RIPPLE);
@@ skipping 70 matching lines @@
 gfx::Rect TrayPopupUtils::GetInkDropBounds(TrayPopupInkDropStyle ink_drop_style,
                                            const views::View* host) {
   gfx::Rect bounds = host->GetLocalBounds();
   bounds.Inset(GetInkDropInsets(ink_drop_style));
   return bounds;
 }
 
 views::Separator* TrayPopupUtils::CreateListItemSeparator(bool left_inset) {
   views::Separator* separator =
       new views::Separator(views::Separator::HORIZONTAL);
-  separator->SetColor(kHorizontalSeparatorColor);
-  separator->SetPreferredSize(kSeparatorWidth);
+  separator->SetColor(kMenuSeparatorColor);
   separator->SetBorder(views::CreateEmptyBorder(
-      kMenuSeparatorVerticalPadding - kSeparatorWidth,
+      kMenuSeparatorVerticalPadding - views::Separator::kThickness,
       left_inset
           ? kMenuExtraMarginFromLeftEdge + kMenuButtonSize +
                 kTrayPopupLabelHorizontalPadding
           : 0,
       kMenuSeparatorVerticalPadding, 0));
   return separator;
 }
 
 views::Separator* TrayPopupUtils::CreateListSubHeaderSeparator() {
   views::Separator* separator =
       new views::Separator(views::Separator::HORIZONTAL);
-  separator->SetColor(kHorizontalSeparatorColor);
-  separator->SetPreferredSize(kSeparatorWidth);
+  separator->SetColor(kMenuSeparatorColor);
   separator->SetBorder(views::CreateEmptyBorder(
-      kMenuSeparatorVerticalPadding - kSeparatorWidth, 0, 0, 0));
+      kMenuSeparatorVerticalPadding - views::Separator::kThickness, 0, 0, 0));
   return separator;
 }
 
 bool TrayPopupUtils::CanOpenWebUISettings(LoginStatus status) {
   // TODO(tdanderson): Consider moving this into WmShell, or introduce a
   // CanShowSettings() method in each delegate type that has a
   // ShowSettings() method.
   return status != LoginStatus::NOT_LOGGED_IN &&
          status != LoginStatus::LOCKED &&
          !WmShell::Get()->GetSessionStateDelegate()->IsInSecondaryLoginScreen();
 }
 
 }  // namespace ash