Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(528)

Issue 2895473005: [Payments] Have expiration date be on the same line in CC editor (Closed)

Created:
3 years, 7 months ago by Mathieu
Modified:
3 years, 7 months ago
Reviewers:
anthonyvd
CC:
chromium-reviews, tfarina
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Payments] Have expiration date be on the same line in CC editor Fields are now validated together and will appear invalid at the same time if the date is invalid. Requires creating a special validation delegate. BUG=724584 TEST=browser_tests Review-Url: https://codereview.chromium.org/2895473005 Cr-Commit-Position: refs/heads/master@{#473607} Committed: https://chromium.googlesource.com/chromium/src/+/af7b1e7969eb07a91bc75accb5e9af2daffd4790

Patch Set 1 : Initial #

Total comments: 12

Patch Set 2 : addressed comments #

Total comments: 6

Patch Set 3 : addressed comments + big rebase #

Patch Set 4 : addressed comments + rebase #

Total comments: 8

Patch Set 5 : addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+281 lines, -121 lines) Patch
M chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/payments/credit_card_editor_view_controller.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc View 1 2 3 4 10 chunks +137 lines, -17 lines 0 comments Download
M chrome/browser/ui/views/payments/credit_card_editor_view_controller_browsertest.cc View 1 2 5 chunks +14 lines, -10 lines 0 comments Download
M chrome/browser/ui/views/payments/editor_view_controller.h View 1 2 4 chunks +15 lines, -12 lines 0 comments Download
M chrome/browser/ui/views/payments/editor_view_controller.cc View 1 2 8 chunks +41 lines, -24 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_browsertest_base.cc View 1 2 7 chunks +22 lines, -14 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h View 1 2 3 4 2 chunks +8 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc View 1 2 7 chunks +11 lines, -9 lines 0 comments Download
M chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc View 1 2 9 chunks +25 lines, -19 lines 0 comments Download
M chrome/browser/ui/views/payments/validating_combobox.h View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/payments/validating_combobox.cc View 1 2 2 chunks +3 lines, -10 lines 0 comments Download

Messages

Total messages: 27 (18 generated)
Mathieu
PTAL
3 years, 7 months ago (2017-05-19 19:01:26 UTC) #2
anthonyvd
https://codereview.chromium.org/2895473005/diff/20001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc (right): https://codereview.chromium.org/2895473005/diff/20001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc#newcode135 chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:135: const std::string& app_locale_; Maybe it doesn't matter in this ...
3 years, 7 months ago (2017-05-19 19:17:00 UTC) #4
Mathieu
PTAL! https://codereview.chromium.org/2895473005/diff/20001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc (right): https://codereview.chromium.org/2895473005/diff/20001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc#newcode135 chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:135: const std::string& app_locale_; On 2017/05/19 19:16:59, anthonyvd wrote: ...
3 years, 7 months ago (2017-05-19 21:13:46 UTC) #5
anthonyvd
https://codereview.chromium.org/2895473005/diff/40001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc (right): https://codereview.chromium.org/2895473005/diff/40001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc#newcode105 chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:105: view_parent->GetViewByID(autofill::CREDIT_CARD_EXP_MONTH)); Does this break? You're not adding the offset? ...
3 years, 7 months ago (2017-05-19 21:27:01 UTC) #10
Mathieu
Sorry it was too late to not include MAD's change in the rebase. Your comments ...
3 years, 7 months ago (2017-05-20 01:20:26 UTC) #13
anthonyvd
lgtm w/ few small comments. https://codereview.chromium.org/2895473005/diff/80001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc (right): https://codereview.chromium.org/2895473005/diff/80001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc#newcode106 chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:106: bool IsValidTextfield(views::Textfield* textfield) override ...
3 years, 7 months ago (2017-05-22 13:18:18 UTC) #20
Mathieu
Thanks https://codereview.chromium.org/2895473005/diff/80001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc File chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc (right): https://codereview.chromium.org/2895473005/diff/80001/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc#newcode106 chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:106: bool IsValidTextfield(views::Textfield* textfield) override { return true; } ...
3 years, 7 months ago (2017-05-22 16:30:40 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2895473005/100001
3 years, 7 months ago (2017-05-22 16:31:16 UTC) #24
commit-bot: I haz the power
3 years, 7 months ago (2017-05-22 17:15:57 UTC) #27
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/af7b1e7969eb07a91bc75accb5e9...

Powered by Google App Engine
This is Rietveld 408576698