Clean up settings editor aria label code #106897

src/vs/workbench/contrib/preferences/browser/settingsTree.ts

@@ -690,9 +690,6 @@ export abstract class AbstractSettingRenderer extends Disposable implements ITre
 			template.descriptionElement.innerText = element.description;
 		}
 
-		const baseId = (element.displayCategory + '_' + element.displayLabel).replace(/ /g, '_').toLowerCase();
-		template.descriptionElement.id = baseId + '_setting_description';
-
 		template.otherOverridesElement.innerText = '';
 		template.otherOverridesElement.style.display = 'none';
 		if (element.overriddenScopeList.length) {
@@ -776,68 +773,6 @@ export abstract class AbstractSettingRenderer extends Disposable implements ITre
 
 	protected abstract renderValue(dataElement: SettingsTreeSettingElement, template: ISettingItemTemplate, onChange: (value: any) => void): void;
 
-	protected setElementAriaLabels(dataElement: SettingsTreeSettingElement, templateId: string, template: ISettingItemTemplate): string {
-		// Create base Id for element references
-		const baseId = (dataElement.displayCategory + '_' + dataElement.displayLabel).replace(/ /g, '_').toLowerCase();
-
-		const modifiedText = template.otherOverridesElement.textContent ?
-			template.otherOverridesElement.textContent : (dataElement.isConfigured ? localize('settings.Modified', ' Modified. ') : '');
-
-		let itemElement: HTMLElement | null = null;
-
-		// Use '.' as reader pause
-		let label = dataElement.displayCategory + ' ' + dataElement.displayLabel + '. ';
-
-		// Setup and add ARIA attributes
-		// Create id and label for control/input element - parent is wrapper div
-
-		if (templateId === SETTINGS_TEXT_TEMPLATE_ID) {
-			if (itemElement = (<ISettingTextItemTemplate>template).inputBox.inputElement) {
-				itemElement.setAttribute('role', 'textbox');
-				label += modifiedText;
-			}
-		} else if (templateId === SETTINGS_NUMBER_TEMPLATE_ID) {
-			if (itemElement = (<ISettingNumberItemTemplate>template).inputBox.inputElement) {
-				itemElement.setAttribute('role', 'textbox');
-				label += ' number. ' + modifiedText;
-			}
-		} else if (templateId === SETTINGS_BOOL_TEMPLATE_ID) {
-			if (itemElement = (<ISettingBoolItemTemplate>template).checkbox.domNode) {
-				itemElement.setAttribute('role', 'checkbox');
-				label += modifiedText;
-				// Add checkbox target to description clickable and able to toggle checkbox
-				template.descriptionElement.setAttribute('checkbox_label_target_id', baseId + '_setting_item');
-			}
-		} else if (templateId === SETTINGS_ENUM_TEMPLATE_ID) {
-			if (itemElement = <HTMLElement>template.controlElement.firstElementChild) {
-				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 '';
-		}
-
-		// We don't have control element, return empty label
-		if (!itemElement) {
-			return '';
-		}
-
-		// Labels will not be read on descendent input elements of the parent treeitem
-		// unless defined as roles for input items
-		// voiceover does not seem to use labeledby correctly, set labels directly on input elements
-		itemElement.id = baseId + '_setting_item';
-		itemElement.setAttribute('aria-label', label);
-		itemElement.setAttribute('aria-describedby', baseId + '_setting_description settings_aria_more_actions_shortcut_label');
-
-		return label;
-	}
-
 	disposeTemplate(template: IDisposableTemplate): void {
 		dispose(template.toDispose);
 	}
@@ -1187,8 +1122,6 @@ export class SettingObjectRenderer extends AbstractSettingRenderer implements IT
 			valueSuggester: createObjectValueSuggester(dataElement),
 		});
 
-		this.setElementAriaLabels(dataElement, this.templateId, template);
-
 		template.context = dataElement;
 	}
 }
@@ -1317,17 +1250,15 @@ export class SettingTextRenderer extends AbstractSettingRenderer implements ITre
 	}
 
 	protected renderValue(dataElement: SettingsTreeSettingElement, template: ISettingTextItemTemplate, onChange: (value: string) => void): void {
-		const label = this.setElementAriaLabels(dataElement, SETTINGS_TEXT_TEMPLATE_ID, template);
-
 		template.onChange = undefined;
 		template.inputBox.value = dataElement.value;
 		template.onChange = value => {
-			if (!renderValidations(dataElement, template, false, label)) {
+			if (!renderValidations(dataElement, template, false)) {
 				onChange(value);
 			}
 		};
 
-		renderValidations(dataElement, template, true, label);
+		renderValidations(dataElement, template, true);
 	}
 }
 
@@ -1404,9 +1335,6 @@ export class SettingEnumRenderer extends AbstractSettingRenderer implements ITre
 
 		template.selectBox.setOptions(displayOptions);
 
-		const label = this.setElementAriaLabels(dataElement, SETTINGS_ENUM_TEMPLATE_ID, template);
-		template.selectBox.setAriaLabel(label);
-
 		let idx = dataElement.setting.enum!.indexOf(dataElement.value);
 		if (idx === -1) {
 			idx = dataElement.setting.enum!.indexOf(dataElement.defaultValue);
@@ -1469,17 +1397,15 @@ export class SettingNumberRenderer extends AbstractSettingRenderer implements IT
 		const nullNumParseFn = (dataElement.valueType === 'nullable-integer' || dataElement.valueType === 'nullable-number')
 			? ((v: string) => v === '' ? null : numParseFn(v)) : numParseFn;
 
-		const label = this.setElementAriaLabels(dataElement, SETTINGS_NUMBER_TEMPLATE_ID, template);
-
 		template.onChange = undefined;
 		template.inputBox.value = dataElement.value;
 		template.onChange = value => {
-			if (!renderValidations(dataElement, template, false, label)) {
+			if (!renderValidations(dataElement, template, false)) {
 				onChange(nullNumParseFn(value));
 			}
 		};
 
-		renderValidations(dataElement, template, true, label);
+		renderValidations(dataElement, template, true);
 	}
 }
 
@@ -1572,9 +1498,6 @@ export class SettingBoolRenderer extends AbstractSettingRenderer implements ITre
 		template.onChange = undefined;
 		template.checkbox.checked = dataElement.value;
 		template.onChange = onChange;
-
-		// Setup and add ARIA attributes
-		this.setElementAriaLabels(dataElement, SETTINGS_BOOL_TEMPLATE_ID, template);
 	}
 }
 
@@ -1693,18 +1616,18 @@ export class SettingTreeRenderers {
 /**
  * Validate and render any error message. Returns true if the value is invalid.
  */
-function renderValidations(dataElement: SettingsTreeSettingElement, template: ISettingTextItemTemplate, calledOnStartup: boolean, originalAriaLabel: string): boolean {
+function renderValidations(dataElement: SettingsTreeSettingElement, template: ISettingTextItemTemplate, calledOnStartup: boolean): boolean {
 	if (dataElement.setting.validator) {
 		const errMsg = dataElement.setting.validator(template.inputBox.value);
 		if (errMsg) {
 			DOM.addClass(template.containerElement, 'invalid-input');
 			template.validationErrorMessageElement.innerText = errMsg;
 			const validationError = localize('validationError', "Validation Error.");
-			template.inputBox.inputElement.parentElement!.setAttribute('aria-label', [originalAriaLabel, validationError, errMsg].join(' '));
+			template.inputBox.inputElement.parentElement!.setAttribute('aria-label', [validationError, errMsg].join(' '));
 			if (!calledOnStartup) { ariaAlert(validationError + ' ' + errMsg); }
 			return true;
 		} else {
-			template.inputBox.inputElement.parentElement!.setAttribute('aria-label', originalAriaLabel);
+			template.inputBox.inputElement.parentElement!.removeAttribute('aria-label');
 		}
 	}
 	DOM.removeClass(template.containerElement, 'invalid-input');
@@ -1923,9 +1846,13 @@ export class SettingsTree extends WorkbenchObjectTree<SettingsTreeElement> {
 					}
 				},
 				accessibilityProvider: {
-					getAriaLabel() {
-						// TODO@roblourens https://github.com/microsoft/vscode/issues/95862
-						return '';
+					getAriaLabel(element: SettingsTreeElement) {
+						// const modifiedText = template.otherOverridesElement.textContent ?
+						// 	template.otherOverridesElement.textContent : (dataElement.isConfigured ? localize('settings.Modified', ' Modified. ') : '');
+
+						return (element instanceof SettingsTreeSettingElement) ?
+							`${element.displayCategory} ${element.displayLabel}. ${element.description}` :
+							element.id;
 					},
 					getWidgetAriaLabel() {
 						return localize('settings', "Settings");

src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts

@@ -263,6 +263,7 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
 		super();
 
 		this.listElement = DOM.append(container, $('div'));
+		this.listElement.setAttribute('role', 'list');
 		this.getContainerClasses().forEach(c => this.listElement.classList.add(c));
 		this.listElement.setAttribute('tabindex', '0');
 		DOM.append(container, this.renderAddButton());
@@ -379,6 +380,7 @@ abstract class AbstractListSettingWidget<TDataItem extends object> extends Dispo
 
 		actionBar.push(this.getActionsForItem(item, idx), { icon: true, label: true });
 		rowElement.title = this.getLocalizedRowTitle(item);
+		rowElement.setAttribute('aria-label', rowElement.title);
 
 		if (item.selected && listFocused) {
 			this.listDisposables.add(disposableTimeout(() => rowElement.focus()));