[Web Payments] Add inline items to the Order Summary section.

chrome/browser/ui/views/payments/payment_sheet_view_controller.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 "chrome/browser/ui/views/payments/payment_sheet_view_controller.h"
 
 #include <algorithm>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/ui/views/payments/payment_request_dialog_view.h"
 #include "chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h"
 #include "chrome/browser/ui/views/payments/payment_request_row_view.h"
 #include "chrome/browser/ui/views/payments/payment_request_views_util.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
@@ skipping 132 matching lines @@
   views::ImageView* chevron = new views::ImageView();
   chevron->set_can_process_events_within_subtree(false);
   chevron->SetImage(gfx::CreateVectorIcon(
       views::kSubmenuArrowIcon,
       color_utils::DeriveDefaultIconColor(name_label->enabled_color())));
   layout->AddView(chevron);
 
   return std::move(row);
 }
 
+std::unique_ptr<views::GridLayout> CreateOrderSummarySectionContainerLayout(

[Mathieu, 2017/03/06 15:30:45]

Let's document this a bit, including what |host| and |trailing| are.

[anthonyvd, 2017/03/06 16:00:21]

Oops, good call. Done.

+    views::View* host,
+    bool trailing) {
+  std::unique_ptr<views::GridLayout> layout =
+      base::MakeUnique<views::GridLayout>(host);
+
+  views::ColumnSet* columns = layout->AddColumnSet(0);
+  columns->AddColumn(
+      trailing ? views::GridLayout::TRAILING : views::GridLayout::LEADING,
+      views::GridLayout::LEADING, 1, views::GridLayout::USE_PREF, 0, 0);
+
+  return layout;
+}
+
 }  // namespace
 
 PaymentSheetViewController::PaymentSheetViewController(
     PaymentRequest* request,
     PaymentRequestDialogView* dialog)
     : PaymentRequestSheetController(request, dialog),
       pay_button_(nullptr),
       widest_name_column_view_width_(ComputeWidestNameColumnViewWidth()) {
   request->AddObserver(this);
 }
@@ skipping 111 matching lines @@
   pay_button_->SetEnabled(enabled);
 }
 
 std::unique_ptr<views::View>
 PaymentSheetViewController::CreateOrderSummarySectionContent() {
   CurrencyFormatter* formatter = request()->GetOrCreateCurrencyFormatter(
       request()->details()->total->amount->currency,
       request()->details()->total->amount->currency_system,
       g_browser_process->GetApplicationLocale());
   base::string16 label_value = l10n_util::GetStringFUTF16(
-      IDS_PAYMENT_REQUEST_ORDER_SUMMARY_SECTION_TOTAL_FORMAT,
-      base::UTF8ToUTF16(request()->details()->total->label),
+      IDS_PAYMENT_REQUEST_ORDER_SUMMARY_SHEET_TOTAL_FORMAT,
       base::UTF8ToUTF16(formatter->formatted_currency_code()),
       formatter->Format(request()->details()->total->amount->value));
 
-  return base::MakeUnique<views::Label>(label_value);
+  return CreateBoldLabel(label_value);
 }
 
 // Creates the Order Summary row, which contains an "Order Summary" label,
-// a Total Amount label, and a Chevron.
+// an inline list of display items, a Total Amount label, and a Chevron.
 // +----------------------------------------------+
-// | Order Summary           Total USD $12.34   > |
+// | Order Summary   Item 1            $ 1.34     |
+// |                 Item 2            $ 2.00   > |
+// |                 2 more items...              |
+// |                 Total         USD $12.34     |
 // +----------------------------------------------+
 std::unique_ptr<views::Button>
 PaymentSheetViewController::CreatePaymentSheetSummaryRow() {
+  std::unique_ptr<views::View> item_summary = base::MakeUnique<views::View>();

[Mathieu, 2017/03/06 15:30:48]

nit: item_summaries?

[anthonyvd, 2017/03/06 16:00:21]

Done.

+  std::unique_ptr<views::GridLayout> item_summary_layout =
+      CreateOrderSummarySectionContainerLayout(item_summary.get(),
+                                               false /* trailing */);

[Mathieu, 2017/03/06 15:30:49]

nit: would prefer /*trailing=*/false otherwise it could be interpreted both ways

[anthonyvd, 2017/03/06 16:00:21]

Hah, didn't think about that! Done. I'll prefer that style in the future :)

+
+  std::unique_ptr<views::View> item_amounts = base::MakeUnique<views::View>();
+  std::unique_ptr<views::GridLayout> item_amounts_layout =
+      CreateOrderSummarySectionContainerLayout(item_amounts.get(),
+                                               true /* trailing */);
+
+  const std::vector<mojom::PaymentItemPtr>& items =
+      request()->details()->display_items;
+  // The inline items section contains the first 2 display items of the
+  // request's details, followed by a label indicating "N more items..." if
+  // there are more than 2 items in the details. The total label and amount
+  // always follow.
+

[Mathieu, 2017/03/06 15:30:49]

nit: remove extra line

[anthonyvd, 2017/03/06 16:00:20]

Done.

+  for (size_t i = 0; i < items.size() && i < 2; ++i) {

[Mathieu, 2017/03/06 15:30:45]

constexpr int kMaxNumberOfItemsShown = 2; and reuse below.

OR

Consider reusing |i| after the loop has finished.

[anthonyvd, 2017/03/06 16:00:21]

Done. 

+    item_summary_layout->StartRow(0, 0);
+    item_summary_layout->AddView(
+        new views::Label(base::ASCIIToUTF16(items[i]->label)));
+
+    item_amounts_layout->StartRow(0, 0);
+    item_amounts_layout->AddView(new views::Label(
+        request()->GetFormattedCurrencyAmount(items[i]->amount->value)));

[Mathieu, 2017/03/06 15:30:47]

this is nice! Could we update it in order_summary_view_controller as well?

[anthonyvd, 2017/03/06 16:00:21]

Sure, done.

+  }
+
+  int hidden_item_count = items.size() - 2;
+  if (hidden_item_count > 0) {
+    item_summary_layout->StartRow(0, 0);
+    views::Label* label = new views::Label(
+        l10n_util::GetStringFUTF16(IDS_PAYMENT_REQUEST_ORDER_SUMMARY_MORE_ITEMS,
+                                   base::IntToString16(hidden_item_count)));
+    label->SetAutoColorReadabilityEnabled(false);
+    constexpr SkColor kMoreItemsLightGray = 0xFF757575;

[Mathieu, 2017/03/06 15:30:49]

In editor view controller I was doing this:

  label->SetDisabledColor(label->GetNativeTheme()->GetSystemColor(
      ui::NativeTheme::kColorId_LabelDisabledColor));
  label->SetEnabled(false);

I guess 1 comment and one question
(1) Let's use label->GetNativeTheme()->GetSystemColor(
      ui::NativeTheme::kColorId_LabelDisabledColor)
(2) seems like there are multiple ways to skin a cat! Does
SetAutoColorReadabilityEnabled do something interesting vs
SetDisabledColor+SetDisabled? Perhaps the clickability of the label?

[anthonyvd, 2017/03/06 16:00:20]

(1) Done.
(2) It looks like SetAutoColorReadabilityEnabled(false) prevents the view from
fudging the color to contrast on the background properly. There's no advantage
to that in our case IMO, so your way is strictly better.

+    label->SetEnabledColor(kMoreItemsLightGray);
+    item_summary_layout->AddView(label);
+
+    item_amounts_layout->StartRow(0, 0);
+    item_amounts_layout->AddView(new views::Label(base::ASCIIToUTF16("")));
+  }
+
+  item_summary_layout->StartRow(0, 0);
+  item_summary_layout->AddView(
+      CreateBoldLabel(base::ASCIIToUTF16(request()->details()->total->label))

[Mathieu, 2017/03/06 15:30:49]

Where is the total amount being added?

[anthonyvd, 2017/03/06 16:00:21]

The CreateOrderSummarySectionContent() call below does. It's a remnant of
earlier times. I inlined the code instead.

+          .release());
+
+  item_amounts_layout->StartRow(0, 0);
+  item_amounts_layout->AddView(CreateOrderSummarySectionContent().release());
+
+  item_summary->SetLayoutManager(item_summary_layout.release());
+  item_amounts->SetLayoutManager(item_amounts_layout.release());
+
   std::unique_ptr<views::Button> section = CreatePaymentSheetRow(
       this,
       l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_ORDER_SUMMARY_SECTION_NAME),
-      std::unique_ptr<views::View>(nullptr),
-      CreateOrderSummarySectionContent(),
+      std::move(item_summary), std::move(item_amounts),
       widest_name_column_view_width_);
   section->set_tag(static_cast<int>(
       PaymentSheetViewControllerTags::SHOW_ORDER_SUMMARY_BUTTON));
   section->set_id(
       static_cast<int>(DialogViewID::PAYMENT_SHEET_SUMMARY_SECTION));
   return section;
 }
 
 std::unique_ptr<views::View>
 PaymentSheetViewController::CreateShippingSectionContent() {
@@ skipping 99 matching lines @@
       CreateContactInfoSectionContent(), std::unique_ptr<views::View>(nullptr),
       widest_name_column_view_width_);
   section->set_tag(static_cast<int>(
       PaymentSheetViewControllerTags::SHOW_CONTACT_INFO_BUTTON));
   section->set_id(
       static_cast<int>(DialogViewID::PAYMENT_SHEET_CONTACT_INFO_SECTION));
   return section;
 }
 
 }  // namespace payments