Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(20)

Issue 2816083002: [WebPayments] Desktop implementation of Contact Editor (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by tmartino
Modified:
3 months, 3 weeks ago
Reviewers:
Mathieu, sebsg, anthonyvd
CC:
browser-components-watch_chromium.org, chromium-reviews, estade+watch_chromium.org, gogerald+paymentswatch_chromium.org, mahmadi+paymentswatch_chromium.org, mathp+autofillwatch_chromium.org, rogerm+autofillwatch_chromium.org, rouslan+payments_chromium.org, rouslan+autofill_chromium.org, sebsg+autofillwatch_chromium.org, sebsg+paymentswatch_chromium.org, tfarina, vabr+watchlistautofill_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[WebPayments] Desktop implementation of Contact Editor BUG=709535 Review-Url: https://codereview.chromium.org/2816083002 Cr-Commit-Position: refs/heads/master@{#466662} Committed: https://chromium.googlesource.com/chromium/src/+/27028d78159cff5447dd3f98c5da756c91c65658

Patch Set 1 #

Patch Set 2 : removing extraneous method #

Patch Set 3 : compile error + rebase #

Total comments: 27

Patch Set 4 : mathp comments #

Patch Set 5 : rebase #

Total comments: 6

Patch Set 6 : adding resource ids #

Patch Set 7 : adding tests #

Patch Set 8 : rebase 4/20 #

Patch Set 9 : update test js #

Patch Set 10 : fixing race condition in test #

Total comments: 2

Patch Set 11 : browsertest nits #

Patch Set 12 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+535 lines, -16 lines) Patch
M chrome/browser/ui/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/payments/contact_info_editor_view_controller.h View 1 2 3 1 chunk +78 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc View 1 2 3 1 chunk +196 lines, -0 lines 0 comments Download
A chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +178 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_browsertest_base.h View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_browsertest_base.cc View 1 2 3 4 5 6 7 8 3 chunks +26 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_dialog_view.h View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_dialog_view.cc View 1 2 3 4 2 chunks +12 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/payments/profile_list_view_controller.h View 2 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/ui/views/payments/profile_list_view_controller.cc View 6 chunks +10 lines, -10 lines 0 comments Download
M chrome/test/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/data/payments/contact_details.js View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/browser/validation.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -1 line 0 comments Download
M components/autofill/core/browser/validation.cc View 1 2 3 4 5 6 7 8 2 chunks +11 lines, -3 lines 0 comments Download
M components/payments_strings.grdp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +6 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 60 (48 generated)
tmartino
4 months ago (2017-04-13 20:21:20 UTC) #4
Mathieu
Thanks Tommy, love this very meticulous work. Small comments from me https://codereview.chromium.org/2816083002/diff/40001/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc File chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc (right): ...
4 months ago (2017-04-17 02:50:10 UTC) #12
sebsg
LGTM, from my very minimal knowledge of UI stuff :)
4 months ago (2017-04-18 14:38:22 UTC) #19
sebsg
Added a question/comment https://codereview.chromium.org/2816083002/diff/80001/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc File chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc (right): https://codereview.chromium.org/2816083002/diff/80001/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc#newcode153 chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc:153: autofill::AutofillCountry::CountryCodeForLocale(locale_); Do you think we could ...
4 months ago (2017-04-18 15:44:33 UTC) #22
anthonyvd
lgtm with a few questions. https://codereview.chromium.org/2816083002/diff/80001/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc File chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc (right): https://codereview.chromium.org/2816083002/diff/80001/chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc#newcode42 chrome/browser/ui/views/payments/contact_info_editor_view_controller.cc:42: if (spec()->request_payer_name()) { Quick ...
4 months ago (2017-04-18 15:55:43 UTC) #23
tmartino
Finally solved the weird build issue that was breaking tryjobs on old patchsets. The changes ...
4 months ago (2017-04-18 19:39:18 UTC) #26
Mathieu
Thanks, would love to see tests alongside the code.
4 months ago (2017-04-18 20:47:02 UTC) #29
tmartino
Added tests! I'm on my way out the door so I'll fix the patch failure ...
4 months ago (2017-04-19 22:44:12 UTC) #34
tmartino
There were some changes to the State class (becoming more strict about not showing if ...
3 months, 4 weeks ago (2017-04-20 17:51:22 UTC) #41
Mathieu
Very cool Tommy! lgtm with 2 nits https://codereview.chromium.org/2816083002/diff/180001/chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc File chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc (right): https://codereview.chromium.org/2816083002/diff/180001/chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc#newcode137 chrome/browser/ui/views/payments/contact_info_editor_view_controller_browsertest.cc:137: base::RunLoop populate_data_loop; ...
3 months, 3 weeks ago (2017-04-24 12:13:59 UTC) #46
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/2816083002/220001
3 months, 3 weeks ago (2017-04-24 16:51:53 UTC) #57
commit-bot: I haz the power
3 months, 3 weeks ago (2017-04-24 16:56:54 UTC) #60
Message was sent while issue was closed.
Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/27028d78159cff5447dd3f98c5da...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b