[ash-md] Adjusts layout of lists with sticky header rows to match specs

ash/common/system/chromeos/network/vpn_list_view.cc

Index: ash/common/system/chromeos/network/vpn_list_view.cc
diff --git a/ash/common/system/chromeos/network/vpn_list_view.cc b/ash/common/system/chromeos/network/vpn_list_view.cc
index d35dda19a5ac9af3af811c49b517b5ca9e26e350..4f6e10da93e6a058acac28c3244adf2e81989ea8 100644
--- a/ash/common/system/chromeos/network/vpn_list_view.cc
+++ b/ash/common/system/chromeos/network/vpn_list_view.cc
@@ -110,17 +110,17 @@ class VPNListProviderEntryMd : public views::ButtonListener,
   VPNListProviderEntryMd(ViewClickListener* parent,
                          const std::string& name,
                          int button_accessible_name_id)
-      : parent_(parent) {
+      : parent_(parent), tri_view_(nullptr) {
+    // TODO(varkha): Make this a sticky section header.
     SetLayoutManager(new views::FillLayout);
-    TriView* tri_view = TrayPopupUtils::CreateDefaultRowView();
-    tri_view->SetContainerVisible(TriView::Container::START, false);
-    AddChildView(tri_view);
+    tri_view_ = TrayPopupUtils::CreateSubHeaderRowView();
+    AddChildView(tri_view_);
 
     views::Label* label = TrayPopupUtils::CreateDefaultLabel();
     TrayPopupItemStyle style(TrayPopupItemStyle::FontStyle::SUB_HEADER);
     style.SetupLabel(label);
     label->SetText(base::ASCIIToUTF16(name));
-    tri_view->AddView(TriView::Container::CENTER, label);
+    tri_view_->AddView(TriView::Container::CENTER, label);
 
     gfx::ImageSkia icon = gfx::CreateVectorIcon(kSystemMenuAddConnectionIcon,
                                                 style.GetIconColor());
@@ -129,7 +129,18 @@ class VPNListProviderEntryMd : public views::ButtonListener,
                              icon, button_accessible_name_id);
     add_vpn_button->SetInkDropColor(style.GetIconColor());
     add_vpn_button->SetEnabled(true);
-    tri_view->AddView(TriView::Container::END, add_vpn_button);
+    tri_view_->AddView(TriView::Container::END, add_vpn_button);
+  }
+
+  // Sets up the border. When the provider header is the first item in the
+  // list (i.e. when the list was empty before adding the provider row) there is
+  // already |kMenuSeparatorVerticalPadding| padding at the top of the scroll
+  // contents, so only add that padding when the list was not |empty|.
+  // TODO(varkha): Remove this special handling when this header becomes sticky.
+  void SetBorderForHeader(bool empty) {
+    tri_view_->SetBorder(views::CreateEmptyBorder(

[bruthig, 2016/11/28 21:43:12]

How much of this is going to live and how much is going to die when addressing
the TODO?  Anything that is going to live should be pushed into
TPIS::CreateSubHeaderRowView().  You could even move it all there now and have
it take in the |empty| param.  I'm assuming this layout should be the same for
Networking sub-section headers as well if/when it is fixed to use a TriView?

I've been trying to keep any TriView border configurations inside of TPIS and if
anything custom is required by TPIS clients they can update the borders of the
individual containers via TriView::SetContainerBorder().

[varkha, 2016/11/28 22:48:25]

It is manipulated depending on whether it is the first row (not just at creation
time). That will not be needed when the header is setup as a sticky header row.
I can still add a parameter to ctor - that will make things a bit less verbose I
think.

+        empty ? 0 : kMenuSeparatorVerticalPadding, kTrayPopupPaddingHorizontal,
+        kMenuSeparatorVerticalPadding, 0));
   }
 
  protected:
@@ -142,6 +153,9 @@ class VPNListProviderEntryMd : public views::ButtonListener,
   // Our parent to handle events.
   ViewClickListener* parent_;
 
+  // Container that owns the child controls.
+  TriView* tri_view_;
+
   DISALLOW_COPY_AND_ASSIGN(VPNListProviderEntryMd);
 };
 
@@ -449,29 +463,26 @@ void VPNListView::AddProviderAndNetworks(
     const VPNProvider& vpn_provider,
     const chromeos::NetworkStateHandler::NetworkStateList& networks) {
   // Add a visual separator, unless this is the topmost entry in the list.
-  if (!list_empty_) {
-    views::Separator* const separator =
-        new views::Separator(views::Separator::HORIZONTAL);
-    separator->SetColor(kBorderLightColor);
-    container()->AddChildView(separator);
-  } else {
-    list_empty_ = false;
-  }
+  if (!list_empty_)
+    container()->AddChildView(TrayPopupUtils::CreateListSubHeaderSeparator());
   std::string vpn_name =
       vpn_provider.third_party
           ? vpn_provider.third_party_provider_name
           : l10n_util::GetStringUTF8(IDS_ASH_STATUS_TRAY_VPN_BUILT_IN_PROVIDER);
 
   // Add a list entry for the VPN provider.
-  views::View* provider_view = nullptr;
+  views::View* view = nullptr;
   if (UseMd()) {
-    provider_view = new VPNListProviderEntryMd(
+    VPNListProviderEntryMd* provider_view = new VPNListProviderEntryMd(
         this, vpn_name, IDS_ASH_STATUS_TRAY_ADD_CONNECTION);
+    provider_view->SetBorderForHeader(list_empty_);
+    view = provider_view;
   } else {
-    provider_view = new VPNListProviderEntry(this, vpn_name);
+    view = new VPNListProviderEntry(this, vpn_name);
   }
-  container()->AddChildView(provider_view);
-  provider_view_map_[provider_view] = vpn_provider;
+  container()->AddChildView(view);
+  provider_view_map_[view] = vpn_provider;
+  list_empty_ = false;
   // Add the networks belonging to this provider, in the priority order returned
   // by shill.
   for (const chromeos::NetworkState* const& network : networks) {