Commit 421b8c44 authored by Sujie Zhu's avatar Sujie Zhu Committed by Chromium LUCI CQ

[Nickname Management] Clean up nickname management in settings UI

Clean up nicknameManagementEnabled flag and remove the legacy elements and
CSS config.

Local build test video after clean up(googlers only):
https://drive.google.com/file/d/1uYmVuIjlThiGl-TTeGvPrZWvDtkBJwKJ/view?usp=sharing&resourcekey=0-hMW3nVLEyQ7fbLzV5RHwqg

Bug: 1082013
Change-Id: Ie5ba1372cf510decb17a113c67ed5aee11eeb755
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2600180
Commit-Queue: Sujie Zhu <sujiezhu@google.com>
Reviewed-by: default avatarEsmael Elmoslimany <aee@chromium.org>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#839145}
parent 0852cc53
<style include="cr-shared-style settings-shared md-select"> <style include="cr-shared-style settings-shared md-select">
cr-input { cr-input {
--cr-input-error-display: none; /* Larger cr-input margin (by reserving space for error display). */
margin-bottom: var(--cr-form-field-bottom-spacing);
width: var(--settings-input-max-width);
}
/* Larger cr-input margin (by reserving space for error display) when
nickname management is enabled. */
:host([nickname-management-enabled_]) cr-input {
--cr-input-error-display: block; --cr-input-error-display: block;
margin-bottom: 0; margin-bottom: 0;
width: var(--settings-input-max-width);
} }
/* Override the padding-top (the space is set by save-to-this-device) when /* Override the padding-top (the space is set by save-to-this-device). */
nickname management is enabled. */ div[slot='button-container'] {
:host([nickname-management-enabled_]) div[slot='button-container'] {
padding-top: 0; padding-top: 0;
} }
...@@ -22,33 +15,11 @@ ...@@ -22,33 +15,11 @@
margin-inline-start: 8px; margin-inline-start: 8px;
} }
#expired {
align-items: center;
background-color: var(--paper-red-50);
color: var(--settings-error-color);
display: flex;
height: 40px;
margin-top: 12px;
padding: 0 0 0 8px;
}
@media (prefers-color-scheme: dark) {
#expired {
background-color: unset;
font-weight: bold;
padding: 0;
}
}
#month { #month {
width: 70px; width: 70px;
} }
#saved-to-this-device-only-label { #saved-to-this-device-only-label {
margin-top: var(--cr-form-field-bottom-spacing);
}
:host([nickname-management-enabled_]) #saved-to-this-device-only-label {
/* Overall space between input fields, including space between /* Overall space between input fields, including space between
nicknameInput and saved-to-this-device text, between nicknameInput and saved-to-this-device text, between
saved-to-this-device text and button. */ saved-to-this-device text and button. */
...@@ -91,13 +62,13 @@ ...@@ -91,13 +62,13 @@
} }
#expired-error, #expired-error,
:host([nickname-management-enabled_][expired_]) #expiration { :host([expired_]) #expiration {
color: var(--google-red-600); color: var(--google-red-600);
} }
@media (prefers-color-scheme: dark) { @media (prefers-color-scheme: dark) {
#expired-error, #expired-error,
:host([nickname-management-enabled_][expired_]) #expiration { :host([expired_]) #expiration {
color: var(--google-red-refresh-300); color: var(--google-red-refresh-300);
} }
} }
...@@ -105,12 +76,6 @@ ...@@ -105,12 +76,6 @@
<cr-dialog id="dialog" close-text="$i18n{close}"> <cr-dialog id="dialog" close-text="$i18n{close}">
<div slot="title">[[title_]]</div> <div slot="title">[[title_]]</div>
<div slot="body"> <div slot="body">
<!-- Place and autofocus the cardholder name as the first field when
nickname management is not enabled. -->
<cr-input id="legacyNameInput" label="$i18n{creditCardName}"
value="{{creditCard.name}}" spellcheck="false"
hidden$="[[nicknameManagementEnabled_]]" autofocus>
</cr-input>
<cr-input id="numberInput" label="$i18n{creditCardNumber}" <cr-input id="numberInput" label="$i18n{creditCardNumber}"
value="{{creditCard.cardNumber}}" autofocus> value="{{creditCard.cardNumber}}" autofocus>
</cr-input> </cr-input>
...@@ -136,33 +101,20 @@ ...@@ -136,33 +101,20 @@
<option>[[item]]</option> <option>[[item]]</option>
</template> </template>
</select> </select>
<!-- Use new error message text under the drop down when nickname <div id="expired-error">$i18n{creditCardExpired}</div>
management is enabled.--> <!-- Place cardholder name field and nickname field after expiration.-->
<div id="expired-error" hidden="[[!nicknameManagementEnabled_]]"> <cr-input id="nameInput" label="$i18n{creditCardName}"
$i18n{creditCardExpired} value="{{creditCard.name}}" spellcheck="false">
</div> </cr-input>
<!-- Reuse current error message span when nickname management is <cr-input id="nicknameInput" label="$i18n{creditCardNickname}"
disabled.--> value="{{creditCard.nickname}}" spellcheck="false" maxlength="25"
<span id="expired" hidden="[[!showLegacyExpiredError_(expired_, on-input="validateNickname_"
nicknameManagementEnabled_)]]"> invalid="[[nicknameInvalid_]]"
$i18n{creditCardExpired} error-message="$i18n{creditCardNicknameInvalid}">
</span> <div id="charCount" slot="suffix">
<!-- Place cardholder name field and nickname field after expiration [[computeNicknameCharCount_(creditCard.nickname)]]/25
when nickname management is enabled.--> </div>
<template is="dom-if" if="[[nicknameManagementEnabled_]]"> </cr-input>
<cr-input id="nameInput" label="$i18n{creditCardName}"
value="{{creditCard.name}}" spellcheck="false">
</cr-input>
<cr-input id="nicknameInput" label="$i18n{creditCardNickname}"
value="{{creditCard.nickname}}" spellcheck="false" maxlength="25"
on-input="validateNickname_"
invalid="[[nicknameInvalid_]]"
error-message="$i18n{creditCardNicknameInvalid}">
<div id="charCount" slot="suffix">
[[computeNicknameCharCount_(creditCard.nickname)]]/25
</div>
</cr-input>
</template>
<div id="saved-to-this-device-only-label"> <div id="saved-to-this-device-only-label">
$i18n{savedToThisDeviceOnly} $i18n{savedToThisDeviceOnly}
</div> </div>
......
...@@ -70,18 +70,6 @@ Polymer({ ...@@ -70,18 +70,6 @@ Polymer({
/** @private {string|undefined} */ /** @private {string|undefined} */
expirationMonth_: String, expirationMonth_: String,
/**
* True if nickname management is enabled.
* @private
*/
nicknameManagementEnabled_: {
type: Boolean,
reflectToAttribute: true,
value() {
return loadTimeData.getBoolean('nicknameManagementEnabled');
}
},
/** /**
* Whether the current nickname input is invalid. * Whether the current nickname input is invalid.
* @private * @private
...@@ -211,24 +199,12 @@ Polymer({ ...@@ -211,24 +199,12 @@ Polymer({
!this.expired_ && !this.nicknameInvalid_; !this.expired_ && !this.nicknameInvalid_;
}, },
/**
* @return {boolean} True iff the card is expired and nickname management is
* disabled.
* @private
*/
// TODO(crbug.com/1082013): Remove legacy expired error message when nickname
// management is fully enabled.
showLegacyExpiredError_() {
return !this.nicknameManagementEnabled_ && this.expired_;
},
/** /**
* Handles a11y error announcement the same way as in cr-input. * Handles a11y error announcement the same way as in cr-input.
* @private * @private
*/ */
onExpiredChanged_() { onExpiredChanged_() {
const ERROR_ID = const ERROR_ID = 'expired-error';
this.nicknameManagementEnabled_ ? 'expired-error' : 'expired';
const errorElement = this.$$(`#${ERROR_ID}`); const errorElement = this.$$(`#${ERROR_ID}`);
// Readding attributes is needed for consistent announcement by VoiceOver // Readding attributes is needed for consistent announcement by VoiceOver
if (this.expired_) { if (this.expired_) {
......
...@@ -1118,10 +1118,6 @@ void AddAutofillStrings(content::WebUIDataSource* html_source, ...@@ -1118,10 +1118,6 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
base::FeatureList::IsEnabled( base::FeatureList::IsEnabled(
autofill::features::kAutofillSaveAndFillVPA)); autofill::features::kAutofillSaveAndFillVPA));
html_source->AddBoolean(
"nicknameManagementEnabled",
base::FeatureList::IsEnabled(
autofill::features::kAutofillEnableCardNicknameManagement));
AddLocalizedStringsBulk(html_source, kLocalizedStrings); AddLocalizedStringsBulk(html_source, kLocalizedStrings);
} }
......
...@@ -105,28 +105,7 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -105,28 +105,7 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
return (new Date().getFullYear() - 1).toString(); return (new Date().getFullYear() - 1).toString();
} }
test('add card dialog when nickname management disabled', async function() { test('add card dialog', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: false});
const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open.
await whenAttributeIs(creditCardDialog.$$('#dialog'), 'open', '');
// Verify the nickname input field is not shown when nickname management is
// disabled.
assertFalse(!!creditCardDialog.$$('#nicknameInput'));
assertFalse(!!creditCardDialog.$$('#nameInput'));
// Legacy cardholder name input field is not hidden.
assertFalse(creditCardDialog.$$('#legacyNameInput').hidden);
// Verify the legacy cardholder name field is autofocused when nickname
// management is enabled.
assertTrue(
creditCardDialog.$$('#legacyNameInput').matches(':focus-within'));
});
test('add card dialog when nickname management is enabled', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCardDialog = createAddCreditCardDialog(); const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open. // Wait for the dialog to open.
...@@ -136,46 +115,13 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -136,46 +115,13 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
// enabled. // enabled.
assertTrue(!!creditCardDialog.$$('#nicknameInput')); assertTrue(!!creditCardDialog.$$('#nicknameInput'));
assertTrue(!!creditCardDialog.$$('#nameInput')); assertTrue(!!creditCardDialog.$$('#nameInput'));
// Legacy cardholder name input field is hidden.
assertTrue(creditCardDialog.$$('#legacyNameInput').hidden);
// Verify the card number field is autofocused when nickname management is // Verify the card number field is autofocused when nickname management is
// enabled. // enabled.
assertTrue(creditCardDialog.$$('#numberInput').matches(':focus-within')); assertTrue(creditCardDialog.$$('#numberInput').matches(':focus-within'));
}); });
test('save new card when nickname management is disabled', async function() { test('save new card', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: false});
const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open.
await whenAttributeIs(creditCardDialog.$$('#dialog'), 'open', '');
// Fill in name to the legacy name input field and card number.
creditCardDialog.$$('#legacyNameInput').value = 'Jane Doe';
creditCardDialog.$$('#numberInput').value = '4111111111111111';
// Select next year as the expiration year.
creditCardDialog.$.year.value = nextYear();
creditCardDialog.$.year.dispatchEvent(new CustomEvent('change'));
flush();
assertTrue(creditCardDialog.$.expired.hidden);
assertFalse(creditCardDialog.$.saveButton.disabled);
const savedPromise = eventToPromise('save-credit-card', creditCardDialog);
creditCardDialog.$.saveButton.click();
const saveEvent = await savedPromise;
// Verify the input values are correctly passed to save-credit-card.
// guid is empty when saving a new card.
assertEquals(saveEvent.detail.guid, undefined);
assertEquals(saveEvent.detail.name, 'Jane Doe');
assertEquals(saveEvent.detail.cardNumber, '4111111111111111');
assertEquals(saveEvent.detail.expirationYear, nextYear());
});
test('save new card when nickname management is enabled', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCardDialog = createAddCreditCardDialog(); const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open. // Wait for the dialog to open.
...@@ -190,7 +136,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -190,7 +136,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
creditCardDialog.$.year.dispatchEvent(new CustomEvent('change')); creditCardDialog.$.year.dispatchEvent(new CustomEvent('change'));
flush(); flush();
assertTrue(creditCardDialog.$.expired.hidden); const expiredError = creditCardDialog.$$('#expired-error');
assertEquals('hidden', getComputedStyle(expiredError).visibility);
assertFalse(creditCardDialog.$.saveButton.disabled); assertFalse(creditCardDialog.$.saveButton.disabled);
const savedPromise = eventToPromise('save-credit-card', creditCardDialog); const savedPromise = eventToPromise('save-credit-card', creditCardDialog);
...@@ -207,7 +154,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -207,7 +154,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
}); });
test('trim credit card when save', async function() { test('trim credit card when save', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCardDialog = createAddCreditCardDialog(); const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open. // Wait for the dialog to open.
...@@ -222,7 +168,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -222,7 +168,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
creditCardDialog.$.year.dispatchEvent(new CustomEvent('change')); creditCardDialog.$.year.dispatchEvent(new CustomEvent('change'));
flush(); flush();
assertTrue(creditCardDialog.$.expired.hidden); const expiredError = creditCardDialog.$$('#expired-error');
assertEquals('hidden', getComputedStyle(expiredError).visibility);
assertFalse(creditCardDialog.$.saveButton.disabled); assertFalse(creditCardDialog.$.saveButton.disabled);
const savedPromise = eventToPromise('save-credit-card', creditCardDialog); const savedPromise = eventToPromise('save-credit-card', creditCardDialog);
...@@ -239,7 +186,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -239,7 +186,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
}); });
test('update local card value', async function() { test('update local card value', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCard = createCreditCardEntry(); const creditCard = createCreditCardEntry();
creditCard.name = 'Wrong name'; creditCard.name = 'Wrong name';
creditCard.nickname = 'Shopping Card'; creditCard.nickname = 'Shopping Card';
...@@ -257,7 +203,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -257,7 +203,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
assertEquals(creditCardDialog.$$('#numberInput').value, '4444333322221111'); assertEquals(creditCardDialog.$$('#numberInput').value, '4444333322221111');
assertEquals(creditCardDialog.$.year.value, nextYear()); assertEquals(creditCardDialog.$.year.value, nextYear());
assertTrue(creditCardDialog.$.expired.hidden); const expiredError = creditCardDialog.$$('#expired-error');
assertEquals('hidden', getComputedStyle(expiredError).visibility);
assertFalse(creditCardDialog.$.saveButton.disabled); assertFalse(creditCardDialog.$.saveButton.disabled);
// Update cardholder name, card number, expiration year and nickname, and // Update cardholder name, card number, expiration year and nickname, and
...@@ -282,7 +229,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -282,7 +229,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
}); });
test('show error message when input nickname is invalid', async function() { test('show error message when input nickname is invalid', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCardDialog = createAddCreditCardDialog(); const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open. // Wait for the dialog to open.
...@@ -326,7 +272,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -326,7 +272,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
}); });
test('disable save button when input nickname is invalid', async function() { test('disable save button when input nickname is invalid', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCard = createCreditCardEntry(); const creditCard = createCreditCardEntry();
creditCard.name = 'Wrong name'; creditCard.name = 'Wrong name';
// Set the expiration year to next year to avoid expired card. // Set the expiration year to next year to avoid expired card.
...@@ -351,7 +296,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -351,7 +296,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
}); });
test('only show nickname character count when focused', async function() { test('only show nickname character count when focused', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCardDialog = createAddCreditCardDialog(); const creditCardDialog = createAddCreditCardDialog();
// Wait for the dialog to open. // Wait for the dialog to open.
...@@ -390,49 +334,7 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -390,49 +334,7 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
assertTrue(characterCount.textContent.includes('5/25')); assertTrue(characterCount.textContent.includes('5/25'));
}); });
test('expired card when nickname management is disabled', async function() { test('expired card', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: false});
const creditCard = createCreditCardEntry();
// Set the expiration year to the previous year to simulate expired card.
creditCard.expirationYear = lastYear();
// Edit dialog for an existing card with no nickname.
const creditCardDialog = createEditCreditCardDialog([creditCard]);
// Wait for the dialog to open.
await whenAttributeIs(creditCardDialog.$$('#dialog'), 'open', '');
// Verify save button is disabled for expired credit card.
assertTrue(creditCardDialog.$.saveButton.disabled);
// Legacy expired error message is shown, the new error message is still
// hidden.
assertFalse(creditCardDialog.$.expired.hidden);
assertTrue(creditCardDialog.$$('#expired-error').hidden);
// Check a11y attributes added for correct error announcement.
assertEquals('alert', creditCardDialog.$.expired.getAttribute('role'));
for (const select of [creditCardDialog.$.month, creditCardDialog.$.year]) {
assertEquals('true', select.getAttribute('aria-invalid'));
assertEquals('expired', select.getAttribute('aria-errormessage'));
}
// Update the expiration year to next year to avoid expired card.
creditCardDialog.$.year.value = nextYear();
creditCardDialog.$.year.dispatchEvent(new CustomEvent('change'));
flush();
// Expired error message is hidden for valid expiration date.
assertTrue(creditCardDialog.$.expired.hidden);
assertTrue(creditCardDialog.$$('#expired-error').hidden);
assertFalse(creditCardDialog.$.saveButton.disabled);
// Check a11y attributes for expiration error removed.
assertEquals(null, creditCardDialog.$.expired.getAttribute('role'));
for (const select of [creditCardDialog.$.month, creditCardDialog.$.year]) {
assertEquals('false', select.getAttribute('aria-invalid'));
assertEquals(null, select.getAttribute('aria-errormessage'));
}
});
test('expired card when nickname management is enabled', async function() {
loadTimeData.overrideValues({nicknameManagementEnabled: true});
const creditCard = createCreditCardEntry(); const creditCard = createCreditCardEntry();
// Set the expiration year to the previous year to simulate expired card. // Set the expiration year to the previous year to simulate expired card.
creditCard.expirationYear = lastYear(); creditCard.expirationYear = lastYear();
...@@ -445,10 +347,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -445,10 +347,8 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
// Verify save button is disabled for expired credit card. // Verify save button is disabled for expired credit card.
assertTrue(creditCardDialog.$.saveButton.disabled); assertTrue(creditCardDialog.$.saveButton.disabled);
const expiredError = creditCardDialog.$$('#expired-error'); const expiredError = creditCardDialog.$$('#expired-error');
// The new expired error message is shown, the legacy error message is still // The expired error message is shown.
// hidden.
assertEquals('visible', getComputedStyle(expiredError).visibility); assertEquals('visible', getComputedStyle(expiredError).visibility);
assertTrue(creditCardDialog.$.expired.hidden);
// Check a11y attributes added for correct error announcement. // Check a11y attributes added for correct error announcement.
assertEquals('alert', expiredError.getAttribute('role')); assertEquals('alert', expiredError.getAttribute('role'));
for (const select of [creditCardDialog.$.month, creditCardDialog.$.year]) { for (const select of [creditCardDialog.$.month, creditCardDialog.$.year]) {
...@@ -463,7 +363,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() { ...@@ -463,7 +363,6 @@ suite('PaymentsSectionCreditCardEditDialogTest', function() {
// Expired error message is hidden for valid expiration date. // Expired error message is hidden for valid expiration date.
assertEquals('hidden', getComputedStyle(expiredError).visibility); assertEquals('hidden', getComputedStyle(expiredError).visibility);
assertTrue(creditCardDialog.$.expired.hidden);
assertFalse(creditCardDialog.$.saveButton.disabled); assertFalse(creditCardDialog.$.saveButton.disabled);
// Check a11y attributes for expiration error removed. // Check a11y attributes for expiration error removed.
assertEquals(null, expiredError.getAttribute('role')); assertEquals(null, expiredError.getAttribute('role'));
......
...@@ -309,14 +309,15 @@ suite('PaymentsSection', function() { ...@@ -309,14 +309,15 @@ suite('PaymentsSection', function() {
.then(function() { .then(function() {
// Not expired, but still can't be saved, because there's no // Not expired, but still can't be saved, because there's no
// name. // name.
assertTrue(creditCardDialog.$.expired.hidden); const expiredError = creditCardDialog.$$('#expired-error');
assertEquals('hidden', getComputedStyle(expiredError).visibility);
assertTrue(creditCardDialog.$.saveButton.disabled); assertTrue(creditCardDialog.$.saveButton.disabled);
// Add a name. // Add a name.
creditCardDialog.set('creditCard.name', 'Jane Doe'); creditCardDialog.set('creditCard.name', 'Jane Doe');
flush(); flush();
assertTrue(creditCardDialog.$.expired.hidden); assertEquals('hidden', getComputedStyle(expiredError).visibility);
assertFalse(creditCardDialog.$.saveButton.disabled); assertFalse(creditCardDialog.$.saveButton.disabled);
const savedPromise = const savedPromise =
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment