Skip to content

Commit

Permalink
[Autofill] Prompt before removing a local credit card.
Browse files Browse the repository at this point in the history
This CL adds a confirmation dialog to confirm a user's intent to remove
a locally stored credit card in settings. This aligns the experience for
removing local credit cards with that of addresses.

The CL also changes the word 'Remove' to 'Delete', in line with the
same change made to addresses in https://crrev.com/a5094829015

It was adapted from initial work done by schwering@ in
https://chromium-review.googlesource.com/c/chromium/src/+/3501833

Bug: 1301287
Change-Id: I294ce7faaa09265cefeff6de79d685385321cba9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4219354
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Christoph Schwering <schwering@google.com>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1103276}
  • Loading branch information
stephenmcgruer authored and Chromium LUCI CQ committed Feb 9, 2023
1 parent 37526a7 commit 4643d3c
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 15 deletions.
9 changes: 6 additions & 3 deletions chrome/app/settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -396,15 +396,18 @@
<message name="IDS_SETTINGS_ADDRESS_REMOVED_MESSAGE" desc="Hidden text that is read to screen readers to confirm that an address was removed.">
Address deleted
</message>
<message name="IDS_SETTINGS_CREDIT_CARD_REMOVE" desc="Label for a context menu item that removes the selected credit card." meaning="Remove selected credit card.">
Remove
</message>
<message name="IDS_SETTINGS_CREDIT_CARD_CLEAR" desc="Label for a context menu item clears the locally cached credit card that is also saved on Google Payments. Clicking this will NOT remove the credit card from Google Payments.">
Clear copy
</message>
<message name="IDS_SETTINGS_EDIT_CREDIT_CARD_TITLE" desc="The title for the dialog that's shown when editing a card. This can be either credit, debit, or prepaid card..">
Edit card
</message>
<message name="IDS_SETTINGS_LOCAL_CARD_REMOVE_CONFIRMATION_TITLE" desc="Local card remove dialog confirmation title.">
Delete card
</message>
<message name="IDS_SETTINGS_LOCAL_CARD_REMOVE_CONFIRMATION_DESCRIPTION" desc="The notice/warning for the user before the card deletion.">
This payment method will be deleted from this device
</message>
<message name="IDS_SETTINGS_PAYMENTS_MANAGE_CREDIT_CARDS" desc="Shown in the payments section of settings. Descriptive text to inform the user that credit cards can be accessed online. Has a link.">
To add or manage Google Pay payment methods, visit your <ph name="BEGIN_LINK">&lt;a href="$1" target="_blank"&gt;</ph>Google Account<ph name="END_LINK">&lt;/a&gt;</ph>
</message>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1bf450d14bc37b72be4a5440c891c4fbec2b24d5
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d09b299f0af1dd52ab535d74ab0f9f9a4534c491
1 change: 1 addition & 0 deletions chrome/browser/resources/settings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ build_webui("build") {
"autofill_page/credit_card_list_entry.ts",
"autofill_page/iban_edit_dialog.ts",
"autofill_page/iban_list_entry.ts",
"autofill_page/local_credit_card_remove_confirmation_dialog.ts",
"autofill_page/password_check_edit_disclaimer_dialog.ts",
"autofill_page/password_check_list_item.ts",
"autofill_page/password_check.ts",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<cr-dialog show-on-attach id="dialog" close-text="$i18n{close}">
<div slot="title">$i18n{removeLocalCreditCardConfirmationTitle}</div>
<div slot="body">$i18n{removeLocalCreditCardConfirmationDescription}</div>
<div slot="button-container">
<cr-button class="cancel-button" on-click="onCancelClick_" id="cancel">
$i18n{cancel}
</cr-button>
<cr-button class="action-button" on-click="onRemoveClick_" id="remove">
$i18n{delete}
</cr-button>
</div>
</cr-dialog>
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

/**
* @fileoverview 'local-credit-card-remove-confirmation-dialog' is the dialog
* that allows removing a locally saved credit card.
*/
import 'chrome://resources/cr_elements/cr_button/cr_button.js';
import 'chrome://resources/cr_elements/cr_dialog/cr_dialog.js';

import {CrDialogElement} from 'chrome://resources/cr_elements/cr_dialog/cr_dialog.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {getTemplate} from './local_credit_card_remove_confirmation_dialog.html.js';


export interface SettingsLocalCreditCardRemoveConfirmationDialogElement {
$: {
dialog: CrDialogElement,
remove: HTMLElement,
cancel: HTMLElement,
};
}

export class SettingsLocalCreditCardRemoveConfirmationDialogElement extends
PolymerElement {
static get is() {
return 'settings-local-credit-card-remove-confirmation-dialog';
}

static get template() {
return getTemplate();
}

wasConfirmed(): boolean {
return this.$.dialog.getNative().returnValue === 'success';
}

private onRemoveClick_() {
this.$.dialog.close();
}

private onCancelClick_() {
this.$.dialog.cancel();
}
}

declare global {
interface HTMLElementTagNameMap {
'settings-local-credit-card-remove-confirmation-dialog':
SettingsLocalCreditCardRemoveConfirmationDialogElement;
}
}

customElements.define(
SettingsLocalCreditCardRemoveConfirmationDialogElement.is,
SettingsLocalCreditCardRemoveConfirmationDialogElement);
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ <h2 class="flex">$i18n{creditCards}</h2>

<button id="menuRemoveCreditCard" class="dropdown-item"
hidden$="[[!activeCreditCard_.metadata.isLocal]]"
on-click="onMenuRemoveCreditCardClick_">$i18n{removeCreditCard}</button>
on-click="onMenuRemoveCreditCardClick_">$i18n{delete}</button>
<button id="menuClearCreditCard" class="dropdown-item"
on-click="onMenuClearCreditCardClick_"
hidden$="[[!activeCreditCard_.metadata.isCached]]">
Expand Down Expand Up @@ -174,3 +174,10 @@ <h2 class="flex">$i18n{creditCards}</h2>
on-close="onVirtualCardUnenrollDialogClose_">
</settings-virtual-card-unenroll-dialog>
</template>

<template is="dom-if" if="[[showLocalCreditCardRemoveConfirmationDialog_]]"
restamp>
<settings-local-credit-card-remove-confirmation-dialog
on-close="onLocalCreditCardRemoveConfirmationDialogClose_">
</setting-local-credit-card-remove-confirmation-dialog>
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import '../controls/settings_toggle_button.js';
import '../prefs/prefs.js';
import './credit_card_edit_dialog.js';
import './iban_edit_dialog.js';
import './local_credit_card_remove_confirmation_dialog.js';
import './passwords_shared.css.js';
import './payments_list.js';
import './virtual_card_unenroll_dialog.js';
Expand Down Expand Up @@ -140,6 +141,7 @@ export class SettingsPaymentsSectionElement extends

showCreditCardDialog_: Boolean,
showIbanDialog_: Boolean,
showLocalCreditCardRemoveConfirmationDialog_: Boolean,
showVirtualCardUnenrollDialog_: Boolean,
migratableCreditCardsInfo_: String,

Expand Down Expand Up @@ -189,6 +191,7 @@ export class SettingsPaymentsSectionElement extends
private activeIban_: chrome.autofillPrivate.IbanEntry|null;
private showCreditCardDialog_: boolean;
private showIbanDialog_: boolean;
private showLocalCreditCardRemoveConfirmationDialog_: boolean;
private showVirtualCardUnenrollDialog_: boolean;
private migratableCreditCardsInfo_: string;
private migrationEnabled_: boolean;
Expand Down Expand Up @@ -410,13 +413,31 @@ export class SettingsPaymentsSectionElement extends
window.open(loadTimeData.getString('manageCreditCardsUrl'));
}

private onLocalCreditCardRemoveConfirmationDialogClose_() {
// Only remove the credit card entry if the user closed the dialog via the
// confirmation button (instead of cancel or close).
const confirmationDialog = this.shadowRoot!.querySelector(
'settings-local-credit-card-remove-confirmation-dialog');
assert(confirmationDialog);
if (confirmationDialog.wasConfirmed()) {
assert(this.activeCreditCard_);
assert(this.activeCreditCard_.guid);
this.paymentsManager_.removeCreditCard(this.activeCreditCard_.guid);
this.activeCreditCard_ = null;
}

this.showLocalCreditCardRemoveConfirmationDialog_ = false;
assert(this.activeDialogAnchor_);
focusWithoutInk(this.activeDialogAnchor_);
this.activeDialogAnchor_ = null;
}

/**
* Handles clicking on the "Remove" credit card button.
*/
private onMenuRemoveCreditCardClick_() {
this.paymentsManager_.removeCreditCard(this.activeCreditCard_!.guid!);
this.showLocalCreditCardRemoveConfirmationDialog_ = true;
this.$.creditCardSharedMenu.close();
this.activeCreditCard_ = null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -996,8 +996,11 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_ADDRESS_REMOVE_CONFIRMATION_TITLE},
{"removeAddressConfirmationDescription",
IDS_AUTOFILL_DELETE_SYNC_ADDRESS_SOURCE_NOTICE},
{"removeLocalCreditCardConfirmationTitle",
IDS_SETTINGS_LOCAL_CARD_REMOVE_CONFIRMATION_TITLE},
{"removeLocalCreditCardConfirmationDescription",
IDS_SETTINGS_LOCAL_CARD_REMOVE_CONFIRMATION_DESCRIPTION},
{"addressRemovedMessage", IDS_SETTINGS_ADDRESS_REMOVED_MESSAGE},
{"removeCreditCard", IDS_SETTINGS_CREDIT_CARD_REMOVE},
{"clearCreditCard", IDS_SETTINGS_CREDIT_CARD_CLEAR},
{"creditCardType", IDS_SETTINGS_AUTOFILL_CREDIT_CARD_TYPE_COLUMN_LABEL},
{"creditCardExpiration", IDS_SETTINGS_CREDIT_CARD_EXPIRATION_DATE},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -613,19 +613,28 @@ export class TestPaymentsManager extends TestBrowserProxy implements
*/
assertExpectations(expected: PaymentsManagerExpectations) {
assertEquals(
expected.requestedCreditCards, this.getCallCount('getCreditCardList'));
expected.requestedCreditCards, this.getCallCount('getCreditCardList'),
'requestedCreditCards mismatch');
assertEquals(
expected.listeningCreditCards,
this.getCallCount('setPersonalDataManagerListener') -
this.getCallCount('removePersonalDataManagerListener'));
this.getCallCount('removePersonalDataManagerListener'),
'listeningCreditCards mismatch');
assertEquals(
expected.removedCreditCards, this.getCallCount('removeCreditCard'));
expected.removedCreditCards, this.getCallCount('removeCreditCard'),
'removedCreditCards mismatch');
assertEquals(
expected.clearedCachedCreditCards,
this.getCallCount('clearCachedCreditCard'));
this.getCallCount('clearCachedCreditCard'),
'clearedCachedCreditCards mismatch');
assertEquals(
expected.addedVirtualCards, this.getCallCount('addVirtualCard'),
'addedVirtualCards mismatch');
assertEquals(
expected.requestedIbans, this.getCallCount('getIbanList'),
'requestedIbans mismatch');
assertEquals(
expected.addedVirtualCards, this.getCallCount('addVirtualCard'));
assertEquals(expected.requestedIbans, this.getCallCount('getIbanList'));
assertEquals(expected.removedIbans, this.getCallCount('removeIban'));
expected.removedIbans, this.getCallCount('removeIban'),
'removedIbans mismatch');
}
}
63 changes: 62 additions & 1 deletion chrome/test/data/webui/settings/payments_section_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ suite('PaymentsSection', function() {
paymentsManager.assertExpectations(expectations);
});

test('verifyRemoveCreditCardClicked', async function() {
test('verifyRemoveLocalCreditCardDialogConfirmed', async function() {
const creditCard = createCreditCardEntry();

creditCard.metadata!.isLocal = true;
Expand All @@ -989,13 +989,74 @@ suite('PaymentsSection', function() {
section.$.menuRemoveCreditCard.click();
flush();

const confirmationDialog = section.shadowRoot!.querySelector(
'settings-local-credit-card-remove-confirmation-dialog');
assertTrue(!!confirmationDialog);
await whenAttributeIs(confirmationDialog.$.dialog, 'open', '');

const removeButton = confirmationDialog.$.remove;
assertTrue(!!removeButton);
removeButton.click();
flush();

await whenAttributeIs(confirmationDialog.$.dialog, 'open', null);

// Wait for the dialog close event to propagate to the PaymentManager.
await flushTasks();

const paymentsManager =
PaymentsManagerImpl.getInstance() as TestPaymentsManager;
const expectations = getDefaultExpectations();
expectations.removedCreditCards = 1;
paymentsManager.assertExpectations(expectations);
});

test('verifyRemoveLocalCreditCardDialogCancelled', async function() {
const creditCard = createCreditCardEntry();

creditCard.metadata!.isLocal = true;
creditCard.metadata!.isCached = false;
creditCard.metadata!.isVirtualCardEnrollmentEligible = false;
creditCard.metadata!.isVirtualCardEnrolled = false;

const section = await createPaymentsSection(
[creditCard], /*ibans=*/[], /*upiIds=*/[], /*prefValues=*/ {});
assertEquals(1, getLocalAndServerCreditCardListItems().length);

const rowShadowRoot = getCardRowShadowRoot(section.$.paymentsList);
assertFalse(!!rowShadowRoot.querySelector('#remoteCreditCardLink'));
const menuButton =
rowShadowRoot.querySelector<HTMLElement>('#creditCardMenu');
assertTrue(!!menuButton);
menuButton.click();
flush();

assertFalse(section.$.menuRemoveCreditCard.hidden);
section.$.menuRemoveCreditCard.click();
flush();

const confirmationDialog = section.shadowRoot!.querySelector(
'settings-local-credit-card-remove-confirmation-dialog');
assertTrue(!!confirmationDialog);
await whenAttributeIs(confirmationDialog.$.dialog, 'open', '');

const cancelButton = confirmationDialog.$.cancel;
assertTrue(!!cancelButton);
cancelButton.click();
flush();

await whenAttributeIs(confirmationDialog.$.dialog, 'open', null);

// Wait for the dialog close event to propagate to the PaymentManager.
await flushTasks();

const paymentsManager =
PaymentsManagerImpl.getInstance() as TestPaymentsManager;
const expectations = getDefaultExpectations();
expectations.removedCreditCards = 0;
paymentsManager.assertExpectations(expectations);
});

test('verifyAddVirtualCardClicked', async function() {
const creditCard = createCreditCardEntry();

Expand Down

0 comments on commit 4643d3c

Please sign in to comment.