Issue 915173011: Add store-locally checkbox to Android card unmasking prompt.

Issue 915173011: Add store-locally checkbox to Android card unmasking prompt. (Closed)

Created:
5 years, 10 months ago by Evan Stade

Modified:
5 years, 10 months ago

Reviewers:
aruslan, newt (away)

CC:
chromium-reviews, estade+watch_chromium.org, browser-components-watch_chromium.org, rouslan+autofillwatch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Add store-locally checkbox to Android card unmasking prompt. Not hooked up to anything until https://codereview.chromium.org/910043002/ lands. BUG=437116 Committed: https://crrev.com/a965b7925558ba0f4f2321091040ab730f0b0ba3 Cr-Commit-Position: refs/heads/master@{#316343}

Patch Set 2 : capitalization fail #

Total comments: 8

Patch Set 4 : glue ui to pref #

Total comments: 2

Patch Set 5 : update string desc #

Created: 5 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+50 lines, -21 lines)			Patch
M	chrome/android/java/res/layout/autofill_card_unmask_prompt.xml	View	1 2	1 chunk	+9 lines, -0 lines	0 comments	Download
M	chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java	View	1 2	4 chunks	+9 lines, -7 lines	0 comments	Download
M	chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java	View	1 2	6 chunks	+13 lines, -3 lines	0 comments	Download
M	chrome/android/java/strings/android_chrome_strings.grd	View	1 2 3 4	1 chunk	+13 lines, -1 line	0 comments	Download
M	chrome/browser/ui/android/autofill/card_unmask_prompt_view_android.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/android/autofill/card_unmask_prompt_view_android.cc	View	1 2 3	2 chunks	+4 lines, -2 lines	0 comments	Download
M	ui/android/java/strings/android_ui_strings.grd	View		1 chunk	+0 lines, -7 lines	0 comments	Download

Messages

Total messages: 15 (5 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Evan Stade

+aruslan for chrome/browser/ui/android/autofill/card_unmask_prompt_view_android.* +newt for the rest

5 years, 10 months ago (2015-02-13 02:10:41 UTC) #2

newt (away)

lgtm modulo nits https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java File chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java (right): https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java#newcode26 chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java:26: boolean defaultToSavingLocally, WindowAndroid windowAndroid) { To ...

5 years, 10 months ago (2015-02-13 18:31:12 UTC) #3

Evan Stade

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java File chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java (right): https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java#newcode26 chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java:26: boolean defaultToSavingLocally, WindowAndroid windowAndroid) { On 2015/02/13 18:31:11, newt ...

5 years, 10 months ago (2015-02-13 22:55:12 UTC) #6

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java
(right):

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskBridge.java:26:
boolean defaultToSavingLocally, WindowAndroid windowAndroid) {
On 2015/02/13 18:31:11, newt wrote:
> To be consistent with terminology, I'd call this "defaultToStoringLocally"

oops, thanks for catching. I had intended to be consistent.

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/stri...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/stri...
chrome/android/java/strings/android_chrome_strings.grd:1029: <message desc="Text
for button that confirms the credit card CVC entry dialog."
name="IDS_CARD_UNMASK_CONFIRM_BUTTON">
On 2015/02/13 18:31:11, newt wrote:
> nit: put the name before the desc

Done.

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/stri...
chrome/android/java/strings/android_chrome_strings.grd:1035: <message
name="IDS_AUTOFILL_CARD_UNMASK_PROMPT_STORE_LOCALLY_CHECKBOX" desc="Checkbox
text that permits a user to skip past this dialog next time this card is used on
this device.">
On 2015/02/13 18:31:12, newt wrote:
> s/card/credit card

is it ok if I leave it at card for consistency with other strings and
classnames?

https://codereview.chromium.org/915173011/diff/20001/chrome/android/java/stri...
chrome/android/java/strings/android_chrome_strings.grd:1035: <message
name="IDS_AUTOFILL_CARD_UNMASK_PROMPT_STORE_LOCALLY_CHECKBOX" desc="Checkbox
text that permits a user to skip past this dialog next time this card is used on
this device.">
On 2015/02/13 18:31:11, newt wrote:
> maybe "IDS_CARD_UNMASK_PROMPT_STORE_LOCALLY"

removed _CHECKBOX

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/915173011/60001

5 years, 10 months ago (2015-02-13 23:19:23 UTC) #8

newt (away)

https://codereview.chromium.org/915173011/diff/60001/chrome/android/java/strings/android_chrome_strings.grd File chrome/android/java/strings/android_chrome_strings.grd (right): https://codereview.chromium.org/915173011/diff/60001/chrome/android/java/strings/android_chrome_strings.grd#newcode1104 chrome/android/java/strings/android_chrome_strings.grd:1104: <message name="IDS_AUTOFILL_CARD_UNMASK_PROMPT_STORE_LOCALLY" desc="Checkbox text that permits a user to ...

5 years, 10 months ago (2015-02-13 23:31:53 UTC) #9

Evan Stade

5 years, 10 months ago (2015-02-13 23:42:07 UTC) #11

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/915173011/80001

5 years, 10 months ago (2015-02-13 23:43:48 UTC) #13

commit-bot: I haz the power

5 years, 10 months ago (2015-02-14 00:33:56 UTC) #15

Message was sent while issue was closed.

Patchset 5 (id:??) landed as
https://crrev.com/a965b7925558ba0f4f2321091040ab730f0b0ba3
Cr-Commit-Position: refs/heads/master@{#316343}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages