Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2843533002:
[Web Payments] Add placeholders in non-selected payment sheet rows (Closed)
|
Created:
3 years, 8 months ago by anthonyvd Modified:
3 years, 8 months ago Reviewers:
Mathieu CC:
agrieve+watch_chromium.org, chromium-reviews, gogerald+paymentswatch_chromium.org, mahmadi+paymentswatch_chromium.org, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org, srahim+watch_chromium.org, tfarina Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[Web Payments] Add placeholders in non-selected payment sheet rows
BUG=710977
Review-Url: https://codereview.chromium.org/2843533002
Cr-Commit-Position: refs/heads/master@{#466805}
Committed: https://chromium.googlesource.com/chromium/src/+/bab199c9ccd8b873ac9d3c2e68ad79607887263c
Patch Set 1 #
Total comments: 8
Patch Set 2 : Fix Android strings. #Patch Set 3 : Address comments. #
Messages
Total messages: 23 (18 generated)
The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run
anthonyvd@chromium.org changed reviewers: + mathp@chromium.org
Hi Math, can you PTAL? Thanks!
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Creative solution, lgtm! https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:74: // |format_string| is a the string with format argument numbers in ICU syntax *is the https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:100: // TODO(anthonyvd): Display something meaningful if the preview can't be let's have a crbug for it :) https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:248: std::unique_ptr<views::Button> CreatePaymentSheetRowWithButton( CreatePaymentSheetRowWithButtonAndPreview ? https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:703: if (state()->available_instruments().size() == 0) { I like .empty()
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by anthonyvd@chromium.org to run a CQ dry run
https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... File chrome/browser/ui/views/payments/payment_sheet_view_controller.cc (right): https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:74: // |format_string| is a the string with format argument numbers in ICU syntax On 2017/04/24 at 19:09:01, Mathieu wrote: > *is the Done. https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:100: // TODO(anthonyvd): Display something meaningful if the preview can't be On 2017/04/24 at 19:09:01, Mathieu wrote: > let's have a crbug for it :) Done. https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:248: std::unique_ptr<views::Button> CreatePaymentSheetRowWithButton( On 2017/04/24 at 19:09:01, Mathieu wrote: > CreatePaymentSheetRowWithButtonAndPreview ? The other one has a preview as well, it's just not truncated the same way. I can't come up with a more appropriate name but added a comment about the difference between the 2 overloads. https://codereview.chromium.org/2843533002/diff/1/chrome/browser/ui/views/pay... chrome/browser/ui/views/payments/payment_sheet_view_controller.cc:703: if (state()->available_instruments().size() == 0) { On 2017/04/24 at 19:09:01, Mathieu wrote: > I like .empty() Done.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by anthonyvd@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org Link to the patchset: https://codereview.chromium.org/2843533002/#ps40001 (title: "Address comments.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493073854455980,
"parent_rev": "f8d6873f9acad2cb256ff35c501a8292f187104b", "commit_rev":
"a3f558e62880fad2db873cd051f6df7938ac2d70"}
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493073854455980,
"parent_rev": "276e666d9ba92c4f1932a98d53df1b005cd096f0", "commit_rev":
"bab199c9ccd8b873ac9d3c2e68ad79607887263c"}
Message was sent while issue was closed.
Description was changed from ========== [Web Payments] Add placeholders in non-selected payment sheet rows BUG=710977 ========== to ========== [Web Payments] Add placeholders in non-selected payment sheet rows BUG=710977 Review-Url: https://codereview.chromium.org/2843533002 Cr-Commit-Position: refs/heads/master@{#466805} Committed: https://chromium.googlesource.com/chromium/src/+/bab199c9ccd8b873ac9d3c2e68ad... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/bab199c9ccd8b873ac9d3c2e68ad... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
