Index: chrome/browser/ui/views/autofill/save_card_bubble_views.cc |
diff --git a/chrome/browser/ui/views/autofill/save_card_bubble_views.cc b/chrome/browser/ui/views/autofill/save_card_bubble_views.cc |
index 56b4018f702a2f715a871771167ce74207939e36..c16875e19b28539bde380322f169127da97c0d66 100644 |
--- a/chrome/browser/ui/views/autofill/save_card_bubble_views.cc |
+++ b/chrome/browser/ui/views/autofill/save_card_bubble_views.cc |
@@ -54,9 +54,7 @@ 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) { |
+ controller_(controller) { |
DCHECK(controller); |
views::BubbleDialogDelegateView::CreateBubble(this); |
chrome::RecordDialogCreation(chrome::DialogIdentifier::SAVE_CARD); |
@@ -98,6 +96,19 @@ views::View* SaveCardBubbleViews::CreateFootnoteView() { |
} |
bool SaveCardBubbleViews::Accept() { |
+ CHECK_LE(view_stack_->size(), static_cast<size_t>(2)); |
Evan Stade
2017/05/19 19:01:28
nit: 2U rather than casting
Evan Stade
2017/05/19 19:01:29
nit: make these DCHECKs
Jared Saul
2017/05/19 19:20:55
Done.
Jared Saul
2017/05/19 19:20:55
Done.
|
+ CHECK(view_stack_->size() == 1 || controller_->ShouldRequestCvcFromUser()); |
Evan Stade
2017/05/19 19:01:29
I don't think this check is super self-explanatory
Jared Saul
2017/05/19 19:20:55
Done; how's this?
|
+ if (controller_->ShouldRequestCvcFromUser() && view_stack_->size() == 1) { |
+ // If user accepted upload but more info is needed, push the next view onto |
+ // the stack. |
+ view_stack_->Push(CreateRequestCvcView(), /*animate=*/true); |
+ // Disable the Save button until a valid CVC is entered: |
+ GetDialogClientView()->UpdateDialogButtons(); |
+ // Resize the bubble if it's grown larger: |
+ SizeToContents(); |
+ return false; |
+ } |
+ // Otherwise, close the bubble as normal. |
if (controller_) |
controller_->OnSaveButton(cvc_textfield_ ? cvc_textfield_->text() |
: base::string16()); |
@@ -181,7 +192,8 @@ void SaveCardBubbleViews::StyledLabelLinkClicked(views::StyledLabel* label, |
// Create view containing everything except for the footnote. |
std::unique_ptr<views::View> SaveCardBubbleViews::CreateMainContentView() { |
- std::unique_ptr<View> view(new View()); |
+ std::unique_ptr<View> view = base::MakeUnique<views::View>(); |
Evan Stade
2017/05/19 22:18:16
nit: great place to use auto
Jared Saul
2017/05/23 18:53:02
Done; replaced the other two "std::unique_ptr<View
|
+ view->set_background(views::Background::CreateSolidBackground(SK_ColorWHITE)); |
view->SetLayoutManager( |
new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, |
views::kUnrelatedControlVerticalSpacing)); |
@@ -208,10 +220,6 @@ std::unique_ptr<views::View> SaveCardBubbleViews::CreateMainContentView() { |
description_view->AddChildView( |
new views::Label(card.AbbreviatedExpirationDateForDisplay())); |
- // Optionally add CVC request field if CVC was missing. |
- if (controller_->ShouldRequestCvcFromUser()) |
- view->AddChildView(CreateRequestCvcView().release()); |
- |
// Optionally add label that will contain an explanation for upload. |
base::string16 explanation = controller_->GetExplanatoryMessage(); |
if (!explanation.empty()) { |
@@ -226,22 +234,33 @@ std::unique_ptr<views::View> SaveCardBubbleViews::CreateMainContentView() { |
std::unique_ptr<views::View> SaveCardBubbleViews::CreateRequestCvcView() { |
std::unique_ptr<View> request_cvc_view = base::MakeUnique<views::View>(); |
- request_cvc_view->SetLayoutManager(new views::BoxLayout( |
+ request_cvc_view->set_background( |
+ views::Background::CreateSolidBackground(SK_ColorWHITE)); |
Evan Stade
2017/05/19 19:01:28
Why do you need to set this color? Why aren't we g
Jared Saul
2017/05/19 19:20:55
It's a limitation of ViewStack, see mathp@/anthony
Evan Stade
2017/05/19 22:18:17
What you need both here and there is CreateThemedS
Jared Saul
2017/05/23 18:53:03
Thanks for the tip; that worked! I also did have
|
+ request_cvc_view->SetLayoutManager( |
+ new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, |
+ views::kUnrelatedControlVerticalSpacing)); |
+ std::unique_ptr<View> inner_request_cvc_view = |
+ base::MakeUnique<views::View>(); |
+ inner_request_cvc_view->set_background( |
+ views::Background::CreateSolidBackground(SK_ColorWHITE)); |
+ inner_request_cvc_view->SetLayoutManager(new views::BoxLayout( |
views::BoxLayout::kHorizontal, 0, 0, views::kRelatedButtonHSpacing)); |
DCHECK(!cvc_textfield_); |
cvc_textfield_ = CreateCvcTextfield(); |
cvc_textfield_->set_controller(this); |
- request_cvc_view->AddChildView(cvc_textfield_); |
+ inner_request_cvc_view->AddChildView(cvc_textfield_); |
views::ImageView* cvc_image = new views::ImageView(); |
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); |
cvc_image->SetImage( |
rb.GetImageSkiaNamed(controller_->GetCvcImageResourceId())); |
- request_cvc_view->AddChildView(cvc_image); |
+ inner_request_cvc_view->AddChildView(cvc_image); |
- request_cvc_view->AddChildView(new views::Label( |
+ inner_request_cvc_view->AddChildView(new views::Label( |
l10n_util::GetStringUTF16(IDS_AUTOFILL_SAVE_CARD_PROMPT_ENTER_CVC))); |
+ |
+ request_cvc_view->AddChildView(inner_request_cvc_view.release()); |
return request_cvc_view; |
} |
@@ -262,7 +281,10 @@ void SaveCardBubbleViews::ContentsChanged(views::Textfield* sender, |
void SaveCardBubbleViews::Init() { |
SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0)); |
- AddChildView(CreateMainContentView().release()); |
+ view_stack_ = base::MakeUnique<ViewStack>(); |
+ view_stack_->set_owned_by_client(); // Required else it'll be deleted twice. |
Evan Stade
2017/05/19 19:01:28
are you sure this is required? Why not make view_s
Jared Saul
2017/05/19 19:20:55
See comment thread:
https://codereview.chromium.or
Evan Stade
2017/05/19 22:18:17
I don't understand how making this a raw pointer n
Jared Saul
2017/05/23 18:53:03
Ah, you're right. I believe those problems stemme
|
+ view_stack_->Push(CreateMainContentView(), /*animate=*/false); |
+ AddChildView(view_stack_.get()); |
} |
} // namespace autofill |