From 7d511ba4ae886f29ae813bab1523cf32de671414 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 21 Aug 2020 04:15:02 +0200 Subject: [PATCH] fix(material/input): do not override existing aria-describedby value (#19587) * fix(material/input): do not override existing aria-describedby value The form-field notifies controls whenever hints or errors have been displayed. It does this, so that controls like the input can refresh their `aria-describedby` value to point to the errors and hints. This currently has the downside of overriding existing `aria-describedby` values that have been manually set by developers. We could fix this by reading the initial value and merging it with the ids received from the form-field. * fixup! fix(material/input): do not override existing aria-describedby value Address feedback * fixup! fix(material/input): do not override existing aria-describedby value Add documentation --- .../creating-a-custom-form-field-control.md | 31 +++++++--- .../example-tel-input-example.html | 3 +- .../form-field-custom-control-example.ts | 7 ++- .../example-tel-input-example.html | 3 +- .../form-field-custom-control-example.ts | 7 ++- .../mdc-form-field/form-field.ts | 6 +- .../mdc-input/input.spec.ts | 59 +++++++++++++++++-- src/material-experimental/mdc-input/input.ts | 1 - src/material/form-field/form-field-control.ts | 6 ++ src/material/form-field/form-field.ts | 6 +- src/material/input/input.spec.ts | 59 +++++++++++++++++-- src/material/input/input.ts | 47 ++++++++------- .../public_api_guard/material/form-field.d.ts | 1 + tools/public_api_guard/material/input.d.ts | 4 +- 14 files changed, 190 insertions(+), 50 deletions(-) diff --git a/guides/creating-a-custom-form-field-control.md b/guides/creating-a-custom-form-field-control.md index ce6e3bc6097d..4b7be7cdeaab 100644 --- a/guides/creating-a-custom-form-field-control.md +++ b/guides/creating-a-custom-form-field-control.md @@ -339,20 +339,33 @@ controlType = 'example-tel-input'; #### `setDescribedByIds(ids: string[])` -This method is used by the `` to specify the IDs that should be used for the -`aria-describedby` attribute of your component. The method has one parameter, the list of IDs, we -just need to apply the given IDs to the element that represents our control. +This method is used by the `` to set element ids that should be used for the +`aria-describedby` attribute of your control. The ids are controlled through the form field +as hints or errors are conditionally displayed and should be reflected in the control's +`aria-describedby` attribute for an improved accessibility experience. -In our concrete example, these IDs would need to be applied to the group element. +The `setDescribedByIds` method is invoked whenever the control's state changes. Custom controls +need to implement this method and update the `aria-describedby` attribute based on the specified +element ids. Below is an example that shows how this can be achieved. + +Note that the method by default will not respect element ids that have been set manually on the +control element through the `aria-describedby` attribute. To ensure that your control does not +accidentally override existing element ids specified by consumers of your control, create an +input called `userAriaDescribedby` like followed: ```ts -setDescribedByIds(ids: string[]) { - this.describedBy = ids.join(' '); -} +@Input('aria-describedby') userAriaDescribedBy: string; ``` -```html -
+The form field will then pick up the user specified `aria-describedby` ids and merge +them with ids for hints or errors whenever `setDescribedByIds` is invoked. + +```ts +setDescribedByIds(ids: string[]) { + const controlElement = this._elementRef.nativeElement + .querySelector('.example-tel-input-container')!; + controlElement.setAttribute('aria-describedby', ids.join(' ')); +} ``` #### `onContainerClick(event: MouseEvent)` diff --git a/src/components-examples/material-experimental/mdc-form-field/mdc-form-field-custom-control/example-tel-input-example.html b/src/components-examples/material-experimental/mdc-form-field/mdc-form-field-custom-control/example-tel-input-example.html index 69ad20628e84..2f909a215a22 100644 --- a/src/components-examples/material-experimental/mdc-form-field/mdc-form-field-custom-control/example-tel-input-example.html +++ b/src/components-examples/material-experimental/mdc-form-field/mdc-form-field-custom-control/example-tel-input-example.html @@ -1,7 +1,6 @@
+ [attr.aria-labelledby]="_formField?.getLabelId()"> {}; onTouched = () => {}; @@ -50,6 +49,8 @@ export class MyTelInput implements ControlValueAccessor, MatFormFieldControl + [attr.aria-labelledby]="_formField?.getLabelId()"> {}; onTouched = () => {}; @@ -81,6 +80,8 @@ export class MyTelInput return this.focused || !this.empty; } + @Input('aria-describedby') userAriaDescribedBy: string; + @Input() get placeholder(): string { return this._placeholder; @@ -185,7 +186,9 @@ export class MyTelInput } setDescribedByIds(ids: string[]) { - this.describedBy = ids.join(' '); + const controlElement = this._elementRef.nativeElement + .querySelector('.example-tel-input-container')!; + controlElement.setAttribute('aria-describedby', ids.join(' ')); } onContainerClick() { diff --git a/src/material-experimental/mdc-form-field/form-field.ts b/src/material-experimental/mdc-form-field/form-field.ts index ab1c17d31685..5c082fb9133c 100644 --- a/src/material-experimental/mdc-form-field/form-field.ts +++ b/src/material-experimental/mdc-form-field/form-field.ts @@ -584,6 +584,10 @@ export class MatFormField implements AfterViewInit, OnDestroy, AfterContentCheck if (this._control) { let ids: string[] = []; + if (this._control.userAriaDescribedBy) { + ids.push(...this._control.userAriaDescribedBy.split(' ')); + } + if (this._getDisplayedMessages() === 'hint') { const startHint = this._hintChildren ? this._hintChildren.find(hint => hint.align === 'start') : null; @@ -600,7 +604,7 @@ export class MatFormField implements AfterViewInit, OnDestroy, AfterContentCheck ids.push(endHint.id); } } else if (this._errorChildren) { - ids = this._errorChildren.map(error => error.id); + ids.push(...this._errorChildren.map(error => error.id)); } this._control.setDescribedByIds(ids); diff --git a/src/material-experimental/mdc-input/input.spec.ts b/src/material-experimental/mdc-input/input.spec.ts index 6cd968b8c617..c9c5680a5781 100644 --- a/src/material-experimental/mdc-input/input.spec.ts +++ b/src/material-experimental/mdc-input/input.spec.ts @@ -474,10 +474,44 @@ describe('MatMdcInput without forms', () => { fixture.componentInstance.label = 'label'; fixture.detectChanges(); - let hint = fixture.debugElement.query(By.css('.mat-mdc-form-field-hint'))!.nativeElement; - let input = fixture.debugElement.query(By.css('input'))!.nativeElement; + const hint = fixture.debugElement.query(By.css('.mat-mdc-form-field-hint'))!.nativeElement; + const input = fixture.debugElement.query(By.css('input'))!.nativeElement; + const hintId = hint.getAttribute('id'); - expect(input.getAttribute('aria-describedby')).toBe(hint.getAttribute('id')); + expect(input.getAttribute('aria-describedby')).toBe(`initial ${hintId}`); + })); + + it('supports user binding to aria-describedby', fakeAsync(() => { + let fixture = createComponent(MatInputWithSubscriptAndAriaDescribedBy); + + fixture.componentInstance.label = 'label'; + fixture.detectChanges(); + + const hint = fixture.debugElement.query(By.css('.mat-mdc-form-field-hint'))!.nativeElement; + const input = fixture.debugElement.query(By.css('input'))!.nativeElement; + const hintId = hint.getAttribute('id'); + + expect(input.getAttribute('aria-describedby')).toBe(hintId); + + fixture.componentInstance.userDescribedByValue = 'custom-error custom-error-two'; + fixture.detectChanges(); + expect(input.getAttribute('aria-describedby')).toBe(`custom-error custom-error-two ${hintId}`); + + fixture.componentInstance.userDescribedByValue = 'custom-error'; + fixture.detectChanges(); + expect(input.getAttribute('aria-describedby')).toBe(`custom-error ${hintId}`); + + fixture.componentInstance.showError = true; + fixture.componentInstance.formControl.markAsTouched(); + fixture.componentInstance.formControl.setErrors({invalid: true}); + fixture.detectChanges(); + expect(input.getAttribute('aria-describedby')).toMatch(/^custom-error mat-mdc-error-\d+$/); + + fixture.componentInstance.label = ''; + fixture.componentInstance.userDescribedByValue = ''; + fixture.componentInstance.showError = false; + fixture.detectChanges(); + expect(input.hasAttribute('aria-describedby')).toBe(false); })); it('sets the aria-describedby to the id of the mat-hint', fakeAsync(() => { @@ -1253,12 +1287,29 @@ class MatInputHintLabel2TestController { } @Component({ - template: `` + template: ` + + + ` }) class MatInputHintLabelTestController { label: string = ''; } +@Component({ + template: ` + + + Some error + ` +}) +class MatInputWithSubscriptAndAriaDescribedBy { + label: string = ''; + userDescribedByValue: string = ''; + showError = false; + formControl = new FormControl(); +} + @Component({template: ``}) class MatInputInvalidTypeTestController { t = 'file'; diff --git a/src/material-experimental/mdc-input/input.ts b/src/material-experimental/mdc-input/input.ts index 4c75fd1f11fb..f2aedb13d52c 100644 --- a/src/material-experimental/mdc-input/input.ts +++ b/src/material-experimental/mdc-input/input.ts @@ -33,7 +33,6 @@ import {MatInput as BaseMatInput} from '@angular/material/input'; '[required]': 'required', '[attr.placeholder]': 'placeholder', '[attr.readonly]': 'readonly && !_isNativeSelect || null', - '[attr.aria-describedby]': '_ariaDescribedby || null', '[attr.aria-invalid]': 'errorState', '[attr.aria-required]': 'required.toString()', }, diff --git a/src/material/form-field/form-field-control.ts b/src/material/form-field/form-field-control.ts index 627ce45163fb..1f0ac7295c8c 100644 --- a/src/material/form-field/form-field-control.ts +++ b/src/material/form-field/form-field-control.ts @@ -63,6 +63,12 @@ export abstract class MatFormFieldControl { */ readonly autofilled?: boolean; + /** + * Value of `aria-describedby` that should be merged with the described-by ids + * which are set by the form-field. + */ + readonly userAriaDescribedBy?: string; + /** Sets the list of element IDs that currently describe this control. */ abstract setDescribedByIds(ids: string[]): void; diff --git a/src/material/form-field/form-field.ts b/src/material/form-field/form-field.ts index a25864928f30..398aab49a123 100644 --- a/src/material/form-field/form-field.ts +++ b/src/material/form-field/form-field.ts @@ -507,6 +507,10 @@ export class MatFormField extends _MatFormFieldMixinBase if (this._control) { let ids: string[] = []; + if (this._control.userAriaDescribedBy) { + ids.push(...this._control.userAriaDescribedBy.split(' ')); + } + if (this._getDisplayedMessages() === 'hint') { const startHint = this._hintChildren ? this._hintChildren.find(hint => hint.align === 'start') : null; @@ -523,7 +527,7 @@ export class MatFormField extends _MatFormFieldMixinBase ids.push(endHint.id); } } else if (this._errorChildren) { - ids = this._errorChildren.map(error => error.id); + ids.push(...this._errorChildren.map(error => error.id)); } this._control.setDescribedByIds(ids); diff --git a/src/material/input/input.spec.ts b/src/material/input/input.spec.ts index 4aeede2eeeac..edbc58ab3ce7 100644 --- a/src/material/input/input.spec.ts +++ b/src/material/input/input.spec.ts @@ -564,10 +564,44 @@ describe('MatInput without forms', () => { fixture.componentInstance.label = 'label'; fixture.detectChanges(); - let hint = fixture.debugElement.query(By.css('.mat-hint'))!.nativeElement; - let input = fixture.debugElement.query(By.css('input'))!.nativeElement; + const hint = fixture.debugElement.query(By.css('.mat-hint'))!.nativeElement; + const input = fixture.debugElement.query(By.css('input'))!.nativeElement; + const hintId = hint.getAttribute('id'); - expect(input.getAttribute('aria-describedby')).toBe(hint.getAttribute('id')); + expect(input.getAttribute('aria-describedby')).toBe(`initial ${hintId}`); + })); + + it('supports user binding to aria-describedby', fakeAsync(() => { + let fixture = createComponent(MatInputWithSubscriptAndAriaDescribedBy); + + fixture.componentInstance.label = 'label'; + fixture.detectChanges(); + + const hint = fixture.debugElement.query(By.css('.mat-hint'))!.nativeElement; + const input = fixture.debugElement.query(By.css('input'))!.nativeElement; + const hintId = hint.getAttribute('id'); + + expect(input.getAttribute('aria-describedby')).toBe(hintId); + + fixture.componentInstance.userDescribedByValue = 'custom-error custom-error-two'; + fixture.detectChanges(); + expect(input.getAttribute('aria-describedby')).toBe(`custom-error custom-error-two ${hintId}`); + + fixture.componentInstance.userDescribedByValue = 'custom-error'; + fixture.detectChanges(); + expect(input.getAttribute('aria-describedby')).toBe(`custom-error ${hintId}`); + + fixture.componentInstance.showError = true; + fixture.componentInstance.formControl.markAsTouched(); + fixture.componentInstance.formControl.setErrors({invalid: true}); + fixture.detectChanges(); + expect(input.getAttribute('aria-describedby')).toMatch(/^custom-error mat-error-\d+$/); + + fixture.componentInstance.label = ''; + fixture.componentInstance.userDescribedByValue = ''; + fixture.componentInstance.showError = false; + fixture.detectChanges(); + expect(input.hasAttribute('aria-describedby')).toBe(false); })); it('sets the aria-describedby to the id of the mat-hint', fakeAsync(() => { @@ -1767,12 +1801,29 @@ class MatInputHintLabel2TestController { } @Component({ - template: `` + template: ` + + + ` }) class MatInputHintLabelTestController { label: string = ''; } +@Component({ + template: ` + + + Some error + ` +}) +class MatInputWithSubscriptAndAriaDescribedBy { + label: string = ''; + userDescribedByValue: string = ''; + showError = false; + formControl = new FormControl(); +} + @Component({template: ``}) class MatInputInvalidTypeTestController { t = 'file'; diff --git a/src/material/input/input.ts b/src/material/input/input.ts index 3c0bdb5eac6b..c08560d26cd8 100644 --- a/src/material/input/input.ts +++ b/src/material/input/input.ts @@ -10,9 +10,11 @@ import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion'; import {getSupportedInputTypes, Platform} from '@angular/cdk/platform'; import {AutofillMonitor} from '@angular/cdk/text-field'; import { + AfterViewInit, Directive, DoCheck, ElementRef, + HostListener, Inject, Input, NgZone, @@ -20,8 +22,6 @@ import { OnDestroy, Optional, Self, - HostListener, - AfterViewInit, } from '@angular/core'; import {FormGroupDirective, NgControl, NgForm} from '@angular/forms'; import { @@ -84,7 +84,6 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase = '[disabled]': 'disabled', '[required]': 'required', '[attr.readonly]': 'readonly && !_isNativeSelect || null', - '[attr.aria-describedby]': '_ariaDescribedby || null', '[attr.aria-invalid]': 'errorState', '[attr.aria-required]': 'required.toString()', }, @@ -97,9 +96,6 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl< private _inputValueAccessor: {value: any}; private _previousPlaceholder: string | null; - /** The aria-describedby attribute on the input for improved a11y. */ - _ariaDescribedby: string; - /** Whether the component is being rendered on the server. */ readonly _isServer: boolean; @@ -199,6 +195,12 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl< /** An object used to control when error messages are shown. */ @Input() errorStateMatcher: ErrorStateMatcher; + /** + * Implemented as part of MatFormFieldControl. + * @docs-private + */ + @Input('aria-describedby') userAriaDescribedBy: string; + /** * Implemented as part of MatFormFieldControl. * @docs-private @@ -228,19 +230,20 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl< ].filter(t => getSupportedInputTypes().has(t)); constructor( - protected _elementRef: ElementRef, - protected _platform: Platform, - /** @docs-private */ - @Optional() @Self() public ngControl: NgControl, - @Optional() _parentForm: NgForm, - @Optional() _parentFormGroup: FormGroupDirective, - _defaultErrorStateMatcher: ErrorStateMatcher, - @Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any, - private _autofillMonitor: AutofillMonitor, - ngZone: NgZone, - // TODO: Remove this once the legacy appearance has been removed. We only need - // to inject the form-field for determining whether the placeholder has been promoted. - @Optional() @Inject(MAT_FORM_FIELD) private _formField?: MatFormField) { + protected _elementRef: ElementRef, + protected _platform: Platform, + /** @docs-private */ + @Optional() @Self() public ngControl: NgControl, + @Optional() _parentForm: NgForm, + @Optional() _parentFormGroup: FormGroupDirective, + _defaultErrorStateMatcher: ErrorStateMatcher, + @Optional() @Self() @Inject(MAT_INPUT_VALUE_ACCESSOR) inputValueAccessor: any, + private _autofillMonitor: AutofillMonitor, + ngZone: NgZone, + // TODO: Remove this once the legacy appearance has been removed. We only need + // to inject the form-field for determining whether the placeholder has been promoted. + @Optional() @Inject(MAT_FORM_FIELD) private _formField?: MatFormField) { + super(_defaultErrorStateMatcher, _parentForm, _parentFormGroup, ngControl); const element = this._elementRef.nativeElement; @@ -440,7 +443,11 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl< * @docs-private */ setDescribedByIds(ids: string[]) { - this._ariaDescribedby = ids.join(' '); + if (ids.length) { + this._elementRef.nativeElement.setAttribute('aria-describedby', ids.join(' ')); + } else { + this._elementRef.nativeElement.removeAttribute('aria-describedby'); + } } /** diff --git a/tools/public_api_guard/material/form-field.d.ts b/tools/public_api_guard/material/form-field.d.ts index 5c0faef0543a..db2c4607da9e 100644 --- a/tools/public_api_guard/material/form-field.d.ts +++ b/tools/public_api_guard/material/form-field.d.ts @@ -96,6 +96,7 @@ export declare abstract class MatFormFieldControl { readonly required: boolean; readonly shouldLabelFloat: boolean; readonly stateChanges: Observable; + readonly userAriaDescribedBy?: string; value: T | null; abstract onContainerClick(event: MouseEvent): void; abstract setDescribedByIds(ids: string[]): void; diff --git a/tools/public_api_guard/material/input.d.ts b/tools/public_api_guard/material/input.d.ts index 27a888c6d481..037d59e00138 100644 --- a/tools/public_api_guard/material/input.d.ts +++ b/tools/public_api_guard/material/input.d.ts @@ -5,7 +5,6 @@ export declare const MAT_INPUT_VALUE_ACCESSOR: InjectionToken<{ }>; export declare class MatInput extends _MatInputMixinBase implements MatFormFieldControl, OnChanges, OnDestroy, AfterViewInit, DoCheck, CanUpdateErrorState { - _ariaDescribedby: string; protected _disabled: boolean; protected _elementRef: ElementRef; protected _id: string; @@ -37,6 +36,7 @@ export declare class MatInput extends _MatInputMixinBase implements MatFormField readonly stateChanges: Subject; get type(): string; set type(value: string); + userAriaDescribedBy: string; get value(): string; set value(value: string); constructor(_elementRef: ElementRef, _platform: Platform, @@ -58,7 +58,7 @@ export declare class MatInput extends _MatInputMixinBase implements MatFormField static ngAcceptInputType_readonly: BooleanInput; static ngAcceptInputType_required: BooleanInput; static ngAcceptInputType_value: any; - static ɵdir: i0.ɵɵDirectiveDefWithMeta; + static ɵdir: i0.ɵɵDirectiveDefWithMeta; static ɵfac: i0.ɵɵFactoryDef; }