Commit 4671e8f1 authored by Stephen McGruer's avatar Stephen McGruer Committed by Commit Bot

Revert "Refactor: Decouple PasswordListItem from embedder"

This reverts commit a89d80c4.

Reason for revert: Being reverted as part of a revert chain for a recent CL that is suspect of causing multiple issues; see https://chromium-review.googlesource.com/c/chromium/src/+/2252612

Original change's description:
> Refactor: Decouple PasswordListItem from embedder
> 
> This CL addresses a few design issues with PasswordListItem that predate
> the introduction of PasswordsListHandler.
> 
> - PasswordListItem would refer to a "menu" even though it knows nothing
> about such menu, which is actually provided by the embedder. This CL
> replaces mentions to passwordMenu in the component with what it actually
> represents: the "more actions" button.
> - The event fired by clicking the button was not documented in the file
> overview. Moreover, the typedef for the event lived in the embedder. Now
> the file exports the typedef.
> - Tests that simulated clicks on the button would use the $$('#id')
> syntax instead of the $.id one. This is now fixed.
> 
> Bug: None
> Change-Id: Ie815237134236468c639e73831bdb3eb7db36f98
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248561
> Commit-Queue: Victor Vianna <victorvianna@google.com>
> Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
> Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#779777}

TBR=fhorschig@chromium.org,aee@chromium.org,victorvianna@google.com

Change-Id: Id573b073d8d5081d5c05527896cfc6e230c4d2ea
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252614Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779919}
parent 1f6fc1e2
...@@ -63,8 +63,8 @@ ...@@ -63,8 +63,8 @@
[[entry.federationText]] [[entry.federationText]]
</span> </span>
</div> </div>
<cr-icon-button class="icon-more-vert" id="moreActionsButton" <cr-icon-button class="icon-more-vert" id="passwordMenu"
on-click="onPasswordMoreActionsButtonTap_" title="$i18n{moreActions}" on-click="onPasswordMenuTap_" title="$i18n{moreActions}"
focus-row-control focus-type="moreActionsButton" focus-row-control focus-type="passwordMenu"
aria-label$="[[getMoreActionsLabel_(entry)]]"></cr-icon-button> aria-label$="[[getMoreActionsLabel_(entry)]]"></cr-icon-button>
</div> </div>
...@@ -3,10 +3,8 @@ ...@@ -3,10 +3,8 @@
// found in the LICENSE file. // found in the LICENSE file.
/** /**
* @fileoverview PasswordListItem represents one row in a list of passwords, * @fileoverview PasswordListItem represents one row in the list of passwords.
* with a "more actions" button. It needs to be its own component because * It needs to be its own component because FocusRowBehavior provides good a11y.
* FocusRowBehavior provides good a11y.
* Clicking the button fires a password-more-actions-clicked event.
*/ */
import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js'; import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js';
...@@ -23,12 +21,6 @@ import {loadTimeData} from '../i18n_setup.js'; ...@@ -23,12 +21,6 @@ import {loadTimeData} from '../i18n_setup.js';
import {MultiStorePasswordUiEntry} from './multi_store_password_ui_entry.js'; import {MultiStorePasswordUiEntry} from './multi_store_password_ui_entry.js';
import {ShowPasswordBehavior} from './show_password_behavior.js'; import {ShowPasswordBehavior} from './show_password_behavior.js';
/**
* @typedef {!Event<!{target: !HTMLElement, listItem:
* !PasswordListItemElement}>}
*/
export let PasswordMoreActionsClickedEvent;
Polymer({ Polymer({
is: 'password-list-item', is: 'password-list-item',
...@@ -50,12 +42,12 @@ Polymer({ ...@@ -50,12 +42,12 @@ Polymer({
}, },
/** /**
* Opens the password action menu.
* @private * @private
*/ */
onPasswordMoreActionsButtonTap_() { onPasswordMenuTap_() {
this.fire( this.fire(
'password-more-actions-clicked', 'password-menu-tap', {target: this.$.passwordMenu, listItem: this});
{target: this.$.moreActionsButton, listItem: this});
}, },
/** /**
......
...@@ -26,7 +26,6 @@ import {loadTimeData} from '../i18n_setup.js'; ...@@ -26,7 +26,6 @@ import {loadTimeData} from '../i18n_setup.js';
// <if expr="chromeos"> // <if expr="chromeos">
import {BlockingRequestManager} from './blocking_request_manager.js'; import {BlockingRequestManager} from './blocking_request_manager.js';
// </if> // </if>
import {PasswordMoreActionsClickedEvent} from './password_list_item.js';
import {PasswordManagerImpl} from './password_manager_proxy.js'; import {PasswordManagerImpl} from './password_manager_proxy.js';
import {PasswordRemoveDialogPasswordsRemovedEvent} from './password_remove_dialog.js'; import {PasswordRemoveDialogPasswordsRemovedEvent} from './password_remove_dialog.js';
...@@ -101,7 +100,7 @@ Polymer({ ...@@ -101,7 +100,7 @@ Polymer({
], ],
listeners: { listeners: {
'password-more-actions-clicked': 'onPasswordMoreActionsClicked_', 'password-menu-tap': 'onPasswordMenuTap_',
'password-remove-dialog-passwords-removed': 'password-remove-dialog-passwords-removed':
'onPasswordRemoveDialogPasswordsRemoved_', 'onPasswordRemoveDialogPasswordsRemoved_',
}, },
...@@ -122,10 +121,11 @@ Polymer({ ...@@ -122,10 +121,11 @@ Polymer({
/** /**
* Opens the password action menu. * Opens the password action menu.
* @param {PasswordMoreActionsClickedEvent} event * @param {!Event<!{target: !HTMLElement, listItem:
* !PasswordListItemElement}>} event
* @private * @private
*/ */
onPasswordMoreActionsClicked_(event) { onPasswordMenuTap_(event) {
const target = event.detail.target; const target = event.detail.target;
this.activePassword = event.detail.listItem; this.activePassword = event.detail.listItem;
......
...@@ -216,7 +216,7 @@ suite('PasswordsDeviceSection', function() { ...@@ -216,7 +216,7 @@ suite('PasswordsDeviceSection', function() {
// dialog is now open. // dialog is now open.
const [password] = const [password] =
passwordsDeviceSection.root.querySelectorAll('password-list-item'); passwordsDeviceSection.root.querySelectorAll('password-list-item');
password.$.moreActionsButton.click(); password.$.passwordMenu.click();
passwordsDeviceSection.$.passwordsListHandler passwordsDeviceSection.$.passwordsListHandler
.$$('#menuMovePasswordToAccount') .$$('#menuMovePasswordToAccount')
.click(); .click();
......
...@@ -493,7 +493,7 @@ suite('PasswordsSection', function() { ...@@ -493,7 +493,7 @@ suite('PasswordsSection', function() {
const firstPassword = passwordList[0]; const firstPassword = passwordList[0];
// Click the remove button on the first password. // Click the remove button on the first password.
firstNode.$.moreActionsButton.click(); firstNode.$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click(); passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click();
const id = await passwordManager.whenCalled('removeSavedPassword'); const id = await passwordManager.whenCalled('removeSavedPassword');
...@@ -515,7 +515,7 @@ suite('PasswordsSection', function() { ...@@ -515,7 +515,7 @@ suite('PasswordsSection', function() {
passwordManager, passwordList, []); passwordManager, passwordList, []);
flush(); flush();
getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click(); getFirstPasswordListItem(passwordsSection).$$('#passwordMenu').click();
assertTrue( assertTrue(
passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').hidden); passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').hidden);
}); });
...@@ -530,7 +530,7 @@ suite('PasswordsSection', function() { ...@@ -530,7 +530,7 @@ suite('PasswordsSection', function() {
passwordManager, passwordList, []); passwordManager, passwordList, []);
flush(); flush();
getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click(); getFirstPasswordListItem(passwordsSection).$$('#passwordMenu').click();
assertFalse( assertFalse(
passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').hidden); passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').hidden);
}); });
...@@ -961,7 +961,7 @@ suite('PasswordsSection', function() { ...@@ -961,7 +961,7 @@ suite('PasswordsSection', function() {
const passwordsSection = elementFactory.createPasswordsSection( const passwordsSection = elementFactory.createPasswordsSection(
passwordManager, [expectedItem], []); passwordManager, [expectedItem], []);
getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click(); getFirstPasswordListItem(passwordsSection).$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').click(); passwordsSection.$.passwordsListHandler.$$('#menuCopyPassword').click();
return passwordManager.whenCalled('requestPlaintextPassword') return passwordManager.whenCalled('requestPlaintextPassword')
...@@ -980,7 +980,7 @@ suite('PasswordsSection', function() { ...@@ -980,7 +980,7 @@ suite('PasswordsSection', function() {
// Click the remove button on the first password and assert that an undo // Click the remove button on the first password and assert that an undo
// toast is shown. // toast is shown.
getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click(); getFirstPasswordListItem(passwordsSection).$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click(); passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click();
assertTrue(toastManager.isToastOpen); assertTrue(toastManager.isToastOpen);
...@@ -1275,13 +1275,13 @@ suite('PasswordsSection', function() { ...@@ -1275,13 +1275,13 @@ suite('PasswordsSection', function() {
// No removal actually happens, so all passwords keep their position. // No removal actually happens, so all passwords keep their position.
const passwordListItems = const passwordListItems =
passwordsSection.root.querySelectorAll('password-list-item'); passwordsSection.root.querySelectorAll('password-list-item');
passwordListItems[0].$.moreActionsButton.click(); passwordListItems[0].$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click(); passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click();
assertEquals( assertEquals(
passwordsSection.i18n('passwordDeletedFromAccount'), passwordsSection.i18n('passwordDeletedFromAccount'),
getToastManager().$.content.textContent); getToastManager().$.content.textContent);
passwordListItems[1].$.moreActionsButton.click(); passwordListItems[1].$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click(); passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click();
assertEquals( assertEquals(
passwordsSection.i18n('passwordDeletedFromDevice'), passwordsSection.i18n('passwordDeletedFromDevice'),
...@@ -1309,7 +1309,7 @@ suite('PasswordsSection', function() { ...@@ -1309,7 +1309,7 @@ suite('PasswordsSection', function() {
!passwordsSection.$.passwordsListHandler.$$('#passwordRemoveDialog')); !passwordsSection.$.passwordsListHandler.$$('#passwordRemoveDialog'));
// Clicking remove in the overflow menu shows the dialog. // Clicking remove in the overflow menu shows the dialog.
getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click(); getFirstPasswordListItem(passwordsSection).$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click(); passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click();
flush(); flush();
const removeDialog = const removeDialog =
...@@ -1348,7 +1348,7 @@ suite('PasswordsSection', function() { ...@@ -1348,7 +1348,7 @@ suite('PasswordsSection', function() {
!passwordsSection.$.passwordsListHandler.$$('#passwordRemoveDialog')); !passwordsSection.$.passwordsListHandler.$$('#passwordRemoveDialog'));
// Clicking remove in the overflow menu shows the dialog. // Clicking remove in the overflow menu shows the dialog.
getFirstPasswordListItem(passwordsSection).$.moreActionsButton.click(); getFirstPasswordListItem(passwordsSection).$$('#passwordMenu').click();
passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click(); passwordsSection.$.passwordsListHandler.$.menuRemovePassword.click();
flush(); flush();
const removeDialog = const removeDialog =
......
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