fix(chips): don't set aria-required when element doesn't have… (#17425)

Currently we always set `aria-required` `mat-chip-list`/`mat-chip-grid`, however it should only be set on form controls. If a chip list is empty we remove its role so we also have to clear the `aria-required`. Fixes #17397.
angular · Oct 18, 2019 · 939c18d · 939c18d
1 parent db879d5
commit 939c18d
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 3 deletions.
diff --git a/src/material-experimental/mdc-chips/chip-grid.spec.ts b/src/material-experimental/mdc-chips/chip-grid.spec.ts
@@ -96,6 +96,22 @@ describe('MatChipGrid', () => {
 
         expect(chips.toArray().every(chip => chip.disabled)).toBe(true);
       }));
+
+      it('should not set a role on the grid when the list is empty', () => {
+        testComponent.chips = [];
+        fixture.detectChanges();
+
+        expect(chipGridNativeElement.hasAttribute('role')).toBe(false);
+      });
+
+      it('should not set aria-required when it does not have a role', () => {
+        testComponent.chips = [];
+        fixture.detectChanges();
+
+        expect(chipGridNativeElement.hasAttribute('role')).toBe(false);
+        expect(chipGridNativeElement.hasAttribute('aria-required')).toBe(false);
+      });
+
     });
 
     describe('focus behaviors', () => {

diff --git a/src/material-experimental/mdc-chips/chip-grid.ts b/src/material-experimental/mdc-chips/chip-grid.ts
@@ -88,7 +88,7 @@ const _MatChipGridMixinBase: CanUpdateErrorStateCtor & typeof MatChipGridBase =
     '[tabIndex]': '_chips && _chips.length === 0 ? -1 : tabIndex',
     // TODO: replace this binding with use of AriaDescriber
     '[attr.aria-describedby]': '_ariaDescribedby || null',
-    '[attr.aria-required]': 'required.toString()',
+    '[attr.aria-required]': 'role ? required : null',
     '[attr.aria-disabled]': 'disabled.toString()',
     '[attr.aria-invalid]': 'errorState',
     '[class.mat-mdc-chip-list-disabled]': 'disabled',

diff --git a/src/material-experimental/mdc-chips/chip-listbox.spec.ts b/src/material-experimental/mdc-chips/chip-listbox.spec.ts
@@ -93,6 +93,22 @@ describe('MatChipListbox', () => {
 
         expect(chips.toArray().every(chip => chip.disabled)).toBe(true);
       }));
+
+      it('should not set a role on the grid when the list is empty', () => {
+        testComponent.chips = [];
+        fixture.detectChanges();
+
+        expect(chipListboxNativeElement.hasAttribute('role')).toBe(false);
+      });
+
+      it('should not set aria-required when it does not have a role', () => {
+        testComponent.chips = [];
+        fixture.detectChanges();
+
+        expect(chipListboxNativeElement.hasAttribute('role')).toBe(false);
+        expect(chipListboxNativeElement.hasAttribute('aria-required')).toBe(false);
+      });
+
     });
 
     describe('with selected chips', () => {

diff --git a/src/material-experimental/mdc-chips/chip-listbox.ts b/src/material-experimental/mdc-chips/chip-listbox.ts
@@ -70,7 +70,7 @@ export const MAT_CHIP_LISTBOX_CONTROL_VALUE_ACCESSOR: any = {
     '[tabIndex]': 'empty ? -1 : tabIndex',
     // TODO: replace this binding with use of AriaDescriber
     '[attr.aria-describedby]': '_ariaDescribedby || null',
-    '[attr.aria-required]': 'required.toString()',
+    '[attr.aria-required]': 'role ? required : null',
     '[attr.aria-disabled]': 'disabled.toString()',
     '[attr.aria-multiselectable]': 'multiple',
     '[attr.aria-orientation]': 'ariaOrientation',

diff --git a/src/material/chips/chip-list.spec.ts b/src/material/chips/chip-list.spec.ts
@@ -132,6 +132,14 @@ describe('MatChipList', () => {
 
         expect(chipListNativeElement.getAttribute('role')).toBeNull('Expect no role attribute');
       });
+
+      it('should not have aria-required when it has no role', () => {
+        fixture.componentInstance.foods = [];
+        fixture.detectChanges();
+
+        expect(chipListNativeElement.hasAttribute('role')).toBe(false);
+        expect(chipListNativeElement.hasAttribute('aria-required')).toBe(false);
+      });
     });
 
     describe('focus behaviors', () => {

diff --git a/src/material/chips/chip-list.ts b/src/material/chips/chip-list.ts
@@ -80,7 +80,7 @@ export class MatChipListChange {
   host: {
     '[attr.tabindex]': 'disabled ? null : _tabIndex',
     '[attr.aria-describedby]': '_ariaDescribedby || null',
-    '[attr.aria-required]': 'required.toString()',
+    '[attr.aria-required]': 'role ? required : null',
     '[attr.aria-disabled]': 'disabled.toString()',
     '[attr.aria-invalid]': 'errorState',
     '[attr.aria-multiselectable]': 'multiple',