Use PaymentRequestItemList for Shipping screen

chrome/browser/ui/views/payments/shipping_list_view_controller.cc

Index: chrome/browser/ui/views/payments/shipping_list_view_controller.cc
diff --git a/chrome/browser/ui/views/payments/shipping_list_view_controller.cc b/chrome/browser/ui/views/payments/shipping_list_view_controller.cc
index 5109745f5c829e6538ab3c169bffd395055549bd..264232ef60e54ff8d2507e3aa34b4d9a5a069983 100644
--- a/chrome/browser/ui/views/payments/shipping_list_view_controller.cc
+++ b/chrome/browser/ui/views/payments/shipping_list_view_controller.cc
@@ -13,11 +13,72 @@
 #include "components/payments/payment_request.h"
 #include "components/strings/grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/views/controls/image_view.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 
 namespace payments {
 
+class ShippingListItem : public payments::PaymentRequestItemList::Item,
+                         public views::ButtonListener {
+ public:
+  ShippingListItem(autofill::AutofillProfile* profile,
+                   PaymentRequest* request,
+                   PaymentRequestItemList* list,
+                   bool selected)
+      : payments::PaymentRequestItemList::Item(request, list, selected),
+        profile_(profile) {}
+  ~ShippingListItem() override {}
+
+ private:
+  // payments::PaymentRequestItemList::Item:
+  std::unique_ptr<views::View> CreateItemView() override {
+    DCHECK(profile_);
+
+    // TODO(tmartino): Pass an actual locale in place of empty string.

[anthonyvd, 2017/03/01 15:37:00]

As of https://codereview.chromium.org/2715213005/, we have 
PaymentRequestDelegate::GetApplicationLocale(), you can use it here (and we
should use that moving forward instead of passing locales around, at least for
things that have access to the PaymentRequest object).

[tmartino, 2017/03/01 20:00:30]

We don't have public access to the delegate from the PaymentRequest. We should
probably add a passthrough method to PaymentRequest to fetch the locale from the
delegate, but I would prefer not to touch PaymentRequest in this CL.

[anthonyvd, 2017/03/01 20:15:34]

Ahh, agreed with the passthrough and ok with deferring it to another CL.

[tmartino, 2017/03/01 22:48:14]

sgtm

+    std::unique_ptr<views::View> content = GetShippingAddressLabel(
+        AddressStyleType::DETAILED, std::string(), *profile_);
+
+    std::unique_ptr<PaymentRequestRowView> row =
+        base::MakeUnique<PaymentRequestRowView>(this);
+    views::GridLayout* layout = new views::GridLayout(row.get());
+    row->SetLayoutManager(layout);
+
+    layout->SetInsets(
+        kPaymentRequestRowVerticalInsets, kPaymentRequestRowHorizontalInsets,
+        kPaymentRequestRowVerticalInsets,
+        kPaymentRequestRowHorizontalInsets + kPaymentRequestRowExtraRightInset);
+
+    // Add a column listing the address.
+    views::ColumnSet* columns = layout->AddColumnSet(0);
+    columns->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
+                       views::GridLayout::USE_PREF, 0, 0);
+
+    columns->AddPaddingColumn(1, 0);
+
+    // Add a column for the checkmark shown next to the selected address.
+    columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
+                       0, views::GridLayout::USE_PREF, 0, 0);
+
+    layout->StartRow(0, 0);
+    content->set_can_process_events_within_subtree(false);
+    layout->AddView(content.release());
+
+    checkmark_ = CreateCheckmark(selected());
+    layout->AddView(checkmark_.get());
+
+    return std::move(row);
+  }
+
+  void SelectedStateChanged() override {}
+
+  // views::ButtonListener:
+  void ButtonPressed(views::Button* sender, const ui::Event& event) override {}
+
+  autofill::AutofillProfile* profile_;
+  std::unique_ptr<views::ImageView> checkmark_;
+};
+
 ShippingListViewController::ShippingListViewController(

[anthonyvd, 2017/03/01 15:37:00]

Normally I'd say we should add tests to this similar to the ones for the payment
method sheet but I suspect we'll only have one set of such tests post-refactor
so let's wait until then. 

They'd basically be a copy-paste of the payment method ones anyways.

[tmartino, 2017/03/01 20:00:30]

Ack

     PaymentRequest* request,
     PaymentRequestDialogView* dialog)
@@ -26,21 +87,11 @@ ShippingListViewController::ShippingListViewController(
 ShippingListViewController::~ShippingListViewController() {}
 
 std::unique_ptr<views::View> ShippingListViewController::CreateView() {
-  std::unique_ptr<views::View> content_view = base::MakeUnique<views::View>();
-
-  views::BoxLayout* layout =
-      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0);
-  layout->set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_START);
-  layout->set_cross_axis_alignment(
-      views::BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH);
-  content_view->SetLayoutManager(layout);
+  auto* selected_profile = request()->selected_shipping_profile();
 
   for (auto* profile : request()->shipping_profiles()) {

[anthonyvd, 2017/03/01 15:37:01]

This loop can be in the constructor (for consistency with the other view
controllers that do the same thing).

[tmartino, 2017/03/01 20:00:30]

Done

-    // TODO(tmartino): Pass an actual locale in place of empty string.
-    content_view->AddChildView(
-        CreateAddressRow(GetShippingAddressLabel(AddressStyleType::DETAILED,
-                                                 std::string(), *profile))
-            .release());
+    list_.AddItem(base::MakeUnique<ShippingListItem>(
+        profile, request(), &list_, profile == selected_profile));
   }
 
   return CreatePaymentView(
@@ -48,31 +99,7 @@ std::unique_ptr<views::View> ShippingListViewController::CreateView() {
           /* show_back_arrow = */ true,
           l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_SHIPPING_SECTION_NAME),
           this),
-      std::move(content_view));
-}
-
-std::unique_ptr<views::Button> ShippingListViewController::CreateAddressRow(
-    std::unique_ptr<views::View> content) {
-  std::unique_ptr<PaymentRequestRowView> row =
-      base::MakeUnique<PaymentRequestRowView>(this);
-  views::GridLayout* layout = new views::GridLayout(row.get());
-  row->SetLayoutManager(layout);
-
-  layout->SetInsets(
-      kPaymentRequestRowVerticalInsets, kPaymentRequestRowHorizontalInsets,
-      kPaymentRequestRowVerticalInsets,
-      kPaymentRequestRowHorizontalInsets + kPaymentRequestRowExtraRightInset);
-
-  // Add a column listing the address.
-  views::ColumnSet* columns = layout->AddColumnSet(0);
-  columns->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, 1,
-                     views::GridLayout::USE_PREF, 0, 0);
-
-  layout->StartRow(0, 0);
-  content->set_can_process_events_within_subtree(false);
-  layout->AddView(content.release());
-
-  return std::move(row);
+      list_.CreateListView());
 }
 
 }  // namespace payments