Skip to content

GitLab

T
tangled
Commit 2fb616ae authored by Alex Ilin's avatar Alex Ilin Committed by Chromium LUCI CQ
Browse files
Options

[profilePicker] Convert remove confirmation to cr-dialog 

The profile card uses cr-action-menu to display the remove confirmation
dialog. This dialog has an accessibility issue because its content isn't
read automatically when the dialog appears. It also doesn't follow the
common Chrome pattern for webUI dialogs.

This CL switches the remove confirmation dialog over to cr-dialog which
is the standard component for displaying dialogs in webUI.

Bug: 1156068
Change-Id: If865a4b9d9ad4d6e15c3f8980828cee98f0898b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615350
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarMonica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843166}
parent 0caf3368
Show whitespace changes
Inline Side-by-side
Showing with 254 additions and 75 deletions
chrome/browser/resources/signin/profile_picker/BUILD.gn
View file @ 2fb616ae
......@@ -222,6 +222,7 @@ js_library("profile_card_menu") {
":manage_profiles_browser_proxy",
"//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
"//ui/webui/resources/cr_elements/cr_action_menu:cr_action_menu.m",
"//ui/webui/resources/cr_elements/cr_dialog:cr_dialog.m",
"//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m",
"//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:i18n_behavior.m",
......
chrome/browser/resources/signin/profile_picker/profile_card.html
View file @ 2fb616ae
......@@ -9,8 +9,7 @@
}
cr-button {
--card-background-color: var(--md-background-color);
background-color: var(--card-background-color);
background-color: var(--profile-card-background-color);
border: none;
border-radius: inherit;
box-shadow: none;
......@@ -24,13 +23,6 @@
box-shadow: 0 0 0 2px rgba(var(--google-blue-600-rgb), .4);
}
.profile-avatar {
border-radius: 37px;
flex-shrink: 0;
height: var(--avatar-icon-size);
width: var(--avatar-icon-size);
}
#avatarContainer {
height: var(--avatar-icon-size);
position: relative;
......@@ -43,7 +35,7 @@
--domain-icon-border-size: 2px;
align-items: center;
background-color: white;
border: var(--domain-icon-border-size) solid var(--card-background-color);
border: var(--domain-icon-border-size) solid var(--profile-card-background-color);
border-radius: 50%;
box-shadow: 0 0 2px rgba(60, 64, 67, 0.12), 0 0 6px rgba(60, 64, 67, 0.15);
display: flex;
......@@ -113,10 +105,6 @@
}
@media (prefers-color-scheme: dark) {
cr-button {
--card-background-color: var(--google-grey-800);
}
#iconContainer {
background-color: var(--md-background-color);
box-shadow: 0 0 2px rgba(60, 64, 67, 0.12), 0 0 6px
......
chrome/browser/resources/signin/profile_picker/profile_card.js
View file @ 2fb616ae
......@@ -4,6 +4,7 @@
import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
import 'chrome://resources/cr_elements/icons.m.js';
import 'chrome://resources/cr_elements/hidden_style_css.m.js';
import './profile_card_menu.js';
import './profile_picker_shared_css.js';
import 'chrome://resources/cr_elements/cr_input/cr_input.m.js';
......
chrome/browser/resources/signin/profile_picker/profile_card_menu.html
View file @ 2fb616ae
<style include="cr-icons profile-picker-shared">
<style include="cr-icons profile-picker-shared cr-hidden-style">
#moreActionsButton {
--cr-icon-button-icon-size: 14px;
--cr-icon-button-margin-end: 0;
......@@ -23,26 +23,13 @@
padding-inline-start: 14px;
}
#removeActionMenu {
pointer-events: none;
cr-dialog::part(dialog) {
width: 448px;
}
#removeConfirmation {
color: var(--cr-primary-text-color);
margin-top: 16px;
pointer-events: none;
width: 234px;
}
#removeConfirmation > * {
margin: 0 16px 16px 16px;
}
.header {
font-size: 1.17em;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
#removeWarningHeader {
line-height: 20px;
padding: 4px 20px 20px 20px;
}
#userName {
......@@ -53,19 +40,58 @@
font-weight: 500;
}
.statistics {
#removeActionDialogBody {
align-items: center;
border: 1px solid var(--google-grey-refresh-100);
border-radius: 4px;
border-radius: 12px;
box-sizing: border-box;
padding: 12px 16px 16px 12px;
width: -webkit-fill-available;
display: flex;
flex-direction: row;
}
#profileCardContainer {
align-items: center;
background-color: var(--profile-card-background-color);
border-radius: 12px 0 0 12px;
display: flex;
flex-direction: column;
height: var(--profile-item-height);
justify-content: center;
position: relative;
width: var(--profile-item-width);
}
#avatarContainer {
height: var(--avatar-icon-size);
position: relative;
}
#profileName {
font-weight: 500;
top: 0;
}
#gaiaName {
bottom: 0;
font-weight: normal;
}
.statistics {
display: grid;
flex-grow: 1;
grid-template-columns: 5fr 1fr;
padding: 20px;
}
.category {
align-self: center;
color: var(--cr-primary-text-color);
line-height: 24px;
text-align: start;
}
.count {
align-self: center;
color: var(--google-grey-refresh-500);
text-align: end;
}
......@@ -76,14 +102,7 @@
--hover-bg-action: rgba(var(--google-red-700-rgb), .9);
--hover-shadow-action-rgb: var(--google-red-300-rgb);
background-color: var(--bg-action);
border-radius: 4px;
color: var(--ink-color-action);
font-weight: 500;
left: 50%;
margin-bottom: 16px;
pointer-events: auto;
transform: translateX(-50%);
width: 111px;
}
#removeConfirmationButton:hover {
......@@ -95,7 +114,7 @@
color: var(--google-grey-500);
}
.statistics {
#removeActionDialogBody {
border-color: var(--google-grey-refresh-700);
}
}
......@@ -117,30 +136,47 @@
</button>
</cr-action-menu>
<cr-action-menu id="removeActionMenu" role-description="$i18n{menu}">
<div id="removeConfirmation">
<div class="header">
<cr-dialog id="removeConfirmationDialog" ignore-enter-key>
<div slot="title">
$i18n{profileMenuRemoveText}
<span class="key-text">[[profileState.localProfileName]]</span>
</div>
<div class="warning-message">
<div id="removeWarningHeader" slot="header" class="warning-message">
[[removeWarningText_]]
<div id="userName" hidden$="[[!profileState.isSyncing]]" class="key-text">
[[profileState.userName]]
</div>
</div>
<table class="statistics">
<div slot="body">
<div id="removeActionDialogBody">
<div id="profileCardContainer">
<div id="avatarContainer">
<img class="profile-avatar" alt="" src="[[profileState.avatarIcon]]">
</div>
<div id="profileName" class="profile-card-info">
[[profileState.localProfileName]]
</div>
<div id="gaiaName" class="profile-card-info">
[[profileState.gaiaName]]
</div>
</div>
<div class="statistics">
<template is="dom-repeat" items="[[profileStatistics_]]">
<tr>
<td class="category">[[getProfileStatisticText_(item)]]</td>
<td class="count">
[[getProfileStatisticCount_(item, statistics_)]]</td>
<tr>
<span class="category">[[getProfileStatisticText_(item)]]</span>
<span class="count">
[[getProfileStatisticCount_(item, statistics_)]]
</span>
</template>
</table>
</div>
<cr-button id="removeConfirmationButton"class="dropdown-item action-button"
on-click="onRemoveComfirationClicked_">
</div>
</div>
<div slot="button-container">
<cr-button class="cancel-button" on-click="onRemoveCancelClicked_">
$i18n{cancel}
</cr-button>
<cr-button id="removeConfirmationButton" class="action-button"
on-click="onRemoveConfirmationClicked_">
$i18n{profileMenuRemoveText}
</cr-button>
</cr-action-menu>
</div>
</cr-dialog>
chrome/browser/resources/signin/profile_picker/profile_card_menu.js
View file @ 2fb616ae
......@@ -4,7 +4,9 @@
import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js';
import 'chrome://resources/cr_elements/cr_action_menu/cr_action_menu.m.js';
import 'chrome://resources/cr_elements/cr_dialog/cr_dialog.m.js';
import 'chrome://resources/cr_elements/cr_icons_css.m.js';
import 'chrome://resources/cr_elements/hidden_style_css.m.js';
import 'chrome://resources/cr_elements/shared_vars_css.m.js';
import './profile_picker_shared_css.js';
import './icons.js';
......@@ -14,7 +16,6 @@ import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {WebUIListenerBehavior} from 'chrome://resources/js/web_ui_listener_behavior.m.js';
import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import './strings.js';
import {ManageProfilesBrowserProxy, ManageProfilesBrowserProxyImpl, ProfileState} from './manage_profiles_browser_proxy.js';
......@@ -147,7 +148,7 @@ Polymer({
this.manageProfilesBrowserProxy_.getProfileStatistics(
this.profileState.profilePath);
this.$.actionMenu.close();
this.$.removeActionMenu.showAt(this.$.moreActionsButton);
this.$.removeConfirmationDialog.showModal();
chrome.metricsPrivate.recordUserAction('ProfilePicker_RemoveOptionClicked');
},
......@@ -197,20 +198,27 @@ Polymer({
* @param {!Event} e
* @private
*/
onRemoveComfirationClicked_(e) {
onRemoveConfirmationClicked_(e) {
e.stopPropagation();
e.preventDefault();
this.manageProfilesBrowserProxy_.removeProfile(
this.profileState.profilePath);
},
/**
* @param {!Event} e
* @private
*/
onRemoveCancelClicked_(e) {
this.$.removeConfirmationDialog.cancel();
},
/**
* Ensure any menu is closed on profile list updated.
* @private
*/
handleProfilesUpdated_() {
this.$.actionMenu.close();
this.$.removeActionMenu.close();
},
/** @private */
......
chrome/browser/resources/signin/profile_picker/profile_picker_main_view.html
View file @ 2fb616ae
......@@ -3,6 +3,9 @@
--avatar-icon-size: 74px;
--banner-img-height: 400px;
--banner-img-width: 169px;
--profile-item-height: 178px;
--profile-item-margin: 4px;
--profile-item-width: 162px;
}
.banner {
......@@ -53,9 +56,6 @@
.profiles-container {
--grid-gutter: 16px;
--profile-item-height:178px;
--profile-item-margin: 4px;
--profile-item-width: 162px;
align-items: center;
display: grid;
grid-column-gap: var(--grid-gutter);
......
chrome/browser/resources/signin/profile_picker/profile_picker_shared_css.html
View file @ 2fb616ae
......@@ -5,11 +5,13 @@
--scrollbar-width: 4px;
--scrollbar-background: var(--google-grey-refresh-100);
--footer-margin: 40px;
--profile-card-background-color: var(--md-background-color);
}
@media (prefers-color-scheme: dark) {
:host {
--scrollbar-background: var(--google-grey-800);
--profile-card-background-color: var(--google-grey-800);
}
}
......@@ -66,5 +68,12 @@
background: var(--scrollbar-background);
border-radius: var(--scrollbar-width);
}
.profile-avatar {
border-radius: 37px;
flex-shrink: 0;
height: var(--avatar-icon-size);
width: var(--avatar-icon-size);
}
</style>
</template>
chrome/browser/ui/webui/signin/profile_picker_ui.cc
View file @ 2fb616ae
......@@ -83,6 +83,7 @@ void AddStrings(content::WebUIDataSource* html_source) {
IDS_PROFILE_PICKER_PROFILE_CARD_NEEDS_SIGNIN_PROMPT},
{"profileCardButtonLabel", IDS_PROFILE_PICKER_PROFILE_CARD_LABEL},
{"menu", IDS_MENU},
{"cancel", IDS_CANCEL},
{"profileMenuName", IDS_PROFILE_PICKER_PROFILE_MENU_BUTTON_NAME},
{"profileMenuRemoveText", IDS_PROFILE_PICKER_PROFILE_MENU_REMOVE_TEXT},
{"profileMenuCustomizeText",
......
chrome/test/data/webui/signin/BUILD.gn
View file @ 2fb616ae
......@@ -16,6 +16,7 @@ js_type_check("closure_compile") {
deps = [
":dice_web_signin_intercept_test",
":local_profile_customization_test",
":profile_card_menu_test",
":profile_customization_test",
":profile_picker_app_test",
":profile_type_choice_test",
......@@ -82,6 +83,15 @@ js_library("profile_picker_app_test") {
externs_list = [ "$externs_path/mocha-2.5.js" ]
}
js_library("profile_card_menu_test") {
deps = [
"..:chai_assert",
"..:test_browser_proxy.m",
"..:test_util.m",
"//chrome/browser/resources/signin/profile_picker:profile_card_menu",
]
}
js_library("test_dice_web_signin_intercept_browser_proxy") {
deps = [
"..:test_browser_proxy.m",
......
chrome/test/data/webui/signin/profile_card_menu_test.js 0 → 100644
View file @ 2fb616ae
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import {ManageProfilesBrowserProxyImpl, ProfileState} from 'chrome://profile-picker/profile_picker.js';
import {assertEquals, assertFalse, assertTrue} from '../chai_assert.js';
import {waitBeforeNextRender} from '../test_util.m.js';
import {TestManageProfilesBrowserProxy} from './test_manage_profiles_browser_proxy.js';
suite('ProfileCardMenuTest', function() {
/** @type {!ProfileCardMenuElement} */
let profileCardMenuElement;
/** @type {!TestManageProfilesBrowserProxy} */
let browserProxy;
const menuButtonIndex = {
CUSTOMIZE: 0,
DELETE: 1,
};
setup(function() {
browserProxy = new TestManageProfilesBrowserProxy();
ManageProfilesBrowserProxyImpl.instance_ = browserProxy;
document.body.innerHTML = '';
profileCardMenuElement = /** @type {!ProfileCardMenuElement} */ (
document.createElement('profile-card-menu'));
document.body.appendChild(profileCardMenuElement);
const testProfileState = /** @type {!ProfileState} */ ({
profilePath: `profilePath`,
localProfileName: `profile`,
isSyncing: true,
needsSignin: false,
gaiaName: `User`,
userName: `User@gmail.com`,
isManaged: false,
avatarIcon: `AvatarUrl`,
});
profileCardMenuElement.profileState = testProfileState;
return waitBeforeNextRender(profileCardMenuElement);
});
// Checks basic layout of the action menu.
test('ProfileCardMenuActionMenu', async function() {
assertFalse(profileCardMenuElement.$$('#actionMenu').open);
assertFalse(profileCardMenuElement.$$('#removeConfirmationDialog').open);
profileCardMenuElement.$$('#moreActionsButton').click();
assertTrue(profileCardMenuElement.$$('#actionMenu').open);
assertFalse(profileCardMenuElement.$$('#removeConfirmationDialog').open);
const menuButtons = profileCardMenuElement.shadowRoot.querySelectorAll(
'#actionMenu > .dropdown-item');
assertEquals(menuButtons.length, 2);
});
// Click on the customize profile menu item calls native to open the profile
// settings page.
test('ProfileCardMenuCustomizeButton', async function() {
profileCardMenuElement.$$('#moreActionsButton').click();
const menuButtons = profileCardMenuElement.$$('#actionMenu')
.querySelectorAll('.dropdown-item');
menuButtons[menuButtonIndex.CUSTOMIZE].click();
await browserProxy.whenCalled('openManageProfileSettingsSubPage');
assertFalse(profileCardMenuElement.$$('#actionMenu').open);
assertFalse(profileCardMenuElement.$$('#removeConfirmationDialog').open);
});
// Click on the delete profile menu item opens the remove confirmation dialog.
test('ProfileCardMenuDeleteButton', async function() {
profileCardMenuElement.$$('#moreActionsButton').click();
const menuButtons = profileCardMenuElement.shadowRoot.querySelectorAll(
'#actionMenu > .dropdown-item');
menuButtons[menuButtonIndex.DELETE].click();
assertFalse(profileCardMenuElement.$$('#actionMenu').open);
assertTrue(profileCardMenuElement.$$('#removeConfirmationDialog').open);
});
// Click on the cancel button in the remove confirmation dialog closes the
// dialog.
test('RemoveConfirmationDialogCancel', async function() {
const dialog = profileCardMenuElement.$$('#removeConfirmationDialog');
dialog.showModal();
assertTrue(dialog.open);
dialog.querySelector('.cancel-button').click();
assertFalse(dialog.open);
assertEquals(browserProxy.getCallCount('removeProfile'), 0);
});
// Click on the delete button in the remove confirmation dialog calls native
// to remove profile.
test('RemoveConfirmationDialogDelete', async function() {
const dialog = profileCardMenuElement.$$('#removeConfirmationDialog');
dialog.showModal();
assertTrue(dialog.open);
dialog.querySelector('.action-button').click();
await browserProxy.whenCalled('removeProfile');
});
});
chrome/test/data/webui/signin/signin_browsertest.js
View file @ 2fb616ae
......@@ -189,6 +189,32 @@ TEST_F('ProfilePickerMainViewTest', 'All', function() {
mocha.run();
});
/**
* Test fixture for
* chrome/browser/resources/signin/profile_picker/profile_card_menu.html.
* This has to be declared as a variable for TEST_F to find it correctly.
*/
// eslint-disable-next-line no-var
var ProfileCardMenuTest = class extends SigninBrowserTest {
/** @override */
get browsePreload() {
return 'chrome://profile-picker/test_loader.html?module=signin/profile_card_menu_test.js';
}
/** @override */
get featureList() {
return {
enabled: [
'features::kNewProfilePicker',
]
};
}
};
TEST_F('ProfileCardMenuTest', 'All', function() {
mocha.run();
});
/**
* Test fixture for
* chrome/browser/resources/signin/profile_customization/profile_customization_app.html.
......
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