fix(material-experimental/mdc-chips): Mirror aria-describedby to matC…

…hipInput Updates mat-chip-grid to associate any ids set for aria-describedby to the matChipInput instance within the grid, if one exists. Removes the aria-describedby attribute on the grid itself since it never receives focus. Fixes angular#24542
ByzantineFailure · Mar 25, 2022 · e1a1b4f · e1a1b4f
1 parent 73dde84
commit e1a1b4f
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 44 deletions.
diff --git a/src/material-experimental/mdc-chips/chip-grid.spec.ts b/src/material-experimental/mdc-chips/chip-grid.spec.ts
@@ -853,13 +853,18 @@ describe('MDC-based MatChipGrid', () => {
     let errorTestComponent: ChipGridWithFormErrorMessages;
     let containerEl: HTMLElement;
     let chipGridEl: HTMLElement;
+    let inputEl: HTMLElement;
 
-    beforeEach(() => {
+    beforeEach(fakeAsync(() => {
       fixture = createComponent(ChipGridWithFormErrorMessages);
+      flush();
+      fixture.detectChanges();
+
       errorTestComponent = fixture.componentInstance;
       containerEl = fixture.debugElement.query(By.css('mat-form-field'))!.nativeElement;
       chipGridEl = fixture.debugElement.query(By.css('mat-chip-grid'))!.nativeElement;
-    });
+      inputEl = fixture.debugElement.query(By.css('input'))!.nativeElement;
+    }));
 
     it('should not show any errors if the user has not interacted', () => {
       expect(errorTestComponent.formControl.untouched)
@@ -908,6 +913,7 @@ describe('MDC-based MatChipGrid', () => {
         .toBe(0);
 
       dispatchFakeEvent(fixture.debugElement.query(By.css('form'))!.nativeElement, 'submit');
+      flush();
       fixture.detectChanges();
 
       fixture.whenStable().then(() => {
@@ -924,10 +930,12 @@ describe('MDC-based MatChipGrid', () => {
           .withContext('Expected aria-invalid to be set to "true".')
           .toBe('true');
       });
+      flush();
     }));
 
     it('should hide the errors and show the hints once the chip grid becomes valid', fakeAsync(() => {
       errorTestComponent.formControl.markAsTouched();
+      flush();
       fixture.detectChanges();
 
       fixture.whenStable().then(() => {
@@ -942,6 +950,7 @@ describe('MDC-based MatChipGrid', () => {
           .toBe(0);
 
         errorTestComponent.formControl.setValue('something');
+        flush();
         fixture.detectChanges();
 
         fixture.whenStable().then(() => {
@@ -956,6 +965,8 @@ describe('MDC-based MatChipGrid', () => {
             .withContext('Expected one hint to be shown once the input is valid.')
             .toBe(1);
         });
+
+        flush();
       });
     }));
 
@@ -966,27 +977,31 @@ describe('MDC-based MatChipGrid', () => {
       expect(containerEl.querySelector('mat-error')!.getAttribute('aria-live')).toBe('polite');
     });
 
-    it('sets the aria-describedby to reference errors when in error state', () => {
+    it('sets the aria-describedby on the input to reference errors when in error state', fakeAsync(() => {
       let hintId = fixture.debugElement
         .query(By.css('.mat-mdc-form-field-hint'))!
         .nativeElement.getAttribute('id');
-      let describedBy = chipGridEl.getAttribute('aria-describedby');
+      let describedBy = inputEl.getAttribute('aria-describedby');
 
       expect(hintId).withContext('hint should be shown').toBeTruthy();
       expect(describedBy).toBe(hintId);
 
       fixture.componentInstance.formControl.markAsTouched();
       fixture.detectChanges();
 
+      // Flush the describedby timer and detect changes caused by it.
+      flush();
+      fixture.detectChanges();
+
       let errorIds = fixture.debugElement
         .queryAll(By.css('.mat-mdc-form-field-error'))
         .map(el => el.nativeElement.getAttribute('id'))
         .join(' ');
-      describedBy = chipGridEl.getAttribute('aria-describedby');
+      let errorDescribedBy = inputEl.getAttribute('aria-describedby');
 
       expect(errorIds).withContext('errors should be shown').toBeTruthy();
-      expect(describedBy).toBe(errorIds);
-    });
+      expect(errorDescribedBy).toBe(errorIds);
+    }));
   });
 
   function createComponent<T>(

diff --git a/src/material-experimental/mdc-chips/chip-grid.ts b/src/material-experimental/mdc-chips/chip-grid.ts
@@ -108,8 +108,6 @@ const _MatChipGridMixinBase = mixinErrorState(MatChipGridBase);
     'class': 'mat-mdc-chip-set mat-mdc-chip-grid mdc-evolution-chip-set',
     '[attr.role]': 'role',
     '[tabIndex]': '_chips && _chips.length === 0 ? -1 : tabIndex',
-    // TODO: replace this binding with use of AriaDescriber
-    '[attr.aria-describedby]': '_ariaDescribedby || null',
     '[attr.aria-disabled]': 'disabled.toString()',
     '[attr.aria-invalid]': 'errorState',
     '[class.mat-mdc-chip-list-disabled]': 'disabled',
@@ -145,6 +143,11 @@ export class MatChipGrid
 
   protected override _defaultRole = 'grid';
 
+  /**
+   * List of element ids to propagate to the chipInput's aria-describedby attribute.
+   */
+  private _ariaDescribedbyIds: string[] = [];
+
   /**
    * Function when touched. Set as part of ControlValueAccessor implementation.
    * @docs-private
@@ -337,6 +340,7 @@ export class MatChipGrid
   /** Associates an HTML input element with this chip grid. */
   registerInput(inputElement: MatChipTextControl): void {
     this._chipInput = inputElement;
+    this._chipInput.setDescribedByIds(this._ariaDescribedbyIds);
   }
 
   /**
@@ -378,7 +382,18 @@ export class MatChipGrid
    * @docs-private
    */
   setDescribedByIds(ids: string[]) {
-    this._ariaDescribedby = ids.join(' ');
+    // We must keep this up to date to handle the case where ids are set
+    // before the chip input is registered.
+    this._ariaDescribedbyIds = ids;
+
+    if (this._chipInput) {
+      // Use a setTimeout in case this is being run during change detection
+      // and the chip input has already determined its host binding for
+      // aria-describedBy.
+      setTimeout(() => {
+        this._chipInput.setDescribedByIds(ids);
+      }, 0);
+    }
   }
 
   /**

diff --git a/src/material-experimental/mdc-chips/chip-input.spec.ts b/src/material-experimental/mdc-chips/chip-input.spec.ts
@@ -25,39 +25,35 @@ describe('MDC-based MatChipInput', () => {
   let chipInputDirective: MatChipInput;
   let dir = 'ltr';
 
-  beforeEach(
-    waitForAsync(() => {
-      TestBed.configureTestingModule({
-        imports: [PlatformModule, MatChipsModule, MatFormFieldModule, NoopAnimationsModule],
-        declarations: [TestChipInput],
-        providers: [
-          {
-            provide: Directionality,
-            useFactory: () => {
-              return {
-                value: dir.toLowerCase(),
-                change: new Subject(),
-              };
-            },
+  beforeEach(waitForAsync(() => {
+    TestBed.configureTestingModule({
+      imports: [PlatformModule, MatChipsModule, MatFormFieldModule, NoopAnimationsModule],
+      declarations: [TestChipInput],
+      providers: [
+        {
+          provide: Directionality,
+          useFactory: () => {
+            return {
+              value: dir.toLowerCase(),
+              change: new Subject(),
+            };
           },
-        ],
-      });
+        },
+      ],
+    });
 
-      TestBed.compileComponents();
-    }),
-  );
+    TestBed.compileComponents();
+  }));
 
-  beforeEach(
-    waitForAsync(() => {
-      fixture = TestBed.createComponent(TestChipInput);
-      testChipInput = fixture.debugElement.componentInstance;
-      fixture.detectChanges();
+  beforeEach(waitForAsync(() => {
+    fixture = TestBed.createComponent(TestChipInput);
+    testChipInput = fixture.debugElement.componentInstance;
+    fixture.detectChanges();
 
-      inputDebugElement = fixture.debugElement.query(By.directive(MatChipInput))!;
-      chipInputDirective = inputDebugElement.injector.get<MatChipInput>(MatChipInput);
-      inputNativeElement = inputDebugElement.nativeElement;
-    }),
-  );
+    inputDebugElement = fixture.debugElement.query(By.directive(MatChipInput))!;
+    chipInputDirective = inputDebugElement.injector.get<MatChipInput>(MatChipInput);
+    inputNativeElement = inputDebugElement.nativeElement;
+  }));
 
   describe('basic behavior', () => {
     it('emits the (chipEnd) on enter keyup', () => {
@@ -230,6 +226,26 @@ describe('MDC-based MatChipInput', () => {
       dispatchKeyboardEvent(inputNativeElement, 'keydown', ENTER, undefined, {shift: true});
       expect(testChipInput.add).not.toHaveBeenCalled();
     });
+
+    it('should set aria-describedby correctly when a non-empty list of ids is passed to setDescribedByIds', fakeAsync(() => {
+      const ids = ['a', 'b', 'c'];
+
+      testChipInput.chipGridInstance.setDescribedByIds(ids);
+      flush();
+      fixture.detectChanges();
+
+      expect(inputNativeElement.getAttribute('aria-describedby')).toEqual('a b c');
+    }));
+
+    it('should set aria-describedby correctly when an empty list of ids is passed to setDescribedByIds', fakeAsync(() => {
+      const ids: string[] = [];
+
+      testChipInput.chipGridInstance.setDescribedByIds(ids);
+      flush();
+      fixture.detectChanges();
+
+      expect(inputNativeElement.getAttribute('aria-describedby')).toBeNull();
+    }));
   });
 });
 

diff --git a/src/material-experimental/mdc-chips/chip-input.ts b/src/material-experimental/mdc-chips/chip-input.ts
@@ -65,6 +65,7 @@ let nextUniqueId = 0;
     '[attr.disabled]': 'disabled || null',
     '[attr.placeholder]': 'placeholder || null',
     '[attr.aria-invalid]': '_chipGrid && _chipGrid.ngControl ? _chipGrid.ngControl.invalid : null',
+    '[attr.aria-describedby]': '_ariaDescribedby || null',
     '[attr.aria-required]': '_chipGrid && _chipGrid.required || null',
     '[attr.required]': '_chipGrid && _chipGrid.required || null',
   },
@@ -73,6 +74,9 @@ export class MatChipInput implements MatChipTextControl, AfterContentInit, OnCha
   /** Used to prevent focus moving to chips while user is holding backspace */
   private _focusLastChipOnBackspace: boolean;
 
+  /** Value for ariaDescribedby property */
+  _ariaDescribedby?: string;
+
   /** Whether the control is focused. */
   focused: boolean = false;
   _chipGrid: MatChipGrid;
@@ -240,6 +244,10 @@ export class MatChipInput implements MatChipTextControl, AfterContentInit, OnCha
     this._focusLastChipOnBackspace = true;
   }
 
+  setDescribedByIds(ids: string[]): void {
+    this._ariaDescribedby = ids.join(' ');
+  }
+
   /** Checks whether a keycode is one of the configured separators. */
   private _isSeparatorKey(event: KeyboardEvent) {
     return !hasModifierKey(event) && new Set(this.separatorKeyCodes).has(event.keyCode);

diff --git a/src/material-experimental/mdc-chips/chip-set.ts b/src/material-experimental/mdc-chips/chip-set.ts
@@ -65,8 +65,6 @@ const _MatChipSetMixinBase = mixinTabIndex(MatChipSetBase);
   host: {
     'class': 'mat-mdc-chip-set mdc-evolution-chip-set',
     '[attr.role]': 'role',
-    // TODO: replace this binding with use of AriaDescriber
-    '[attr.aria-describedby]': '_ariaDescribedby || null',
   },
   encapsulation: ViewEncapsulation.None,
   changeDetection: ChangeDetectionStrategy.OnPush,
@@ -138,9 +136,6 @@ export class MatChipSet
     },
   };
 
-  /** The aria-describedby attribute on the chip list for improved a11y. */
-  _ariaDescribedby: string;
-
   /**
    * Map from class to whether the class is enabled.
    * Enabled classes are set on the MDC chip-set div.

diff --git a/src/material-experimental/mdc-chips/chip-text-control.ts b/src/material-experimental/mdc-chips/chip-text-control.ts
@@ -22,4 +22,7 @@ export interface MatChipTextControl {
 
   /** Focuses the text control. */
   focus(): void;
+
+  /** Sets the list of ids the input is described by. */
+  setDescribedByIds(ids: string[]): void;
 }