From f85a34e9d9023d101f62725569f8f1a9a59b6e87 Mon Sep 17 00:00:00 2001 From: Ryan Sultanem Date: Wed, 20 Nov 2024 10:11:44 +0000 Subject: [PATCH] [BatchUpload] Fix accessibility issues #part 1 Screencast: https://screencast.googleplex.com/cast/NTEyMzkzMjg1NTIwNTg4OHxiZGQ4YWFkZC1kZA Fixes in part #1 (this CL): - Batch Upload entry point properly reads the title, subtitle and the button content. - Account icon is not navigable. - Items icons are not navigable. - Checkbox reads it's content and is not navigable. - Toggle and Expand button reads the section title without the count for clearer context. - When unselecting last checkbox of a section, the focus goes back to the section's toggle since it got disabled. Fixes to be done in part #2 (follow CL): - Make focusing the last checked checkbox announce text that mentions that unchecking it will be disable the section. - Announce the number of selected items after checking or unchecking an item. - Make the toggle say a more specific action: "Select all/none" All changes in part #2 requires adding new strings. Fixed: b:378720472, b:378718895, b:378711564 Bug: b:378746667 Change-Id: I98251bdd5d3adcbe1a7ed3e2a7a62c2327964b58 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6032478 Reviewed-by: David Roger Code-Coverage: findit-for-me@appspot.gserviceaccount.com Commit-Queue: Ryan Sultanem Cr-Commit-Position: refs/heads/main@{#1385535} --- .../password_manager/settings_section.html | 5 +++-- .../password_manager/settings_section.ts | 8 ++++++++ .../batch_upload/batch_upload_app.html.ts | 4 ++-- .../signin/batch_upload/data_section.css | 8 ++++++++ .../signin/batch_upload/data_section.html.ts | 15 +++++++++------ .../signin/batch_upload/data_section.ts | 17 +++++++++++++++-- 6 files changed, 45 insertions(+), 12 deletions(-) diff --git a/chrome/browser/resources/password_manager/settings_section.html b/chrome/browser/resources/password_manager/settings_section.html index c567784273caec..e9e370c22256f4 100644 --- a/chrome/browser/resources/password_manager/settings_section.html +++ b/chrome/browser/resources/password_manager/settings_section.html @@ -90,8 +90,9 @@

$i18n{settings}

restamp> - - $i18n{moveSinglePasswordButton} + + $i18n{moveSinglePasswordButton} diff --git a/chrome/browser/resources/password_manager/settings_section.ts b/chrome/browser/resources/password_manager/settings_section.ts index 43c103b1212c66..c8eb187661b199 100644 --- a/chrome/browser/resources/password_manager/settings_section.ts +++ b/chrome/browser/resources/password_manager/settings_section.ts @@ -444,6 +444,14 @@ export class SettingsSectionElement extends SettingsSectionElementBase { return this.isAccountStoreUser && this.passwordsOnDevice_.length > 0; } + private getAriaLabelMovePasswordsButton_(): string { + return [ + this.movePasswordsLabel_, + this.i18n('movePasswordsInSettingsSubLabel'), + this.i18n('moveSinglePasswordButton'), + ].join('. '); + } + // This updates the local password count coming from the Sync Service API. private async updateLocalPasswordCount_(localPasswordCount: number) { this.localPasswordCount_ = localPasswordCount; diff --git a/chrome/browser/resources/signin/batch_upload/batch_upload_app.html.ts b/chrome/browser/resources/signin/batch_upload/batch_upload_app.html.ts index 9799c0ca43480f..2241028b125019 100644 --- a/chrome/browser/resources/signin/batch_upload/batch_upload_app.html.ts +++ b/chrome/browser/resources/signin/batch_upload/batch_upload_app.html.ts @@ -15,8 +15,8 @@ export function getHtml(this: BatchUploadAppElement) {
${this.dialogSubtitle_}
- Account icon +
${this.accountInfo_.email}
diff --git a/chrome/browser/resources/signin/batch_upload/data_section.css b/chrome/browser/resources/signin/batch_upload/data_section.css index 2f0b49acf14131..0591d2a7e79a63 100644 --- a/chrome/browser/resources/signin/batch_upload/data_section.css +++ b/chrome/browser/resources/signin/batch_upload/data_section.css @@ -72,6 +72,14 @@ .item-checkbox { padding-inline-end: 12px; width: 16px; + flex-grow: 0; +} + +/* Visually hide the label but still allow the screen reader to pick it up. */ +cr-checkbox::part(label-container) { + clip: rect(0,0,0,0); + display: block; + position: fixed; } .item-icon-container { diff --git a/chrome/browser/resources/signin/batch_upload/data_section.html.ts b/chrome/browser/resources/signin/batch_upload/data_section.html.ts index 5e9fca4c098c8d..3e539f1fb989a7 100644 --- a/chrome/browser/resources/signin/batch_upload/data_section.html.ts +++ b/chrome/browser/resources/signin/batch_upload/data_section.html.ts @@ -15,12 +15,14 @@ export function getHtml(this: DataSectionElement) { + @expanded-changed="${this.onExpandChanged_}" + aria-label="${this.titleWithoutCount_}">
+ ?checked="${!this.disabled_}" + aria-label="${this.titleWithoutCount_}"> @@ -32,18 +34,19 @@ export function getHtml(this: DataSectionElement) { data-id="${item.id}" ?checked="${this.isCheckboxChecked_(item.id)}" @change="${this.onCheckedChanged_}"> + ${item.title}, ${item.subtitle}
- Item icon +
-
+ -
+
diff --git a/chrome/browser/resources/signin/batch_upload/data_section.ts b/chrome/browser/resources/signin/batch_upload/data_section.ts index 074addde494315..b6589831011194 100644 --- a/chrome/browser/resources/signin/batch_upload/data_section.ts +++ b/chrome/browser/resources/signin/batch_upload/data_section.ts @@ -68,6 +68,7 @@ export class DataSectionElement extends CrLitElement { return { dataContainer: {type: Object}, title_: {type: String}, + titleWithoutCount_: {type: String}, expanded_: {type: Boolean}, disabled_: {type: Boolean}, dataSelectedCount_: {type: Number}, @@ -80,6 +81,8 @@ export class DataSectionElement extends CrLitElement { // Title of the section, updated on each item checkbox selection based on the // number of selected items. protected title_: string = ''; + // Computed once on page load as it does not contain the selected item count. + protected titleWithoutCount_: string = ''; // If the collapse section is exapnded. protected expanded_: boolean = false; @@ -98,10 +101,18 @@ export class DataSectionElement extends CrLitElement { private intervalDurationOfUpdateHeightRequests_: number|null = null; private collapseAnimationDuration_: number = 0; - override connectedCallback() { + override async connectedCallback() { super.connectedCallback(); this.initializeSectionOutput_(); + + // In tests this id may be empty. + if (this.dataContainer.sectionTitle && + this.dataContainer.sectionTitle.length > 0) { + this.titleWithoutCount_ = + await PluralStringProxyImpl.getInstance().getPluralString( + this.dataContainer.sectionTitle, 0); + } } override firstUpdated() { @@ -227,9 +238,11 @@ export class DataSectionElement extends CrLitElement { // Checkbox off. this.dataSelected.delete(itemId); this.dataSelectedCount_ = this.dataSelected.size; - // If this is the last item unchecked then disable and reset the section. + // If this is the last item unchecked then disable and reset the section + // and focus the toggle since its value changed indirectly. if (this.dataSelectedCount_ === 0) { this.resetWithState_(/*disabled=*/ true); + this.$.toggle.focus(); } }