-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Settings] Improve accessibility for add languages dialog
Previously in crrev.com/c/4602528, we improved the performance settings add exception dialog accessibility by introducing a wrapper component around cr-checkbox. This CL extends that solution to the add languages dialog so that it can also have the index and count of languages read out by screenreaders. A side effect of this is that the visuals when focusing a list item will be different - the entire row is highlighted instead of just the checkbox element. Before: https://screenshot.googleplex.com/98DrDqz4sfrPCd9.png After: https://screenshot.googleplex.com/7xtDeM3e2qKissz.png Bug: 1451532 Change-Id: I004ea4c76efa14eb0248c35162a324ef574b2d0f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4928230 Reviewed-by: John Lee <johntlee@chromium.org> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com> Commit-Queue: Charles Meng <charlesmeng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1210310}
- Loading branch information
Charles Meng
authored and
Chromium LUCI CQ
committed
Oct 16, 2023
1 parent
ff24961
commit ab7f980
Showing
16 changed files
with
300 additions
and
157 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 3 additions & 15 deletions
18
...iscard_exception_current_sites_entry.html → ...ontrols/settings_checkbox_list_entry.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,15 @@ | ||
| <style include="settings-shared"> | ||
| /* add padding so that checkbox ripple is not cut off */ | ||
| .ripple-padding { | ||
| padding-inline-end: 20px; | ||
| padding-inline-start: 20px; | ||
| padding-inline-end: 20px; | ||
| } | ||
|
|
||
| cr-checkbox::part(label-container) { | ||
| min-width: 0; | ||
| } | ||
|
|
||
| .label-slot { | ||
| align-items: center; | ||
| display: flex; | ||
| } | ||
|
|
||
| .checkbox-label { | ||
| margin-inline-start: 10px; | ||
| } | ||
| </style> | ||
| <cr-checkbox id="checkbox" class="list-item no-outline ripple-padding" | ||
| tab-index="-1" checked="{{checked}}"> | ||
| <div class="label-slot"> | ||
| <site-favicon url="[[item]]"></site-favicon> | ||
| <div class="checkbox-label text-elide">[[item]]</div> | ||
| </div> | ||
| tab-index="-1" checked="{{checked}}" part="checkbox"> | ||
| <slot></slot> | ||
| </cr-checkbox> |
142 changes: 142 additions & 0 deletions
142
chrome/browser/resources/settings/controls/settings_checkbox_list_entry.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| // 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 'settings-checkbox-list-entry' is a wrapper for cr-checkbox so | ||
| * that it can have the correct accessibility behavior while inside an | ||
| * iron-list. Because cr-checkbox passes its focus to its inner checkbox | ||
| * element, screen readers are unable to infer a parent-child relationship | ||
| * between the list element and the focused checkbox. As a result using the | ||
| * roles listbox/option and annotating with aria-setsize/aria-posinset will not | ||
| * work properly. | ||
| * | ||
| * To fix this 'settings-checkbox-list-entry' hijacks focus and prevents it from | ||
| * going into the inner element, so that screenreaders will properly read | ||
| * "(x of y)". This however changes the visuals so that when the element is | ||
| * focused the entire row is highlighted instead of just the checkbox. | ||
| * | ||
| * Example usage: | ||
| * <iron-list role="listbox" items="[[items]]"> | ||
| * <template> | ||
| * <settings-checkbox-list-entry role="option" | ||
| * checked="[[isSelected_(item)]]" | ||
| * tabindex="[[tabIndex]]" | ||
| * aria-posinset$="[[addOneTo_(index)]]" | ||
| * aria-setsize$="[[items.length]]" | ||
| * on-change="toggleSelection_"> | ||
| * [[item]] | ||
| * </settings-checkbox-list-entry> | ||
| * </template> | ||
| * </iron-list> | ||
| */ | ||
| import 'chrome://resources/cr_elements/cr_checkbox/cr_checkbox.js'; | ||
| import '../settings_shared.css.js'; | ||
| import '../site_favicon.js'; | ||
|
|
||
| import {CrCheckboxElement} from 'chrome://resources/cr_elements/cr_checkbox/cr_checkbox.js'; | ||
| import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; | ||
|
|
||
| import {getTemplate} from './settings_checkbox_list_entry.html.js'; | ||
|
|
||
| export interface SettingsCheckboxListEntryElement { | ||
| $: { | ||
| checkbox: CrCheckboxElement, | ||
| }; | ||
| } | ||
|
|
||
| export class SettingsCheckboxListEntryElement extends PolymerElement { | ||
| static get is() { | ||
| return 'settings-checkbox-list-entry'; | ||
| } | ||
|
|
||
| static get template() { | ||
| return getTemplate(); | ||
| } | ||
|
|
||
| static get properties() { | ||
| return { | ||
| // Used to set the status of the checkbox when the entry is created, | ||
| // as well as when the list item for the entry changes. | ||
| checked: { | ||
| type: Boolean, | ||
| value: false, | ||
| observer: 'onCheckedChanged_', | ||
| }, | ||
|
|
||
| // Reflects to the tabindex attribute. When it is negative (non-focusable) | ||
| // aria-hidden will be set to "true", so that it will be ignored by screen | ||
| // readers. This is needed because iron-list recycles its entries, so when | ||
| // focusing an entry, the screen reader can be confused by other entries' | ||
| // aria-posinset and aria-setsize attributes if they aren't aria-hidden. | ||
| tabindex: { | ||
| type: Number, | ||
| value: 0, | ||
| observer: 'onTabIndexChanged_', | ||
| reflectToAttribute: true, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| checked: boolean; | ||
| private posinset: number; | ||
| private setsize: number; | ||
| private tabindex: number; | ||
|
|
||
| override ready() { | ||
| super.ready(); | ||
| this.addEventListener('click', this.onClick_); | ||
| this.addEventListener('keydown', this.onKeyDown_); | ||
| this.addEventListener('keyup', this.onKeyUp_); | ||
| } | ||
|
|
||
| private onClick_() { | ||
| this.$.checkbox.click(); | ||
| } | ||
|
|
||
| // Handle key presses in the same way as cr-checkbox, because it no longer | ||
| // receives focus. | ||
| private onKeyDown_(e: KeyboardEvent) { | ||
| if (e.key !== ' ' && e.key !== 'Enter') { | ||
| return; | ||
| } | ||
|
|
||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| if (e.repeat) { | ||
| return; | ||
| } | ||
|
|
||
| if (e.key === 'Enter') { | ||
| this.$.checkbox.click(); | ||
| } | ||
| } | ||
|
|
||
| private onKeyUp_(e: KeyboardEvent) { | ||
| if (e.key === ' ' || e.key === 'Enter') { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| } | ||
|
|
||
| if (e.key === ' ') { | ||
| this.$.checkbox.click(); | ||
| } | ||
| } | ||
|
|
||
| private onCheckedChanged_() { | ||
| this.setAttribute('aria-checked', String(this.$.checkbox.checked)); | ||
| } | ||
|
|
||
| private onTabIndexChanged_() { | ||
| this.setAttribute('aria-hidden', this.tabindex >= 0 ? 'false' : 'true'); | ||
| } | ||
| } | ||
|
|
||
| declare global { | ||
| interface HTMLElementTagNameMap { | ||
| 'settings-checkbox-list-entry': SettingsCheckboxListEntryElement; | ||
| } | ||
| } | ||
|
|
||
| customElements.define( | ||
| SettingsCheckboxListEntryElement.is, SettingsCheckboxListEntryElement); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
97 changes: 0 additions & 97 deletions
97
.../browser/resources/settings/performance_page/tab_discard_exception_current_sites_entry.ts
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.