maxmustermann/vscode
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Improve a11y for settings object renderer (#101058)

Browse source 
* Improve a11y for settings object renderer

* Address comments

* Store rowElements without re-rendering
This commit is contained in:
Aditya Thakral 2020-06-28 16:19:33 -04:00 committed by GitHub
parent d53fcf8781
commit 7798680651
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
src/vs/workbench/contrib/preferences/browser/settingsTree.ts
View file

@ -700,6 +700,11 @@ export abstract class AbstractSettingRenderer extends Disposable implements ITre
itemElement.setAttribute('role', 'combobox');
label += modifiedText;
}
} else if (templateId === SETTINGS_OBJECT_TEMPLATE_ID) {
if (itemElement = (<ISettingObjectItemTemplate>template).objectWidget.domNode) {
itemElement.setAttribute('role', 'list');
label += modifiedText;
}
} else {
// Don't change attributes if we don't know what we areFunctions
return '';
@ -1034,6 +1039,7 @@ export class SettingObjectRenderer extends AbstractSettingRenderer implements IT
protected renderValue(dataElement: SettingsTreeSettingElement, template: ISettingObjectItemTemplate, onChange: (value: string) => void): void {
const items = getObjectDisplayValue(dataElement);
template.objectWidget.setValue(items, {
showAddButton: (
isDefined(dataElement.setting.objectAdditionalProperties) ||
@ -1041,6 +1047,9 @@ export class SettingObjectRenderer extends AbstractSettingRenderer implements IT
!areAllPropertiesDefined(Object.keys(dataElement.setting.objectProperties ?? {}), items)
),
});
this.setElementAriaLabels(dataElement, this.templateId, template);
template.context = dataElement;
}
}

85
src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts
View file

@ -20,7 +20,7 @@ import { foreground, inputBackground, inputBorder, inputForeground, listActiveSe
import { attachButtonStyler, attachInputBoxStyler, attachSelectBoxStyler } from 'vs/platform/theme/common/styler';
import { ICssStyleCollector, IColorTheme, IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService';
import { disposableTimeout } from 'vs/base/common/async';
import { isUndefinedOrNull } from 'vs/base/common/types';
import { isUndefinedOrNull, isDefined } from 'vs/base/common/types';
import { preferencesEditIcon } from 'vs/workbench/contrib/preferences/browser/preferencesWidgets';
import { SelectBox } from 'vs/base/browser/ui/selectBox/selectBox';
import { isIOS } from 'vs/base/common/platform';
@ -226,6 +226,7 @@ interface IEditHandlers<TDataItem extends object> {
abstract class AbstractListSettingWidget<TDataItem extends object> extends Disposable {
private listElement: HTMLElement;
private rowElements: HTMLElement[] = [];
protected readonly _onDidChangeList = this._register(new Emitter<ISettingListChangeEvent<TDataItem>>());
protected readonly model = new ListSettingListModel<TDataItem>(this.getEmptyItem());
@ -257,24 +258,17 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
this._register(DOM.addDisposableListener(this.listElement, DOM.EventType.CLICK, e => this.onListClick(e)));
this._register(DOM.addDisposableListener(this.listElement, DOM.EventType.DBLCLICK, e => this.onListDoubleClick(e)));
this._register(DOM.addStandardDisposableListener(this.listElement, 'keydown', (e: KeyboardEvent) => {
if (e.keyCode === KeyCode.UpArrow) {
const selectedIndex = this.model.getSelected();
this.model.selectPrevious();
if (this.model.getSelected() !== selectedIndex) {
this.renderList();
}
e.preventDefault();
e.stopPropagation();
} else if (e.keyCode === KeyCode.DownArrow) {
const selectedIndex = this.model.getSelected();
this.model.selectNext();
if (this.model.getSelected() !== selectedIndex) {
this.renderList();
}
e.preventDefault();
e.stopPropagation();
this._register(DOM.addStandardDisposableListener(this.listElement, 'keydown', (e: StandardKeyboardEvent) => {
if (e.equals(KeyCode.UpArrow)) {
this.selectPreviousRow();
} else if (e.equals(KeyCode.DownArrow)) {
this.selectNextRow();
} else {
return;
}
e.preventDefault();
e.stopPropagation();
}));
}
@ -322,9 +316,8 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
this.listElement.appendChild(header);
}
this.model.items
.map((item, i) => this.renderDataOrEditItem(item, i, focused))
.forEach(itemElement => this.listElement.appendChild(itemElement));
this.rowElements = this.model.items.map((item, i) => this.renderDataOrEditItem(item, i, focused));
this.rowElements.forEach(rowElement => this.listElement.appendChild(rowElement));
this.listElement.style.height = listHeight + 'px';
}
@ -335,9 +328,13 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
}
private renderDataOrEditItem(item: IListViewItem<TDataItem>, idx: number, listFocused: boolean): HTMLElement {
return item.editing ?
const rowElement = item.editing ?
this.renderEditItem(item, idx) :
this.renderDataItem(item, idx, listFocused);
rowElement.setAttribute('role', 'listitem');
return rowElement;
}
private renderDataItem(item: IListViewItem<TDataItem>, idx: number, listFocused: boolean): HTMLElement {
@ -353,12 +350,8 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
actionBar.push(this.getActionsForItem(item, idx), { icon: true, label: true });
rowElement.title = this.getLocalizedRowTitle(item);
if (item.selected) {
if (listFocused) {
setTimeout(() => {
rowElement.focus();
}, 10);
}
if (item.selected && listFocused) {
this.listDisposables.add(disposableTimeout(() => rowElement.focus()));
}
return rowElement;
@ -427,8 +420,7 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
return;
}
this.model.select(targetIdx);
this.renderList();
this.selectRow(targetIdx);
e.preventDefault();
e.stopPropagation();
}
@ -471,6 +463,26 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
const targetIdx = parseInt(targetIdxStr);
return targetIdx;
}
private selectRow(idx: number): void {
this.model.select(idx);
this.rowElements.forEach(row => row.classList.remove('selected'));
const selectedRow = this.rowElements[this.model.getSelected()!];
selectedRow.classList.add('selected');
selectedRow.focus();
}
private selectNextRow(): void {
this.model.selectNext();
this.selectRow(this.model.getSelected()!);
}
private selectPreviousRow(): void {
this.model.selectPrevious();
this.selectRow(this.model.getSelected()!);
}
}
export interface IListDataItem {
@ -508,7 +520,6 @@ export class ListSettingWidget extends AbstractListSettingWidget<IListDataItem>
protected renderItem(item: IListDataItem): HTMLElement {
const rowElement = $('.setting-list-row');
const valueElement = DOM.append(rowElement, $('.setting-list-value'));
const siblingElement = DOM.append(rowElement, $('.setting-list-sibling'));
@ -758,6 +769,7 @@ export class ObjectSettingWidget extends AbstractListSettingWidget<IObjectDataIt
keyWidget = this.renderEditWidget(item.key, rowElement, true);
} else {
const keyElement = DOM.append(rowElement, $('.setting-list-object-key'));
keyElement.setAttribute('aria-readonly', 'true');
keyElement.textContent = item.key.data;
}
@ -842,11 +854,18 @@ export class ObjectSettingWidget extends AbstractListSettingWidget<IObjectDataIt
}
protected getLocalizedRowTitle(item: IObjectDataItem): string {
const enumDescription = item.key.type === 'enum'
let enumDescription = item.key.type === 'enum'
? item.key.options.find(({ value }) => item.key.data === value)?.description
: undefined;
return enumDescription ?? localize('objectPairHintLabel', "The key `{0}` maps to `{1}`", item.key.data, item.value.data);
// avoid rendering double '.'
if (isDefined(enumDescription) && enumDescription.endsWith('.')) {
enumDescription = enumDescription.slice(0, enumDescription.length - 1);
}
return isDefined(enumDescription)
? `${enumDescription}. Currently set to ${item.value.data}.`
: localize('objectPairHintLabel', "The property `{0}` is set to `{1}`.", item.key.data, item.value.data);
}
protected getLocalizedStrings() {