[Web Payments] Prettify the payment sheet rows in some states.

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>
@@ skipping 69 matching lines @@
   for (int name_id : section_names) {
     label.SetText(l10n_util::GetStringUTF16(name_id));
     widest_column_width = std::max(
         label.GetPreferredSize().width(),
         widest_column_width);
   }
 
   return widest_column_width;
 }
 
-// Creates a clickable row to be displayed in the Payment Sheet. It contains
-// a section name and some content, followed by a chevron as a clickability
-// affordance. Both, either, or none of |content_view| and |extra_content_view|
-// may be present, the difference between the two being that content is pinned
-// to the left and extra_content is pinned to the right.
-// The row also displays a light gray horizontal ruler on its lower boundary.
-// The name column has a fixed width equal to |name_column_width|.
-// +----------------------------+
-// | Name | Content | Extra | > |
-// +~~~~~~~~~~~~~~~~~~~~~~~~~~~~+ <-- ruler
 std::unique_ptr<views::Button> CreatePaymentSheetRow(
     views::ButtonListener* listener,
     const base::string16& section_name,
     std::unique_ptr<views::View> content_view,
     std::unique_ptr<views::View> extra_content_view,
+    std::unique_ptr<views::View> trailing_button,
+    bool clickable,
     int name_column_width) {
   std::unique_ptr<PaymentRequestRowView> row =
-      base::MakeUnique<PaymentRequestRowView>(listener);
+      base::MakeUnique<PaymentRequestRowView>(listener, clickable);
   views::GridLayout* layout = new views::GridLayout(row.get());
 
   // The rows have extra inset compared to the header so that their right edge
   // lines up with the close button's X rather than its invisible right edge.
   layout->SetInsets(
       kPaymentRequestRowVerticalInsets, kPaymentRequestRowHorizontalInsets,
       kPaymentRequestRowVerticalInsets,
       kPaymentRequestRowHorizontalInsets + kPaymentRequestRowExtraRightInset);
   row->SetLayoutManager(layout);
 
@@ skipping 10 matching lines @@
   columns->AddPaddingColumn(0, kPaddingColumnsWidth);
 
   // A column for the content.
   columns->AddColumn(views::GridLayout::FILL, views::GridLayout::LEADING,
                      1, views::GridLayout::USE_PREF, 0, 0);
   // A column for the extra content.
   columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
                      0, views::GridLayout::USE_PREF, 0, 0);
 
   columns->AddPaddingColumn(0, kPaddingColumnsWidth);
-  // A column for the chevron.
+  // A column for the trailing_button.
   columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
                      0, views::GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, 0);
   views::Label* name_label = new views::Label(section_name);
   layout->AddView(name_label);
 
   if (content_view) {
     content_view->set_can_process_events_within_subtree(false);
     layout->AddView(content_view.release());
   } else {
     layout->SkipColumns(1);
   }
 
   if (extra_content_view) {
     extra_content_view->set_can_process_events_within_subtree(false);
     layout->AddView(extra_content_view.release());
   } else {
     layout->SkipColumns(1);
   }
 
-  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);
+  layout->AddView(trailing_button.release());
 
   return std::move(row);
 }
 
+std::unique_ptr<views::Button> CreatePaymentSheetRowWithButton(

[Mathieu, 2017/04/13 00:12:45]

would be great to have some ASCII here too

[anthonyvd, 2017/04/13 18:33:33]

Done.

+    views::ButtonListener* listener,
+    const base::string16& section_name,
+    const base::string16& truncated_content,
+    const base::string16& button_string,
+    int button_tag,
+    int button_id,
+    int name_column_width) {
+  std::unique_ptr<views::Button> button(
+      views::MdTextButton::CreateSecondaryUiBlueButton(listener,
+                                                       button_string));
+  button->set_tag(button_tag);
+  button->set_id(button_id);
+  std::unique_ptr<views::Label> content_view =
+      base::MakeUnique<views::Label>(truncated_content);
+  return CreatePaymentSheetRow(listener, section_name, std::move(content_view),
+                               nullptr, std::move(button),
+                               /* clickable= */ false, name_column_width);

[Mathieu, 2017/04/13 00:12:45]

very nit: I prefer /*clickable=*/false because then it stays one item once
formatting is done (although it's possible git cl format does the right thing
too)

[anthonyvd, 2017/04/13 18:33:33]

Done.

+}
+
+// Creates a clickable row to be displayed in the Payment Sheet. It contains
+// a section name and some content, followed by a chevron as a clickability
+// affordance. Both, either, or none of |content_view| and |extra_content_view|
+// may be present, the difference between the two being that content is pinned
+// to the left and extra_content is pinned to the right.
+// The row also displays a light gray horizontal ruler on its lower boundary.
+// The name column has a fixed width equal to |name_column_width|.
+// +----------------------------+
+// | Name | Content | Extra | > |
+// +~~~~~~~~~~~~~~~~~~~~~~~~~~~~+ <-- ruler
+std::unique_ptr<views::Button> CreatePaymentSheetRowWithChevron(
+    views::ButtonListener* listener,
+    const base::string16& section_name,
+    std::unique_ptr<views::View> content_view,
+    std::unique_ptr<views::View> extra_content_view,
+    int name_column_width) {

[Mathieu, 2017/04/13 00:12:45]

could we also accept the section_tag and section_id here?

[anthonyvd, 2017/04/13 18:33:33]

Done.

+  std::unique_ptr<views::ImageView> chevron =
+      base::MakeUnique<views::ImageView>();
+  chevron->set_can_process_events_within_subtree(false);
+  std::unique_ptr<views::Label> label =
+      base::MakeUnique<views::Label>(section_name);
+  chevron->SetImage(gfx::CreateVectorIcon(
+      views::kSubmenuArrowIcon,
+      color_utils::DeriveDefaultIconColor(label->enabled_color())));
+  return CreatePaymentSheetRow(listener, section_name, std::move(content_view),
+                               std::move(extra_content_view),
+                               std::move(chevron), /* clickable= */ true,
+                               name_column_width);
+}
+
 // Creates a GridLayout object to be used in the Order Summary section's list of
 // items and the list of prices. |host| is the view that will be assigned the
 // returned Layout Manager and |trailing| indicates whether the elements added
 // to the manager should have trailing horizontal alignment. If trailing is
 // |false|, their horizontal alignment is leading.
 std::unique_ptr<views::GridLayout> CreateOrderSummarySectionContainerLayout(
     views::View* host,
     bool trailing) {
   std::unique_ptr<views::GridLayout> layout =
       base::MakeUnique<views::GridLayout>(host);
@@ skipping 193 matching lines @@
       CreateBoldLabel(l10n_util::GetStringFUTF16(
                           IDS_PAYMENT_REQUEST_ORDER_SUMMARY_SHEET_TOTAL_FORMAT,
                           base::UTF8ToUTF16(spec()->GetFormattedCurrencyCode()),
                           spec()->GetFormattedCurrencyAmount(
                               spec()->details().total->amount->value)))
           .release());
 
   item_summaries->SetLayoutManager(item_summaries_layout.release());
   item_amounts->SetLayoutManager(item_amounts_layout.release());
 
-  std::unique_ptr<views::Button> section = CreatePaymentSheetRow(
+  std::unique_ptr<views::Button> section = CreatePaymentSheetRowWithChevron(
       this,
       l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_ORDER_SUMMARY_SECTION_NAME),
       std::move(item_summaries), 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() {
   auto* profile = state()->selected_shipping_profile();
 
   return profile ? GetShippingAddressLabel(AddressStyleType::SUMMARY,
                                            state()->GetApplicationLocale(),
                                            *profile)
                  : base::MakeUnique<views::Label>(base::string16());
 }
 
 // Creates the Shipping row, which contains a "Shipping address" label, the
 // user's selected shipping address, and a chevron.
 // +----------------------------------------------+
 // | Shipping Address   Barack Obama              |
 // |                    1600 Pennsylvania Ave.  > |
 // |                    1800MYPOTUS               |
 // +----------------------------------------------+
 std::unique_ptr<views::Button> PaymentSheetViewController::CreateShippingRow() {
-  std::unique_ptr<views::Button> section = CreatePaymentSheetRow(
-      this, GetShippingAddressSectionString(spec()->shipping_type()),
-      CreateShippingSectionContent(), std::unique_ptr<views::View>(nullptr),
-      widest_name_column_view_width_);
-  section->set_tag(
-      static_cast<int>(PaymentSheetViewControllerTags::SHOW_SHIPPING_BUTTON));
-  section->set_id(
-      static_cast<int>(DialogViewID::PAYMENT_SHEET_SHIPPING_ADDRESS_SECTION));
+  std::unique_ptr<views::Button> section;
+  if (state()->selected_shipping_profile()) {
+    section = CreatePaymentSheetRowWithChevron(
+        this, GetShippingAddressSectionString(spec()->shipping_type()),
+        CreateShippingSectionContent(), std::unique_ptr<views::View>(nullptr),

[Mathieu, 2017/04/13 00:12:45]

I'm curious, we're the only ones that do std::unique_ptr<views::View>(nullptr)
in the codebase. I see a lot more of base::MakeUnique<...>(nullptr), can we do
that?

[anthonyvd, 2017/04/13 18:33:33]

I think just nullptr is fine and we have that in other places. Let's go with
that everywhere since it's the shorter form (and nullptr doesn't change
ownership so the semantics of unique_ptr are irrelevant).

+        widest_name_column_view_width_);
+    section->set_tag(
+        static_cast<int>(PaymentSheetViewControllerTags::SHOW_SHIPPING_BUTTON));
+    section->set_id(
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_SHIPPING_ADDRESS_SECTION));
+  } else {
+    base::string16 button_string =
+        state()->shipping_profiles().size()
+            ? l10n_util::GetStringUTF16(
+                  IDS_PAYMENT_REQUEST_CHOOSE_SECTION_BUTTON)
+            : l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_ADD_SECTION_BUTTON);
+
+    section = CreatePaymentSheetRowWithButton(
+        this, GetShippingAddressSectionString(spec()->shipping_type()),
+        base::ASCIIToUTF16(""), button_string,
+        static_cast<int>(PaymentSheetViewControllerTags::SHOW_SHIPPING_BUTTON),
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_SHIPPING_ADDRESS_SECTION),

[Mathieu, 2017/04/13 00:12:45]

I think we should create a different DialogViewID for buttons, so that we can
test the proper interactions

[anthonyvd, 2017/04/13 18:33:33]

Done.

+        widest_name_column_view_width_);
+  }
+
   return section;
 }
 
 // Creates the Payment Method row, which contains a "Payment" label, the user's
 // masked Credit Card details, the icon for the selected card, and a chevron.
 // +----------------------------------------------+
 // | Payment         Visa ****0000                |
 // |                 John Smith        | VISA | > |

[Mathieu, 2017/04/13 00:12:45]

update comment to mention button

[anthonyvd, 2017/04/13 18:33:33]

Done.

 // |                                              |
 // +----------------------------------------------+
 std::unique_ptr<views::Button>
 PaymentSheetViewController::CreatePaymentMethodRow() {
   PaymentInstrument* selected_instrument = state()->selected_instrument();
 
   std::unique_ptr<views::View> content_view;
   std::unique_ptr<views::ImageView> card_icon_view;
+  std::unique_ptr<views::Button> section;
   if (selected_instrument) {
     content_view = base::MakeUnique<views::View>();
 
     views::GridLayout* layout = new views::GridLayout(content_view.get());
     content_view->SetLayoutManager(layout);
     views::ColumnSet* columns = layout->AddColumnSet(0);
     columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
                        1, views::GridLayout::USE_PREF, 0, 0);
 
     layout->StartRow(0, 0);
     layout->AddView(new views::Label(selected_instrument->label()));
     layout->StartRow(0, 0);
     layout->AddView(new views::Label(selected_instrument->sublabel()));
 
     card_icon_view = CreateInstrumentIconView(
         selected_instrument->icon_resource_id(), selected_instrument->label());
     card_icon_view->SetImageSize(gfx::Size(32, 20));
+
+    section = CreatePaymentSheetRowWithChevron(
+        this,
+        l10n_util::GetStringUTF16(
+            IDS_PAYMENT_REQUEST_PAYMENT_METHOD_SECTION_NAME),
+        std::move(content_view), std::move(card_icon_view),
+        widest_name_column_view_width_);
+    section->set_tag(static_cast<int>(
+        PaymentSheetViewControllerTags::SHOW_PAYMENT_METHOD_BUTTON));
+    section->set_id(
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_PAYMENT_METHOD_SECTION));
+  } else {
+    base::string16 button_string =
+        state()->available_instruments().size()
+            ? l10n_util::GetStringUTF16(
+                  IDS_PAYMENT_REQUEST_CHOOSE_SECTION_BUTTON)
+            : l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_ADD_SECTION_BUTTON);
+
+    section = CreatePaymentSheetRowWithButton(
+        this,
+        l10n_util::GetStringUTF16(
+            IDS_PAYMENT_REQUEST_PAYMENT_METHOD_SECTION_NAME),
+        base::ASCIIToUTF16(""), button_string,
+        static_cast<int>(
+            PaymentSheetViewControllerTags::SHOW_PAYMENT_METHOD_BUTTON),
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_PAYMENT_METHOD_SECTION),
+        widest_name_column_view_width_);
   }
 
-  std::unique_ptr<views::Button> section = CreatePaymentSheetRow(
-      this,
-      l10n_util::GetStringUTF16(
-          IDS_PAYMENT_REQUEST_PAYMENT_METHOD_SECTION_NAME),
-      std::move(content_view),
-      std::move(card_icon_view),
-      widest_name_column_view_width_);
-  section->set_tag(static_cast<int>(
-      PaymentSheetViewControllerTags::SHOW_PAYMENT_METHOD_BUTTON));
-  section->set_id(
-      static_cast<int>(DialogViewID::PAYMENT_SHEET_PAYMENT_METHOD_SECTION));
   return section;
 }
 
 std::unique_ptr<views::View>
 PaymentSheetViewController::CreateContactInfoSectionContent() {
   autofill::AutofillProfile* profile = state()->selected_contact_profile();
   return profile ? payments::GetContactInfoLabel(
                        AddressStyleType::SUMMARY,
                        state()->GetApplicationLocale(), *profile, *spec())
                  : base::MakeUnique<views::Label>(base::string16());
 }
 
 // Creates the Contact Info row, which contains a "Contact info" label; the
 // name, email address, and/or phone number; and a chevron.
 // +----------------------------------------------+
 // | Contact info       Barack Obama              |
 // |                    1800MYPOTUS             > |
 // |                    potus@whitehouse.gov      |
 // +----------------------------------------------+
 std::unique_ptr<views::Button>
 PaymentSheetViewController::CreateContactInfoRow() {
-  std::unique_ptr<views::Button> section = CreatePaymentSheetRow(
-      this,
-      l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_CONTACT_INFO_SECTION_NAME),
-      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));
+  std::unique_ptr<views::Button> section;
+  if (state()->selected_contact_profile()) {
+    section = CreatePaymentSheetRowWithChevron(
+        this,
+        l10n_util::GetStringUTF16(
+            IDS_PAYMENT_REQUEST_CONTACT_INFO_SECTION_NAME),
+        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));
+  } else {
+    base::string16 button_string =
+        state()->contact_profiles().size()
+            ? l10n_util::GetStringUTF16(
+                  IDS_PAYMENT_REQUEST_CHOOSE_SECTION_BUTTON)
+            : l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_ADD_SECTION_BUTTON);
+
+    section = CreatePaymentSheetRowWithButton(
+        this,
+        l10n_util::GetStringUTF16(
+            IDS_PAYMENT_REQUEST_CONTACT_INFO_SECTION_NAME),
+        base::ASCIIToUTF16(""), button_string,
+        static_cast<int>(
+            PaymentSheetViewControllerTags::SHOW_CONTACT_INFO_BUTTON),
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_CONTACT_INFO_SECTION),
+        widest_name_column_view_width_);
+  }
+
   return section;
 }
 
 std::unique_ptr<views::Button>
 PaymentSheetViewController::CreateShippingOptionRow() {
   mojom::PaymentShippingOption* selected_option =
       spec()->selected_shipping_option();
-  if (!selected_option)
-    return nullptr;
 
-  std::unique_ptr<views::View> option_label = CreateShippingOptionLabel(
-      selected_option, selected_option ? spec()->GetFormattedCurrencyAmount(
-                                             selected_option->amount->value)
-                                       : base::ASCIIToUTF16(""));
-  std::unique_ptr<views::Button> section = CreatePaymentSheetRow(
-      this, GetShippingOptionSectionString(spec()->shipping_type()),
-      std::move(option_label), std::unique_ptr<views::View>(nullptr),
-      widest_name_column_view_width_);
-  section->set_tag(static_cast<int>(
-      PaymentSheetViewControllerTags::SHOW_SHIPPING_OPTION_BUTTON));
-  section->set_id(
-      static_cast<int>(DialogViewID::PAYMENT_SHEET_SHIPPING_OPTION_SECTION));
+  // The shipping option section displays the currently selected option if there
+  // is one. It's not possible to select an option without selecting an address
+  // first.
+  std::unique_ptr<views::Button> section;
+  if (state()->selected_shipping_profile()) {
+    if (spec()->details().shipping_options.size()) {

[Mathieu, 2017/04/13 00:12:45]

if (!... or .empty()

[anthonyvd, 2017/04/13 18:33:33]

Done.

+      // TODO(anthonyvd): Display placeholder if there's no available shipping
+      // option.
+      return nullptr;
+    }
+
+    std::unique_ptr<views::View> option_label = CreateShippingOptionLabel(
+        selected_option, selected_option ? spec()->GetFormattedCurrencyAmount(
+                                               selected_option->amount->value)
+                                         : base::ASCIIToUTF16(""));
+    section = CreatePaymentSheetRowWithChevron(
+        this, GetShippingOptionSectionString(spec()->shipping_type()),
+        std::move(option_label), std::unique_ptr<views::View>(nullptr),
+        widest_name_column_view_width_);
+    section->set_tag(static_cast<int>(
+        PaymentSheetViewControllerTags::SHOW_SHIPPING_OPTION_BUTTON));
+    section->set_id(
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_SHIPPING_OPTION_SECTION));
+  } else {
+    // TODO(anthonyvd): This should be a disabled row with a disabled button and
+    // some text to indicate that an address must be selected first.
+    section = CreatePaymentSheetRowWithButton(
+        this, GetShippingOptionSectionString(spec()->shipping_type()),
+        base::ASCIIToUTF16(""),
+        l10n_util::GetStringUTF16(IDS_PAYMENT_REQUEST_CHOOSE_SECTION_BUTTON),
+        static_cast<int>(
+            PaymentSheetViewControllerTags::SHOW_SHIPPING_OPTION_BUTTON),
+        static_cast<int>(DialogViewID::PAYMENT_SHEET_SHIPPING_OPTION_SECTION),
+        widest_name_column_view_width_);
+  }
+
   return section;
 }
 
 }  // namespace payments