fix(material/badge): correctly apply badge description

jelbourn · jelbourn · commit 10c26e611443 · 2021-09-09T19:51:02.000-07:00
Previously, `MatBadge` was applying the provided description as an `aria-label` to the badge content element, which is a child element of the badge host. Then it was _also_ applying an `aria-describedby` to that same content element. The end result is that the badge was being treated mainly as text content for a control, which isn't really the best way to convey its complmentary nature. This behavior was causing problems in NVDA, where the badge content was overriding the host button's label.

This change makes the badge content itself `aria-hidden` and applies the badge description to the badge's _host_ element via `aria-describedby` and completely removes any setting of `aria-label`. It also makes a handful of minor cleanup refactorings.
diff --git a/src/dev-app/badge/badge-demo.html b/src/dev-app/badge/badge-demo.html
@@ -1,71 +1,24 @@
 <div>
 
-  <div class="demo-badge">
-    <h3>Text</h3>
-    <span [matBadge]="badgeContent" matBadgeOverlap="false" *ngIf="visible">
-      Hello
-    </span>
-
-    <span matBadge="11111" matBadgeOverlap="false">
-      Hello
-    </span>
-
-    <span matBadge="22" matBadgeOverlap="false" matBadgePosition="below after" matBadgeColor="accent">
-      Hello
-    </span>
-
-    <span matBadge="22" matBadgeOverlap="false" matBadgePosition="above before" matBadgeColor="warn">
-      Hello
-    </span>
-
-    <span matBadge="⚡️" matBadgeOverlap="false" matBadgePosition="below before">
-      Hello
-    </span>
-
-    <span [matBadge]="badgeContent" matBadgeDescription="I've got {{badgeContent}} problems">
-      Aria
-    </span>
-
-    <span [matBadge]="badgeContent" matBadgeHidden="true">
-      Hidden
-    </span>
-
-    <input type="text" [(ngModel)]="badgeContent" />
-    <button (click)="visible = !visible">Toggle</button>
-  </div>
-
   <div class="demo-badge">
     <h3>Buttons</h3>
-    <button mat-raised-button [matBadge]="badgeContent">
-      <mat-icon color="primary">home</mat-icon>
-    </button>
-
-    <button mat-raised-button matBadge="22" matBadgePosition="below after" color="primary" matBadgeColor="accent">
-      <mat-icon color="accent">home</mat-icon>
-    </button>
-
-    <button mat-raised-button matBadge="22" matBadgePosition="above before">
-      <mat-icon color="primary">home</mat-icon>
+    <button mat-stroked-button matBadge="7" matBadgeDescription="7 unread messages">
+      Inbox
     </button>
 
-    <button mat-raised-button [matBadge]="badgeContent" matBadgeDisabled>
-      <mat-icon color="primary">home</mat-icon>
+    <button mat-stroked-button matBadge="7" matBadgePosition="below after"
+        matBadgeDescription="7 unread messages">
+      Inbox
     </button>
 
-    <button mat-stroked-button [matBadge]="badgeContent">
-      <mat-icon color="primary">home</mat-icon>
+    <button mat-stroked-button matBadge="7" disabled matBadgeDisabled
+        matBadgeDescription="7 unread messages">
+      Inbox
     </button>
 
-    <button disabled mat-raised-button [matBadge]="badgeContent" matBadgeDisabled>
-      <mat-icon color="primary">home</mat-icon>
-    </button>
-
-    <button mat-raised-button matBadge="22" matBadgePosition="below before">
-      <mat-icon color="primary">home</mat-icon>
-    </button>
-
-    <button mat-raised-button>
-      <mat-icon matBadge="22" color="accent">home</mat-icon>
+    <button mat-stroked-button matBadge="7" matBadgeColor="accent"
+        matBadgeDescription="7 unread messages">
+      Inbox
     </button>
   </div>
 
@@ -99,4 +52,38 @@ <h3>Size</h3>
 
   </div>
 
+  <div class="demo-badge">
+    <h3>Text</h3>
+    <span [matBadge]="badgeContent" matBadgeOverlap="false" *ngIf="visible">
+      Hello
+    </span>
+
+    <span matBadge="11111" matBadgeOverlap="false">
+      Hello
+    </span>
+
+    <span matBadge="22" matBadgeOverlap="false" matBadgePosition="below after" matBadgeColor="accent">
+      Hello
+    </span>
+
+    <span matBadge="22" matBadgeOverlap="false" matBadgePosition="above before" matBadgeColor="warn">
+      Hello
+    </span>
+
+    <span matBadge="⚡️" matBadgeOverlap="false" matBadgePosition="below before">
+      Hello
+    </span>
+
+    <span [matBadge]="badgeContent" matBadgeDescription="I've got {{badgeContent}} problems">
+      Aria
+    </span>
+
+    <span [matBadge]="badgeContent" matBadgeHidden="true">
+      Hidden
+    </span>
+
+    <input type="text" [(ngModel)]="badgeContent" />
+    <button (click)="visible = !visible">Toggle</button>
+  </div>
+
 </div>
diff --git a/src/material/badge/badge.spec.ts b/src/material/badge/badge.spec.ts
@@ -7,8 +7,8 @@ import {ThemePalette} from '@angular/material/core';
 describe('MatBadge', () => {
   let fixture: ComponentFixture<any>;
   let testComponent: BadgeTestApp;
-  let badgeNativeElement: HTMLElement;
-  let badgeDebugElement: DebugElement;
+  let badgeHostNativeElement: HTMLElement;
+  let badgeHostDebugElement: DebugElement;
 
   beforeEach(fakeAsync(() => {
     TestBed
@@ -22,12 +22,12 @@ describe('MatBadge', () => {
     testComponent = fixture.debugElement.componentInstance;
     fixture.detectChanges();
 
-    badgeDebugElement = fixture.debugElement.query(By.directive(MatBadge))!;
-    badgeNativeElement = badgeDebugElement.nativeElement;
+    badgeHostDebugElement = fixture.debugElement.query(By.directive(MatBadge))!;
+    badgeHostNativeElement = badgeHostDebugElement.nativeElement;
   }));
 
   it('should update the badge based on attribute', () => {
-    const badgeElement = badgeNativeElement.querySelector('.mat-badge-content')!;
+    const badgeElement = badgeHostNativeElement.querySelector('.mat-badge-content')!;
     expect(badgeElement.textContent).toContain('1');
 
     testComponent.badgeContent = '22';
@@ -36,7 +36,7 @@ describe('MatBadge', () => {
   });
 
   it('should be able to pass in falsy values to the badge content', () => {
-    const badgeElement = badgeNativeElement.querySelector('.mat-badge-content')!;
+    const badgeElement = badgeHostNativeElement.querySelector('.mat-badge-content')!;
     expect(badgeElement.textContent).toContain('1');
 
     testComponent.badgeContent = 0;
@@ -45,7 +45,7 @@ describe('MatBadge', () => {
   });
 
   it('should treat null and undefined as empty strings in the badge content', () => {
-    const badgeElement = badgeNativeElement.querySelector('.mat-badge-content')!;
+    const badgeElement = badgeHostNativeElement.querySelector('.mat-badge-content')!;
     expect(badgeElement.textContent).toContain('1');
 
     testComponent.badgeContent = null;
@@ -60,83 +60,83 @@ describe('MatBadge', () => {
   it('should apply class based on color attribute', () => {
     testComponent.badgeColor = 'primary';
     fixture.detectChanges();
-    expect(badgeNativeElement.classList.contains('mat-badge-primary')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-primary')).toBe(true);
 
     testComponent.badgeColor = 'accent';
     fixture.detectChanges();
-    expect(badgeNativeElement.classList.contains('mat-badge-accent')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-accent')).toBe(true);
 
     testComponent.badgeColor = 'warn';
     fixture.detectChanges();
-    expect(badgeNativeElement.classList.contains('mat-badge-warn')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-warn')).toBe(true);
 
     testComponent.badgeColor = undefined;
     fixture.detectChanges();
 
-    expect(badgeNativeElement.classList).not.toContain('mat-badge-accent');
+    expect(badgeHostNativeElement.classList).not.toContain('mat-badge-accent');
   });
 
   it('should update the badge position on direction change', () => {
-    expect(badgeNativeElement.classList.contains('mat-badge-above')).toBe(true);
-    expect(badgeNativeElement.classList.contains('mat-badge-after')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-above')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-after')).toBe(true);
 
     testComponent.badgeDirection = 'below before';
     fixture.detectChanges();
 
-    expect(badgeNativeElement.classList.contains('mat-badge-below')).toBe(true);
-    expect(badgeNativeElement.classList.contains('mat-badge-before')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-below')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-before')).toBe(true);
   });
 
   it('should change visibility to hidden', () => {
-    expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(false);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-hidden')).toBe(false);
 
     testComponent.badgeHidden = true;
     fixture.detectChanges();
 
-    expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-hidden')).toBe(true);
   });
 
   it('should change badge sizes', () => {
-    expect(badgeNativeElement.classList.contains('mat-badge-medium')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-medium')).toBe(true);
 
     testComponent.badgeSize = 'small';
     fixture.detectChanges();
 
-    expect(badgeNativeElement.classList.contains('mat-badge-small')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-small')).toBe(true);
 
     testComponent.badgeSize = 'large';
     fixture.detectChanges();
 
-    expect(badgeNativeElement.classList.contains('mat-badge-large')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-large')).toBe(true);
   });
 
   it('should change badge overlap', () => {
-    expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(false);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-overlap')).toBe(false);
 
     testComponent.badgeOverlap = true;
     fixture.detectChanges();
 
-    expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(true);
+    expect(badgeHostNativeElement.classList.contains('mat-badge-overlap')).toBe(true);
   });
 
   it('should toggle `aria-describedby` depending on whether the badge has a description', () => {
-    const badgeContent = badgeNativeElement.querySelector('.mat-badge-content')!;
-
-    expect(badgeContent.getAttribute('aria-describedby')).toBeFalsy();
+    expect(badgeHostNativeElement.hasAttribute('aria-describedby')).toBeFalse();
 
     testComponent.badgeDescription = 'Describing a badge';
     fixture.detectChanges();
 
-    expect(badgeContent.getAttribute('aria-describedby')).toBeTruthy();
+    const describedById = badgeHostNativeElement.getAttribute('aria-describedby') || '';
+    const description = document.getElementById(describedById)?.textContent;
+    expect(description).toBe('Describing a badge');
 
     testComponent.badgeDescription = '';
     fixture.detectChanges();
 
-    expect(badgeContent.getAttribute('aria-describedby')).toBeFalsy();
+    expect(badgeHostNativeElement.hasAttribute('aria-describedby')).toBeFalse();
   });
 
   it('should toggle visibility based on whether the badge has content', () => {
-    const classList = badgeNativeElement.classList;
+    const classList = badgeHostNativeElement.classList;
 
     expect(classList.contains('mat-badge-hidden')).toBe(false);
 
@@ -162,7 +162,7 @@ describe('MatBadge', () => {
   });
 
   it('should apply view encapsulation on create badge content', () => {
-    const badge = badgeNativeElement.querySelector('.mat-badge-content')!;
+    const badge = badgeHostNativeElement.querySelector('.mat-badge-content')!;
     let encapsulationAttr: Attr | undefined;
 
     for (let i = 0; i < badge.attributes.length; i++) {
@@ -176,7 +176,7 @@ describe('MatBadge', () => {
   });
 
   it('should toggle a class depending on the badge disabled state', () => {
-    const element: HTMLElement = badgeDebugElement.nativeElement;
+    const element: HTMLElement = badgeHostDebugElement.nativeElement;
 
     expect(element.classList).not.toContain('mat-badge-disabled');
 
@@ -186,25 +186,6 @@ describe('MatBadge', () => {
     expect(element.classList).toContain('mat-badge-disabled');
   });
 
-  it('should update the aria-label if the description changes', () => {
-    const badgeContent = badgeNativeElement.querySelector('.mat-badge-content')!;
-
-    fixture.componentInstance.badgeDescription = 'initial content';
-    fixture.detectChanges();
-
-    expect(badgeContent.getAttribute('aria-label')).toBe('initial content');
-
-    fixture.componentInstance.badgeDescription = 'changed content';
-    fixture.detectChanges();
-
-    expect(badgeContent.getAttribute('aria-label')).toBe('changed content');
-
-    fixture.componentInstance.badgeDescription = '';
-    fixture.detectChanges();
-
-    expect(badgeContent.hasAttribute('aria-label')).toBe(false);
-  });
-
   it('should clear any pre-existing badges', () => {
     const preExistingFixture = TestBed.createComponent(PreExistingBadge);
     preExistingFixture.detectChanges();
@@ -220,7 +201,7 @@ describe('MatBadge', () => {
   });
 
   it('should expose the badge element', () => {
-    const badgeElement = badgeNativeElement.querySelector('.mat-badge-content')!;
+    const badgeElement = badgeHostNativeElement.querySelector('.mat-badge-content')!;
     expect(fixture.componentInstance.badgeInstance.getBadgeElement()).toBe(badgeElement);
   });
 
@@ -288,9 +269,7 @@ class NestedBadge {
 
 
 @Component({
-  template: `
-    <ng-template matBadge="1">Notifications</ng-template>
-  `
+  template: `<ng-template matBadge="1">Notifications</ng-template>`,
 })
 class BadgeOnTemplate {
 }
diff --git a/src/material/badge/badge.ts b/src/material/badge/badge.ts
