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
angular · Aug 21, 2020 · 7d511ba · 7d511ba
1 parent 79e4d28
commit 7d511ba
Show file tree

Hide file tree

Showing 14 changed files with 190 additions and 50 deletions.
diff --git 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 `<mat-form-field>` 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 `<mat-form-field>` 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
-<div role="group" [formGroup]="parts" [attr.aria-describedby]="describedBy">
+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/...-experimental/mdc-form-field/mdc-form-field-custom-control/example-tel-input-example.html b/...-experimental/mdc-form-field/mdc-form-field-custom-control/example-tel-input-example.html
@@ -1,7 +1,6 @@
 <div role="group" class="example-tel-input-container"
      [formGroup]="parts"
-     [attr.aria-labelledby]="_formField?.getLabelId()"
-     [attr.aria-describedby]="describedBy">
+     [attr.aria-labelledby]="_formField?.getLabelId()">
   <input
     class="example-tel-input-element"
     formControlName="area" size="3"

diff --git a/...imental/mdc-form-field/mdc-form-field-custom-control/form-field-custom-control-example.ts b/...imental/mdc-form-field/mdc-form-field-custom-control/form-field-custom-control-example.ts
@@ -38,7 +38,6 @@ export class MyTelInput implements ControlValueAccessor, MatFormFieldControl<MyT
   errorState = false;
   controlType = 'example-tel-input';
   id = `example-tel-input-${MyTelInput.nextId++}`;
-  describedBy = '';
   onChange = (_: any) => {};
   onTouched = () => {};
 
@@ -50,6 +49,8 @@ export class MyTelInput implements ControlValueAccessor, MatFormFieldControl<MyT
 
   get shouldLabelFloat() { return this.focused || !this.empty; }
 
+  @Input('aria-describedby') userAriaDescribedBy: string;
+
   @Input()
   get placeholder(): string { return this._placeholder; }
   set placeholder(value: string) {
@@ -121,7 +122,9 @@ export class MyTelInput implements ControlValueAccessor, MatFormFieldControl<MyT
   }
 
   setDescribedByIds(ids: string[]) {
-    this.describedBy = ids.join(' ');
+    const controlElement = this._elementRef.nativeElement
+      .querySelector('.example-tel-input-container')!;
+    controlElement.setAttribute('aria-describedby', ids.join(' '));
   }
 
   onContainerClick(event: MouseEvent) {

diff --git a/...nts-examples/material/form-field/form-field-custom-control/example-tel-input-example.html b/...nts-examples/material/form-field/form-field-custom-control/example-tel-input-example.html
@@ -1,7 +1,6 @@
 <div role="group" class="example-tel-input-container"
      [formGroup]="parts"
-     [attr.aria-labelledby]="_formField?.getLabelId()"
-     [attr.aria-describedby]="describedBy">
+     [attr.aria-labelledby]="_formField?.getLabelId()">
   <input class="example-tel-input-element"
          formControlName="area" size="3"
          maxLength="3"

diff --git a/...amples/material/form-field/form-field-custom-control/form-field-custom-control-example.ts b/...amples/material/form-field/form-field-custom-control/form-field-custom-control-example.ts
@@ -65,7 +65,6 @@ export class MyTelInput
   focused = false;
   controlType = 'example-tel-input';
   id = `example-tel-input-${MyTelInput.nextId++}`;
-  describedBy = '';
   onChange = (_: any) => {};
   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
@@ -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
@@ -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: `<mat-form-field [hintLabel]="label"><input matInput></mat-form-field>`
+  template: `
+    <mat-form-field [hintLabel]="label">
+      <input matInput aria-describedby="initial">
+    </mat-form-field>`
 })
 class MatInputHintLabelTestController {
   label: string = '';
 }
 
+@Component({
+  template: `
+    <mat-form-field [hintLabel]="label">
+      <input matInput [formControl]="formControl" [aria-describedby]="userDescribedByValue">
+      <mat-error *ngIf="showError">Some error</mat-error>
+    </mat-form-field>`
+})
+class MatInputWithSubscriptAndAriaDescribedBy {
+  label: string = '';
+  userDescribedByValue: string = '';
+  showError = false;
+  formControl = new FormControl();
+}
+
 @Component({template: `<mat-form-field><input matInput [type]="t"></mat-form-field>`})
 class MatInputInvalidTypeTestController {
   t = 'file';

diff --git 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
@@ -63,6 +63,12 @@ export abstract class MatFormFieldControl<T> {
    */
   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
@@ -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
@@ -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: `<mat-form-field [hintLabel]="label"><input matInput></mat-form-field>`
+  template: `
+    <mat-form-field [hintLabel]="label">
+      <input matInput aria-describedby="initial">
+    </mat-form-field>`
 })
 class MatInputHintLabelTestController {
   label: string = '';
 }
 
+@Component({
+  template: `
+    <mat-form-field [hintLabel]="label">
+      <input matInput [formControl]="formControl" [aria-describedby]="userDescribedByValue">
+      <mat-error *ngIf="showError">Some error</mat-error>
+    </mat-form-field>`
+})
+class MatInputWithSubscriptAndAriaDescribedBy {
+  label: string = '';
+  userDescribedByValue: string = '';
+  showError = false;
+  formControl = new FormControl();
+}
+
 @Component({template: `<mat-form-field><input matInput [type]="t"></mat-form-field>`})
 class MatInputInvalidTypeTestController {
   t = 'file';