Skip to content

Commit adfb849

Browse files
cleidighcleidigh
authored
Settings: Labeling for "Modified in ..." Fixes microsoft#59492 (microsoft#60226)
* Settings: Labeling for "Modified in ..." Fixes microsoft#59492 * Eliminate unnecessary empty itemElement object * Remove label on Booleans * Test, Cleanup
1 parent 97720f6 commit adfb849

1 file changed

Lines changed: 70 additions & 75 deletions

File tree

‎src/vs/workbench/parts/preferences/browser/settingsTree.ts‎

Lines changed: 70 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,6 @@ export class SettingsRenderer implements ITreeRenderer {
10531053
template.labelElement.textContent = element.displayLabel;
10541054
template.labelElement.title = titleTooltip;
10551055

1056-
this.renderValue(element, templateId, <ISettingItemTemplate>template);
10571056
template.descriptionElement.innerHTML = '';
10581057
if (element.setting.descriptionIsMarkdown) {
10591058
const renderedDescription = this.renderDescriptionMarkdown(element, element.description, template.toDispose);
@@ -1065,11 +1064,6 @@ export class SettingsRenderer implements ITreeRenderer {
10651064
const baseId = (element.displayCategory + '_' + element.displayLabel).replace(/ /g, '_').toLowerCase();
10661065
template.descriptionElement.id = baseId + '_setting_description';
10671066

1068-
if (templateId === SETTINGS_BOOL_TEMPLATE_ID) {
1069-
// Add checkbox target to description clickable and able to toggle checkbox
1070-
template.descriptionElement.setAttribute('checkbox_label_target_id', baseId + '_setting_item');
1071-
}
1072-
10731067
template.otherOverridesElement.innerHTML = '';
10741068

10751069
if (element.overriddenScopeList.length) {
@@ -1097,8 +1091,11 @@ export class SettingsRenderer implements ITreeRenderer {
10971091
e.stopPropagation();
10981092
});
10991093
}
1094+
11001095
}
11011096

1097+
this.renderValue(element, templateId, <ISettingItemTemplate>template);
1098+
11021099
// Remove tree attributes - sometimes overridden by tree - should be managed there
11031100
template.containerElement.parentElement.removeAttribute('role');
11041101
template.containerElement.parentElement.removeAttribute('aria-level');
@@ -1165,23 +1162,7 @@ export class SettingsRenderer implements ITreeRenderer {
11651162
template.onChange = onChange;
11661163

11671164
// Setup and add ARIA attributes
1168-
// Create id and label for control/input element - parent is wrapper div
1169-
const baseId = (dataElement.displayCategory + '_' + dataElement.displayLabel).replace(/ /g, '_').toLowerCase();
1170-
const modifiedText = dataElement.isConfigured ? 'Modified' : '';
1171-
const label = dataElement.displayCategory + ' ' + dataElement.displayLabel + ' ' + modifiedText;
1172-
1173-
// We use the parent control div for the aria-labelledby target
1174-
// Does not appear you can use the direct label on the element itself within a tree
1175-
template.checkbox.domNode.parentElement.id = baseId + '_setting_label';
1176-
template.checkbox.domNode.parentElement.setAttribute('aria-label', label);
1177-
1178-
// Labels will not be read on descendent input elements of the parent treeitem
1179-
// unless defined as role=treeitem and indirect aria-labelledby approach
1180-
template.checkbox.domNode.id = baseId + '_setting_item';
1181-
template.checkbox.domNode.setAttribute('role', 'checkbox');
1182-
template.checkbox.domNode.setAttribute('aria-labelledby', baseId + '_setting_label');
1183-
template.checkbox.domNode.setAttribute('aria-describedby', baseId + '_setting_description');
1184-
1165+
this.setElementAriaLabels(dataElement, SETTINGS_BOOL_TEMPLATE_ID, template);
11851166
}
11861167

11871168
private renderEnum(dataElement: SettingsTreeSettingElement, template: ISettingEnumItemTemplate, onChange: (value: string) => void): void {
@@ -1198,88 +1179,41 @@ export class SettingsRenderer implements ITreeRenderer {
11981179
isMarkdown: enumDescriptionsAreMarkdown
11991180
}));
12001181

1201-
const modifiedText = dataElement.isConfigured ? 'Modified' : '';
1202-
1203-
// Use ',.' as reader pause
1204-
const label = dataElement.displayCategory + ' ' + dataElement.displayLabel + ',. ' + modifiedText;
1205-
const baseId = (dataElement.displayCategory + '_' + dataElement.displayLabel).replace(/ /g, '_').toLowerCase();
1206-
1182+
const label = this.setElementAriaLabels(dataElement, SETTINGS_ENUM_TEMPLATE_ID, template);
12071183
template.selectBox.setAriaLabel(label);
12081184

12091185
const idx = dataElement.setting.enum.indexOf(dataElement.value);
12101186
template.onChange = null;
12111187
template.selectBox.select(idx);
12121188
template.onChange = idx => onChange(dataElement.setting.enum[idx]);
12131189

1214-
if (template.controlElement.firstElementChild) {
1215-
// SelectBox needs to have treeitem changed to combobox to read correctly within tree
1216-
template.controlElement.firstElementChild.setAttribute('role', 'combobox');
1217-
template.controlElement.firstElementChild.setAttribute('aria-describedby', baseId + '_setting_description settings_aria_more_actions_shortcut_label');
1218-
}
1219-
12201190
template.enumDescriptionElement.innerHTML = '';
12211191
}
12221192

12231193
private renderText(dataElement: SettingsTreeSettingElement, template: ISettingTextItemTemplate, onChange: (value: string) => void): void {
1224-
const modifiedText = dataElement.isConfigured ? 'Modified' : '';
12251194

1226-
// Use ',.' as reader pause
1227-
const label = dataElement.displayCategory + ' ' + dataElement.displayLabel + ',. ' + modifiedText;
1195+
const label = this.setElementAriaLabels(dataElement, SETTINGS_TEXT_TEMPLATE_ID, template);
1196+
12281197
template.onChange = null;
12291198
template.inputBox.value = dataElement.value;
12301199
template.onChange = value => { renderValidations(dataElement, template, false, label); onChange(value); };
12311200

1232-
// Setup and add ARIA attributes
1233-
// Create id and label for control/input element - parent is wrapper div
1234-
const baseId = (dataElement.displayCategory + '_' + dataElement.displayLabel).replace(/ /g, '_').toLowerCase();
1235-
1236-
// We use the parent control div for the aria-labelledby target
1237-
// Does not appear you can use the direct label on the element itself within a tree
1238-
template.inputBox.inputElement.parentElement.id = baseId + '_setting_label';
1239-
template.inputBox.inputElement.parentElement.setAttribute('aria-label', label);
1240-
1241-
// Labels will not be read on descendent input elements of the parent treeitem
1242-
// unless defined as role=treeitem and indirect aria-labelledby approach
1243-
template.inputBox.inputElement.id = baseId + '_setting_item';
1244-
template.inputBox.inputElement.setAttribute('role', 'textbox');
1245-
template.inputBox.inputElement.setAttribute('aria-labelledby', baseId + '_setting_label');
1246-
template.inputBox.inputElement.setAttribute('aria-describedby', baseId + '_setting_description settings_aria_more_actions_shortcut_label');
1247-
12481201
renderValidations(dataElement, template, true, label);
12491202
}
12501203

1251-
12521204
private renderNumber(dataElement: SettingsTreeSettingElement, template: ISettingTextItemTemplate, onChange: (value: number) => void): void {
1253-
const modifiedText = dataElement.isConfigured ? 'Modified' : '';
1254-
1255-
// Use ',.' as reader pause
1256-
const label = dataElement.displayCategory + ' ' + dataElement.displayLabel + ' number,. ' + modifiedText;
12571205
const numParseFn = (dataElement.valueType === 'integer' || dataElement.valueType === 'nullable-integer')
12581206
? parseInt : parseFloat;
12591207

12601208
const nullNumParseFn = (dataElement.valueType === 'nullable-integer' || dataElement.valueType === 'nullable-number')
12611209
? (v => v === '' ? null : numParseFn(v)) : numParseFn;
12621210

1211+
const label = this.setElementAriaLabels(dataElement, SETTINGS_NUMBER_TEMPLATE_ID, template);
1212+
12631213
template.onChange = null;
12641214
template.inputBox.value = dataElement.value;
12651215
template.onChange = value => { renderValidations(dataElement, template, false, label); onChange(nullNumParseFn(value)); };
12661216

1267-
// Setup and add ARIA attributes
1268-
// Create id and label for control/input element - parent is wrapper div
1269-
const baseId = (dataElement.displayCategory + '_' + dataElement.displayLabel).replace(/ /g, '_').toLowerCase();
1270-
1271-
// We use the parent control div for the aria-labelledby target
1272-
// Does not appear you can use the direct label on the element itself within a tree
1273-
template.inputBox.inputElement.parentElement.id = baseId + '_setting_label';
1274-
template.inputBox.inputElement.parentElement.setAttribute('aria-label', label);
1275-
1276-
// Labels will not be read on descendent input elements of the parent treeitem
1277-
// unless defined as role=treeitem and indirect aria-labelledby approach
1278-
template.inputBox.inputElement.id = baseId + '_setting_item';
1279-
template.inputBox.inputElement.setAttribute('role', 'textbox');
1280-
template.inputBox.inputElement.setAttribute('aria-labelledby', baseId + '_setting_label');
1281-
template.inputBox.inputElement.setAttribute('aria-describedby', baseId + '_setting_description settings_aria_more_actions_shortcut_label');
1282-
12831217
renderValidations(dataElement, template, true, label);
12841218
}
12851219

@@ -1293,6 +1227,64 @@ export class SettingsRenderer implements ITreeRenderer {
12931227
template.onChange = () => this._onDidOpenSettings.fire(dataElement.setting.key);
12941228
}
12951229

1230+
1231+
private setElementAriaLabels(dataElement: SettingsTreeSettingElement, templateId: string, template: ISettingItemTemplate): string {
1232+
// Create base Id for element references
1233+
const baseId = (dataElement.displayCategory + '_' + dataElement.displayLabel).replace(/ /g, '_').toLowerCase();
1234+
1235+
const modifiedText = template.otherOverridesElement.textContent ?
1236+
template.otherOverridesElement.textContent : (dataElement.isConfigured ? localize('settings.Modified', ' Modified. ') : '');
1237+
1238+
let itemElement = null;
1239+
1240+
// Use '.' as reader pause
1241+
let label = dataElement.displayCategory + ' ' + dataElement.displayLabel + '. ';
1242+
1243+
// Setup and add ARIA attributes
1244+
// Create id and label for control/input element - parent is wrapper div
1245+
1246+
if (templateId === SETTINGS_TEXT_TEMPLATE_ID) {
1247+
if (itemElement = (<ISettingTextItemTemplate>template).inputBox.inputElement) {
1248+
itemElement.setAttribute('role', 'textbox');
1249+
label += modifiedText;
1250+
}
1251+
} else if (templateId === SETTINGS_NUMBER_TEMPLATE_ID) {
1252+
if (itemElement = (<ISettingNumberItemTemplate>template).inputBox.inputElement) {
1253+
itemElement.setAttribute('role', 'textbox');
1254+
label += ' number. ' + modifiedText;
1255+
}
1256+
} else if (templateId === SETTINGS_BOOL_TEMPLATE_ID) {
1257+
if (itemElement = (<ISettingBoolItemTemplate>template).checkbox.domNode) {
1258+
itemElement.setAttribute('role', 'checkbox');
1259+
label += modifiedText;
1260+
// Add checkbox target to description clickable and able to toggle checkbox
1261+
template.descriptionElement.setAttribute('checkbox_label_target_id', baseId + '_setting_item');
1262+
}
1263+
} else if (templateId === SETTINGS_ENUM_TEMPLATE_ID) {
1264+
if (itemElement = template.controlElement.firstElementChild) {
1265+
itemElement.setAttribute('role', 'combobox');
1266+
label += modifiedText;
1267+
}
1268+
} else {
1269+
// Don't change attributes if we don't know what we areFunctions
1270+
return '';
1271+
}
1272+
1273+
// We don't have control element, return empty label
1274+
if (!itemElement) {
1275+
return '';
1276+
}
1277+
1278+
// Labels will not be read on descendent input elements of the parent treeitem
1279+
// unless defined as roles for input items
1280+
// voiceover does not seem to use labeledby correctly, set labels directly on input elements
1281+
itemElement.id = baseId + '_setting_item';
1282+
itemElement.setAttribute('aria-label', label);
1283+
itemElement.setAttribute('aria-describedby', baseId + '_setting_description settings_aria_more_actions_shortcut_label');
1284+
1285+
return label;
1286+
}
1287+
12961288
disposeTemplate(tree: ITree, templateId: string, template: IDisposableTemplate): void {
12971289
dispose(template.toDispose);
12981290
}
@@ -1429,6 +1421,9 @@ export class SettingsAccessibilityProvider implements IAccessibilityProvider {
14291421
}
14301422

14311423
if (element instanceof SettingsTreeSettingElement) {
1424+
if (element.valueType === 'boolean') {
1425+
return '';
1426+
}
14321427
return localize('settingRowAriaLabel', "{0} {1}, Setting", element.displayCategory, element.displayLabel);
14331428
}
14341429

0 commit comments

Comments
 (0)