[Payments] Have expiration date be on the same line in CC editor

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

 // Copyright 2017 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/credit_card_editor_view_controller.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 69 matching lines @@
   years.reserve(kNumberOfExpirationYears);
 
   base::Time::Exploded now_exploded;
   autofill::AutofillClock::Now().LocalExplode(&now_exploded);
   for (int i = 0; i < kNumberOfExpirationYears; i++) {
     years.push_back(base::UTF8ToUTF16(std::to_string(now_exploded.year + i)));
   }
   return years;
 }
 
+// Validates the two comboboxes used for expiration date.
+class ExpirationDateValidationDelegate : public ValidationDelegate {
+ public:
+  ExpirationDateValidationDelegate(EditorViewController* controller,
+                                   const std::string& app_locale)
+      : controller_(controller), app_locale_(app_locale) {}
+
+  // Only the delegate knows how to validate the textfield.
+  bool ValidateTextfield(views::Textfield* textfield) override { return true; }
+
+  bool ValidateCombobox(views::Combobox* combobox) override {
+    // Get the combined date from the month and year dropdowns.
+    views::View* view_parent = combobox->parent();
+
+    views::Combobox* month_combobox = static_cast<views::Combobox*>(
+        view_parent->GetViewByID(autofill::CREDIT_CARD_EXP_MONTH));
+    base::string16 month =
+        month_combobox->model()->GetItemAt(month_combobox->selected_index());
+
+    views::Combobox* year_combobox = static_cast<views::Combobox*>(
+        view_parent->GetViewByID(autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR));
+    base::string16 year =
+        year_combobox->model()->GetItemAt(year_combobox->selected_index());
+
+    autofill::CreditCard card;
+    card.SetExpirationMonthFromString(month, app_locale_);
+    card.SetExpirationYearFromString(year);
+
+    bool is_expired = card.IsExpired(autofill::AutofillClock::Now());
+    controller_->DisplayErrorMessageForField(
+        autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR,
+        is_expired ? l10n_util::GetStringUTF16(
+                         IDS_PAYMENTS_VALIDATION_INVALID_CREDIT_CARD_EXPIRED)
+                   : base::string16());
+
+    month_combobox->SetInvalid(is_expired);
+    year_combobox->SetInvalid(is_expired);
+
+    return !is_expired;
+  }
+
+  void ComboboxModelChanged(views::Combobox* combobox) override {}
+
+ private:
+  EditorViewController* controller_;
+  const std::string& app_locale_;

[anthonyvd, 2017/05/19 19:16:59]

Maybe it doesn't matter in this case because the passed string outlives this
object but we shouldn't have reference type members. Copying a locale string
really shouldn't be an expensive operation anyways.

[Mathieu, 2017/05/19 21:13:46]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ExpirationDateValidationDelegate);
+};
+
 }  // namespace
 
 CreditCardEditorViewController::CreditCardEditorViewController(
     PaymentRequestSpec* spec,
     PaymentRequestState* state,
     PaymentRequestDialogView* dialog,
     BackNavigationType back_navigation,
     int next_ui_tag,
     base::OnceClosure on_edited,
     base::OnceCallback<void(const autofill::CreditCard&)> on_added,
@@ skipping 58 matching lines @@
     card_icon_view->SetImageSize(kCardIconSize);
 
     icons_row->AddChildView(card_icon_view.release());
   }
   view->AddChildView(icons_row.release());
 
   return view;
 }
 
 std::unique_ptr<views::View>
+CreditCardEditorViewController::CreateCustomFieldView(
+    autofill::ServerFieldType type) {
+  DCHECK(type == autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR);

[anthonyvd, 2017/05/19 19:16:59]

nit: DCHECK_EQ

[Mathieu, 2017/05/19 21:13:46]

Done.

+
+  std::unique_ptr<views::View> view = base::MakeUnique<views::View>();
+  std::unique_ptr<views::GridLayout> combobox_layout =
+      base::MakeUnique<views::GridLayout>(view.get());
+  // Column set for short fields.
+  views::ColumnSet* columns = combobox_layout->AddColumnSet(0);
+  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 1,
+                     views::GridLayout::USE_PREF, 0, 0);
+  // Space between the two comboboxes.
+  constexpr int kHorizontalSpacing = 8;
+  columns->AddPaddingColumn(0, kHorizontalSpacing);
+  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 1,
+                     views::GridLayout::USE_PREF, 0, 0);
+
+  combobox_layout->StartRow(0, 0);
+  constexpr int kInputFieldHeight = 28;
+  EditorField tmp_month{autofill::CREDIT_CARD_EXP_MONTH, base::string16(),
+                        EditorField::LengthHint::HINT_SHORT,
+                        /* required= */ true,

[anthonyvd, 2017/05/19 19:16:59]

/*required=*/true

[Mathieu, 2017/05/19 21:13:46]

Done.

+                        EditorField::ControlType::COMBOBOX};
+  std::unique_ptr<ValidatingCombobox> month_combobox =
+      CreateComboboxForField(tmp_month);
+  combobox_layout->AddView(month_combobox.release(), 1, 1,
+                           views::GridLayout::FILL, views::GridLayout::FILL, 0,
+                           kInputFieldHeight);
+
+  EditorField tmp_year{autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR, base::string16(),
+                       EditorField::LengthHint::HINT_SHORT,
+                       /* required= */ true,

[anthonyvd, 2017/05/19 19:16:59]

/*required=*/true

[Mathieu, 2017/05/19 21:13:46]

Done.

+                       EditorField::ControlType::COMBOBOX};
+  std::unique_ptr<ValidatingCombobox> year_combobox =
+      CreateComboboxForField(tmp_year);
+  combobox_layout->AddView(year_combobox.release(), 1, 1,
+                           views::GridLayout::FILL, views::GridLayout::FILL, 0,
+                           kInputFieldHeight);
+
+  view->SetLayoutManager(combobox_layout.release());
+  return view;
+}
+
+std::unique_ptr<views::View>
 CreditCardEditorViewController::CreateExtraViewForField(
     autofill::ServerFieldType type) {
   if (type != kBillingAddressType)
     return nullptr;
 
   std::unique_ptr<views::View> button_view = base::MakeUnique<views::View>();
   button_view->SetLayoutManager(new views::FillLayout);
 
   // The button to add new billing addresses.
   std::unique_ptr<views::Button> add_button(
       views::MdTextButton::Create(this, l10n_util::GetStringUTF16(IDS_ADD)));
   add_button->set_id(
       static_cast<int>(DialogViewID::ADD_BILLING_ADDRESS_BUTTON));
   add_button->set_tag(add_billing_address_button_tag_);
   button_view->AddChildView(add_button.release());
   return button_view;
 }
 
 std::vector<EditorField> CreditCardEditorViewController::GetFieldDefinitions() {
   return std::vector<EditorField>{
       {autofill::CREDIT_CARD_NUMBER,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_CREDIT_CARD_NUMBER),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true},
       {autofill::CREDIT_CARD_NAME_FULL,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_NAME_ON_CARD),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true},
-      {autofill::CREDIT_CARD_EXP_MONTH,
-       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_EXPIRATION_MONTH),
+      {autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR,
+       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_EXPIRATION_DATE),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true,
-       EditorField::ControlType::COMBOBOX},
-      {autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR,
-       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_EXPIRATION_YEAR),
-       EditorField::LengthHint::HINT_SHORT, /* required= */ true,
-       EditorField::ControlType::COMBOBOX},
+       EditorField::ControlType::CUSTOMFIELD},
       {kBillingAddressType,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_BILLING_ADDRESS),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true,
        EditorField::ControlType::COMBOBOX}};
 }
 
 base::string16 CreditCardEditorViewController::GetInitialValueForType(
     autofill::ServerFieldType type) {
   if (!credit_card_to_edit_ || type == kBillingAddressType)
     return base::string16();
@@ skipping 80 matching lines @@
   }
 
   return true;
 }
 
 std::unique_ptr<ValidationDelegate>
 CreditCardEditorViewController::CreateValidationDelegate(
     const EditorField& field) {
   // The supported card networks for non-cc-number types are not passed to avoid
   // the data copy in the delegate.
+  if (field.type == autofill::CREDIT_CARD_EXP_MONTH ||
+      field.type == autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR)
+    return base::MakeUnique<ExpirationDateValidationDelegate>(
+        this, state()->GetApplicationLocale());
   return base::MakeUnique<
       CreditCardEditorViewController::CreditCardValidationDelegate>(
       field, this,
       field.type == autofill::CREDIT_CARD_NUMBER
           ? spec()->supported_card_networks()
           : std::vector<std::string>());
 }
 
 std::unique_ptr<ui::ComboboxModel>
 CreditCardEditorViewController::GetComboboxModelForType(
@@ skipping 98 matching lines @@
 
 bool CreditCardEditorViewController::CreditCardValidationDelegate::
     ValidateValue(const base::string16& value) {
   if (!value.empty()) {
     base::string16 error_message;
     bool is_valid =
         field_.type == autofill::CREDIT_CARD_NUMBER
             ? autofill::IsValidCreditCardNumberForBasicCardNetworks(
                   value, supported_card_networks_, &error_message)
             : autofill::IsValidForType(value, field_.type, &error_message);
-    controller_->DisplayErrorMessageForField(field_, error_message);
+    controller_->DisplayErrorMessageForField(field_.type, error_message);
     return is_valid;
   }
 
   bool is_required_valid = !field_.required;
   const base::string16 displayed_message =
       is_required_valid ? base::ASCIIToUTF16("")
                         : l10n_util::GetStringUTF16(
                               IDS_PAYMENTS_FIELD_REQUIRED_VALIDATION_MESSAGE);
-  controller_->DisplayErrorMessageForField(field_, displayed_message);
+  controller_->DisplayErrorMessageForField(field_.type, displayed_message);
   return is_required_valid;
 }
 
 bool CreditCardEditorViewController::GetSheetId(DialogViewID* sheet_id) {
   *sheet_id = DialogViewID::CREDIT_CARD_EDITOR_SHEET;
   return true;
 }
 
 }  // namespace payments