[Payments] Upload card UI now has a CVC prompt

chrome/browser/ui/views/autofill/save_card_bubble_views.cc

 // Copyright 2015 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/autofill/save_card_bubble_views.h"
 
 #include <stddef.h>
 
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
-#include "chrome/browser/ui/autofill/save_card_bubble_controller.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/legal_message_line.h"
+#include "components/autofill/core/browser/ui/save_card_bubble_controller.h"
 #include "components/strings/grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/border.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/blue_button.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/styled_label.h"
+#include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/layout_constants.h"
+#include "ui/views/window/dialog_client_view.h"
 
 namespace autofill {
 
 namespace {
 
 // Fixed width of the bubble.

[Evan Stade, 2017/04/07 02:14:39]

[1/2] this is in dip

[Jared Saul, 2017/04/08 01:05:36]

Acknowledged.

 const int kBubbleWidth = 395;
+// Fixed width of the CVC TextField.

[Evan Stade, 2017/04/07 02:14:39]

nit: TextField has odd capitalization

[Jared Saul, 2017/04/08 01:05:36]

Whoops you're right, it's "Textfield".  Thanks.

+const int kDefaultWidthForCvc = 8;

[Evan Stade, 2017/04/07 02:14:39]

[2/2] but this is in characters. This distinction should be documented. Although
I think it's even better to share this constant with the 8 in
card_unmask_prompt_view, or heck share the whole textfield init stanza.

[Jared Saul, 2017/04/08 01:05:36]

I'm all for sharing the entire textfield init stanza.  Where do you think it
should be placed?  (In the meantime, documented the distinction.)

[Evan Stade, 2017/04/10 18:32:09]

probably a static fn in chrome/browser/ui/views/autofill/view_util.h

[Jared Saul, 2017/04/11 00:53:06]

Done; created view_util.h, PTAL.

 
 std::unique_ptr<views::StyledLabel> CreateLegalMessageLineLabel(
     const LegalMessageLine& line,
     views::StyledLabelListener* listener) {
   std::unique_ptr<views::StyledLabel> label(
       new views::StyledLabel(line.text(), listener));
   for (const LegalMessageLine::Link& link : line.links()) {
     label->AddStyleRange(link.range,
                          views::StyledLabel::RangeStyleInfo::CreateForLink());
   }
   return label;
 }
 
 }  // namespace
 
 SaveCardBubbleViews::SaveCardBubbleViews(views::View* anchor_view,
                                          content::WebContents* web_contents,
                                          SaveCardBubbleController* controller)
     : LocationBarBubbleDelegateView(anchor_view, web_contents),
       controller_(controller),
+      cvc_textfield_(nullptr),
       learn_more_link_(nullptr) {
   DCHECK(controller);
   views::BubbleDialogDelegateView::CreateBubble(this);
 }
 
 SaveCardBubbleViews::~SaveCardBubbleViews() {}
 
 void SaveCardBubbleViews::Show(DisplayReason reason) {
   ShowForReason(reason);
 }
@@ skipping 21 matching lines @@
       new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
 
   // Add a StyledLabel for each line of the legal message.
   for (const LegalMessageLine& line : controller_->GetLegalMessageLines())
     view->AddChildView(CreateLegalMessageLineLabel(line, this).release());
 
   return view;
 }
 
 bool SaveCardBubbleViews::Accept() {
-  if (controller_)
+  if (controller_) {
+    if (controller_->ShouldRequestCvcFromUser()) {

[Evan Stade, 2017/04/07 02:14:39]

nit: no curlies

also I would slightly prefer this to check the null-ness of cvc_textfield_
instead of asking the controller again.

[Jared Saul, 2017/04/08 01:05:36]

Done and done.

+      controller_->SetCvcEnteredByUser(cvc_textfield_->text());
+    }
     controller_->OnSaveButton();
+  }
   return true;
 }
 
 bool SaveCardBubbleViews::Cancel() {
   if (controller_)
     controller_->OnCancelButton();
   return true;
 }
 
 bool SaveCardBubbleViews::Close() {
@@ skipping 87 matching lines @@
   card_type_icon->SetTooltipText(card.TypeForDisplay());
   card_type_icon->SetBorder(
       views::CreateSolidBorder(1, SkColorSetA(SK_ColorBLACK, 10)));
   description_view->AddChildView(card_type_icon);
 
   description_view->AddChildView(new views::Label(
       base::string16(kMidlineEllipsis) + card.LastFourDigits()));
   description_view->AddChildView(
       new views::Label(card.AbbreviatedExpirationDateForDisplay()));
 
+  // Optionally add CVC request field if CVC was missing.
+  if (controller_->ShouldRequestCvcFromUser()) {

[Evan Stade, 2017/04/07 02:14:39]

nit: no curlies

[Jared Saul, 2017/04/08 01:05:36]

Done.

+    view->AddChildView(CreateRequestCvcView().release());
+  }
+
   // Optionally add label that will contain an explanation for upload.
   base::string16 explanation = controller_->GetExplanatoryMessage();
   if (!explanation.empty()) {
     views::Label* explanation_label = new views::Label(explanation);
     explanation_label->SetMultiLine(true);
     explanation_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     view->AddChildView(explanation_label);
   }
 
   return view;
 }
 
+std::unique_ptr<views::View> SaveCardBubbleViews::CreateRequestCvcView() {
+  std::unique_ptr<View> request_cvc_view = base::MakeUnique<views::View>();
+  request_cvc_view->SetLayoutManager(
+      base::MakeUnique<views::BoxLayout>(views::BoxLayout::kHorizontal, 0, 0,
+                                         views::kRelatedButtonHSpacing)
+          .release());
+
+  DCHECK(!cvc_textfield_);
+  cvc_textfield_ = new views::Textfield();

[Evan Stade, 2017/04/07 02:14:39]

aside: I wish we had a way to set the limit on number of characters input

[Jared Saul, 2017/04/08 01:05:36]

Acknowledged (agreed).

+  cvc_textfield_->set_placeholder_text(
+      l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_PLACEHOLDER_CVC));
+  cvc_textfield_->set_controller(this);
+  cvc_textfield_->set_default_width_in_chars(kDefaultWidthForCvc);
+  request_cvc_view->AddChildView(cvc_textfield_);
+
+  std::unique_ptr<views::ImageView> cvc_image =

[Evan Stade, 2017/04/07 02:14:39]

nit: we don't typically use unique_ptr for views like this

[Jared Saul, 2017/04/08 01:05:36]

Done, changed to match card_unmask_prompt_views.cc.

+      base::MakeUnique<views::ImageView>();
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  cvc_image->SetImage(
+      rb.GetImageSkiaNamed(controller_->GetCvcImageResourceId()));
+  request_cvc_view->AddChildView(cvc_image.release());
+
+  request_cvc_view->AddChildView(new views::Label(
+      l10n_util::GetStringUTF16(IDS_AUTOFILL_SAVE_CARD_PROMPT_ENTER_CVC)));
+  return request_cvc_view;
+}
+
+bool SaveCardBubbleViews::IsDialogButtonEnabled(ui::DialogButton button) const {
+  if (button == ui::DIALOG_BUTTON_CANCEL)
+    return true;
+
+  DCHECK_EQ(ui::DIALOG_BUTTON_OK, button);
+  return !cvc_textfield_ ||
+         controller_->InputCvcIsValid(cvc_textfield_->text());

[Evan Stade, 2017/04/07 02:14:39]

for future consideration: it would be nice if there were some indication the
input cvc were invalid when users can't figure out why the ok button is disabled

[Jared Saul, 2017/04/08 01:05:36]

csashi@ said the same thing, both for this and for card_unmask_prompt_view. 
It's something we should definitely consider for an upcoming improvement.  Just
spitballing ideas here--any idea if there's precedent for disabled buttons to
have tooltip text explaining why they're disabled?

[Evan Stade, 2017/04/10 18:32:09]

https://cs.chromium.org/chromium/src/chrome/browser/ui/views/autofill/card_un...

[Jared Saul, 2017/04/11 00:53:06]

Acknowledged.  Though for the record, that happens *after* the CVC is entered
and the button is clicked (and auth fails).  Looks like that view still doesn't
have anything beforehand that implies CVC is what's preventing the button from
being clicked, hmm.

[Evan Stade, 2017/04/11 15:28:05]

yes. Since this CVC validity check is local and not done on the server you can
do the check whenever you feel is appropriate --- like when CVC loses focus, or
when the user presses "enter" or hovers the mouse over the button or something.

[Jared Saul, 2017/04/11 22:57:47]

Acknowledged.  Wrote myself a note to look into this for here and the unmask
view in the future; I like the loses-focus-or-user-presses-enter idea.

+}
+
+void SaveCardBubbleViews::ContentsChanged(views::Textfield* sender,
+                                          const base::string16& new_contents) {
+  GetDialogClientView()->UpdateDialogButtons();
+}
+
 void SaveCardBubbleViews::Init() {
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
   AddChildView(CreateMainContentView().release());
 }
 
 }  // namespace autofill