Make WebAuthN HoverListView scrollable

HoverListView is a model-agnostic view that holds multiple
clickable item views. In WebAuthn, this view is used to display a list
of BLE authenticators. This CL makes the view scrollable to handle
an arbitrary number of BLE authenticators in the UI.

Bug: 877344
Change-Id: I86f68bc3939ff661c982ae0e6bec1eed2d9a3a3a
Reviewed-on: https://chromium-review.googlesource.com/c/1322832
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607735}

chrome/browser/ui/views/webauthn/hover_list_view.cc

@@ -19,6 +19,7 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
+#include "ui/views/layout/fill_layout.h"
 #include "ui/views/vector_icons.h"
 
 namespace {
@@ -86,7 +87,6 @@ std::unique_ptr<HoverButton> CreateHoverButtonForListItem(
 views::Separator* AddSeparatorAsChild(views::View* view) {
   auto* separator = new views::Separator();
   separator->SetColor(gfx::kGoogleGrey300);
-
   view->AddChildView(separator);
   return separator;
 }
@@ -98,10 +98,14 @@ views::Separator* AddSeparatorAsChild(views::View* view) {
 HoverListView::HoverListView(std::unique_ptr<HoverListModel> model)
     : model_(std::move(model)) {
   DCHECK(model_);
+  SetLayoutManager(std::make_unique<views::FillLayout>());
+
+  item_container_ = new views::View();
+  item_container_->SetLayoutManager(std::make_unique<views::BoxLayout>(
+      views::BoxLayout::kVertical, gfx::Insets(),
+      0 /* betweeen_child_spacing */));
 
-  SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::kVertical, gfx::Insets(), 0));
-  AddSeparatorAsChild(this);
+  AddSeparatorAsChild(item_container_);
 
   for (const auto item_tag : model_->GetItemTags()) {
     AppendListItemView(model_->GetItemIcon(item_tag),
@@ -109,8 +113,15 @@ HoverListView::HoverListView(std::unique_ptr<HoverListModel> model)
   }
 
   if (tags_to_list_item_views_.empty() &&
-      model_->ShouldShowPlaceholderForEmptyList())
+      model_->ShouldShowPlaceholderForEmptyList()) {
     CreateAndAppendPlaceholderItem();
+  }
+
+  scroll_view_ = new views::ScrollView();
+  scroll_view_->SetContents(item_container_);
+  AddChildView(scroll_view_);
+  scroll_view_->ClipHeightTo(GetPreferredViewHeight(),
+                             GetPreferredViewHeight());
 
   model_->SetObserver(this);
 }
@@ -126,8 +137,8 @@ void HoverListView::AppendListItemView(const gfx::VectorIcon& icon,
       CreateHoverButtonForListItem(item_tag, icon, item_text, this);
 
   auto* list_item_view_ptr = hover_button.release();
-  AddChildView(list_item_view_ptr);
-  auto* separator = AddSeparatorAsChild(this);
+  item_container_->AddChildView(list_item_view_ptr);
+  auto* separator = AddSeparatorAsChild(item_container_);
   tags_to_list_item_views_.emplace(
       item_tag, ListItemViews{list_item_view_ptr, separator});
 }
@@ -136,8 +147,8 @@ void HoverListView::CreateAndAppendPlaceholderItem() {
   auto placeholder_item = CreateHoverButtonForListItem(
       kPlaceHolderItemTag, model_->GetPlaceholderIcon(),
       model_->GetPlaceholderText(), nullptr, true /* is_placeholder_item */);
-  AddChildView(placeholder_item.get());
-  auto* separator = AddSeparatorAsChild(this);
+  item_container_->AddChildView(placeholder_item.get());
+  auto* separator = AddSeparatorAsChild(item_container_);
   placeholder_list_item_view_.emplace(
       ListItemViews{placeholder_item.release(), separator});
 }
@@ -146,7 +157,7 @@ void HoverListView::AddListItemView(int item_tag) {
   CHECK(!base::ContainsKey(tags_to_list_item_views_, item_tag));
   if (placeholder_list_item_view_) {
     RemoveListItemView(*placeholder_list_item_view_);
-    placeholder_list_item_view_.emplace();
+    placeholder_list_item_view_.reset();
   }
 
   AppendListItemView(model_->GetItemIcon(item_tag),
@@ -166,6 +177,15 @@ void HoverListView::RemoveListItemView(int item_tag) {
   RemoveListItemView(view_it->second);
   tags_to_list_item_views_.erase(view_it);
 
+  // Removed list item may have not been the bottom-most view in the scroll
+  // view. To enforce that all remaining items are re-shifted to the top,
+  // invalidate all child views.
+  //
+  // TODO(hongjunchoi): Restructure HoverListView and |scroll_view_| so that
+  // InvalidateLayout() does not need to be explicitly called when items are
+  // removed from the list. See: https://crbug.com/904968
+  item_container_->InvalidateLayout();
+
   if (tags_to_list_item_views_.empty() &&
       model_->ShouldShowPlaceholderForEmptyList()) {
     CreateAndAppendPlaceholderItem();
@@ -178,10 +198,10 @@ void HoverListView::RemoveListItemView(int item_tag) {
 }
 
 void HoverListView::RemoveListItemView(ListItemViews list_item) {
-  DCHECK(Contains(list_item.item_view));
-  DCHECK(Contains(list_item.separator_view));
-  RemoveChildView(list_item.item_view);
-  RemoveChildView(list_item.separator_view);
+  DCHECK(item_container_->Contains(list_item.item_view));
+  DCHECK(item_container_->Contains(list_item.separator_view));
+  item_container_->RemoveChildView(list_item.item_view);
+  item_container_->RemoveChildView(list_item.separator_view);
 }
 
 views::Button& HoverListView::GetTopListItemView() const {
@@ -217,3 +237,10 @@ void HoverListView::ButtonPressed(views::Button* sender,
                                   const ui::Event& event) {
   model_->OnListItemSelected(sender->tag());
 }
+
+int HoverListView::GetPreferredViewHeight() const {
+  auto dummy_hover_button = CreateHoverButtonForListItem(
+      -1 /* tag */, gfx::kNoneIcon, base::string16(), nullptr /* listener */);
+  return dummy_hover_button->GetPreferredSize().height() *
+         model_->GetPreferredItemCount();
+}

chrome/browser/ui/views/webauthn/hover_list_view.h

@@ -13,6 +13,7 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/ui/views/hover_button.h"
 #include "chrome/browser/ui/webauthn/hover_list_model.h"
+#include "ui/views/controls/scroll_view.h"
 #include "ui/views/view.h"
 
 namespace gfx {
@@ -57,6 +58,7 @@ class HoverListView : public views::View,
   void RemoveListItemView(int item_tag);
   void RemoveListItemView(ListItemViews list_item);
   views::Button& GetTopListItemView() const;
+  int GetPreferredViewHeight() const;
 
   // views::View:
   void RequestFocus() override;
@@ -73,6 +75,8 @@ class HoverListView : public views::View,
   std::unique_ptr<HoverListModel> model_;
   std::map<int, ListItemViews> tags_to_list_item_views_;
   base::Optional<ListItemViews> placeholder_list_item_view_;
+  views::ScrollView* scroll_view_;
+  views::View* item_container_;
 
   DISALLOW_COPY_AND_ASSIGN(HoverListView);
 };

chrome/browser/ui/webauthn/ble_device_hover_list_model.cc

@@ -16,6 +16,8 @@
 
 namespace {
 
+constexpr size_t kDefaultItemViewCount = 2;
+
 std::map<int, std::string>::iterator FindElementByValue(
     std::map<int, std::string>* item_map,
     base::StringPiece value) {
@@ -102,6 +104,10 @@ void BleDeviceHoverListModel::OnListItemSelected(int item_tag) {
   delegate_->OnItemSelected(authenticator_item->second);
 }
 
+size_t BleDeviceHoverListModel::GetPreferredItemCount() const {
+  return kDefaultItemViewCount;
+}
+
 void BleDeviceHoverListModel::OnAuthenticatorAdded(
     const AuthenticatorReference& authenticator) {
   auto item_tag = authenticator_tags_.empty()

chrome/browser/ui/webauthn/ble_device_hover_list_model.h

@@ -44,6 +44,7 @@ class BleDeviceHoverListModel : public HoverListModel,
   const gfx::VectorIcon& GetItemIcon(int item_tag) const override;
   std::vector<int> GetItemTags() const override;
   void OnListItemSelected(int item_tag) override;
+  size_t GetPreferredItemCount() const override;
 
   // AuthenticatorListObserver:
   void OnAuthenticatorAdded(

chrome/browser/ui/webauthn/hover_list_model.h

@@ -42,6 +42,7 @@ class HoverListModel {
   virtual base::string16 GetItemText(int item_tag) const = 0;
   virtual const gfx::VectorIcon& GetItemIcon(int item_tag) const = 0;
   virtual void OnListItemSelected(int item_tag) = 0;
+  virtual size_t GetPreferredItemCount() const = 0;
 
   void SetObserver(Observer* observer) {
     DCHECK(!observer_);

chrome/browser/ui/webauthn/transport_hover_list_model.cc

@@ -51,3 +51,7 @@ void TransportHoverListModel::OnListItemSelected(int item_tag) {
   if (delegate_)
     delegate_->OnItemSelected(static_cast<AuthenticatorTransport>(item_tag));
 }
+
+size_t TransportHoverListModel::GetPreferredItemCount() const {
+  return transport_list_.size();
+}

chrome/browser/ui/webauthn/transport_hover_list_model.h

@@ -34,6 +34,7 @@ class TransportHoverListModel : public HoverListModel {
   base::string16 GetItemText(int item_tag) const override;
   const gfx::VectorIcon& GetItemIcon(int item_tag) const override;
   void OnListItemSelected(int item_tag) override;
+  size_t GetPreferredItemCount() const override;
 
  private:
   std::vector<AuthenticatorTransport> transport_list_;