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

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

 // Copyright 2015 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/chromeos/network/vpn_list_view.h"
 
 #include <memory>
 #include <utility>
 #include <vector>
 
@@ skipping 92 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(VPNListProviderEntry);
 };
 
 // A list entry that represents a VPN provider with Material Design.
 class VPNListProviderEntryMd : public views::ButtonListener,
                                public views::View {
  public:
   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());
     SystemMenuButton* add_vpn_button =
         new SystemMenuButton(this, TrayPopupInkDropStyle::HOST_CENTERED, icon,
                              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:
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override {
     parent_->OnViewClicked(this);
   }
 
  private:
   // Our parent to handle events.
   ViewClickListener* parent_;
 
+  // Container that owns the child controls.
+  TriView* tri_view_;
+
   DISALLOW_COPY_AND_ASSIGN(VPNListProviderEntryMd);
 };
 
 // A list entry that represents a network. If the network is currently
 // connecting, the icon shown by this list entry will be animated. If the
 // network is currently connected, a disconnect button will be shown next to its
 // name.
 class VPNListNetworkEntry : public VPNListEntryBase,
                             public network_icon::AnimationObserver {
  public:
@@ skipping 287 matching lines @@
   views::View* entry(new VPNListNetworkEntry(this, network));
   container()->AddChildView(entry);
   network_view_service_path_map_[entry] = network->path();
   list_empty_ = false;
 }
 
 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) {
     if (VpnProviderMatchesNetwork(vpn_provider, *network))
       AddNetwork(network);
   }
 }
 
 void VPNListView::AddProvidersAndNetworks(
     const chromeos::NetworkStateHandler::NetworkStateList& networks) {
@@ skipping 15 matching lines @@
     }
   }
 
   // Add providers without any configured networks, in the order that the
   // providers were returned by the extensions system.
   for (const VPNProvider& provider : providers)
     AddProviderAndNetworks(provider, networks);
 }
 
 }  // namespace ash