Skip to content

Commit aed7e8a

Browse files
crisbetojelbourn
authored andcommitted
fix(badge): incorrectly setting aria-describedby (#9957)
* With the current setup, the badge component will set an `aria-describedby` message that doesn't correspond to the actual description, because it does the describing before its description has been updated. These changes make it so it uses the actual description. * Fixes the aria description not being removed once the badge is destroyed. * Cleans up the badge testing setup.
1 parent f30609c commit aed7e8a

File tree

2 files changed

+99
-78
lines changed

2 files changed

+99
-78
lines changed

src/lib/badge/badge.spec.ts

Lines changed: 78 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,132 +1,141 @@
1-
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
1+
import {ComponentFixture, TestBed, fakeAsync} from '@angular/core/testing';
22
import {Component, DebugElement} from '@angular/core';
33
import {By} from '@angular/platform-browser';
44
import {MatBadge, MatBadgeModule} from './index';
55
import {ThemePalette} from '@angular/material/core';
66

77
describe('MatBadge', () => {
88
let fixture: ComponentFixture<any>;
9-
let testComponent: BadgeWithTextContent;
10-
let badgeNativeElement;
9+
let testComponent: BadgeTestApp;
10+
let badgeNativeElement: HTMLElement;
11+
let badgeDebugElement: DebugElement;
1112

12-
beforeEach(async(() => {
13+
beforeEach(fakeAsync(() => {
1314
TestBed.configureTestingModule({
1415
imports: [MatBadgeModule],
15-
declarations: [BadgeWithTextContent],
16+
declarations: [BadgeTestApp],
1617
}).compileComponents();
17-
}));
1818

19-
beforeEach(() => {
20-
fixture = TestBed.createComponent(BadgeWithTextContent);
19+
fixture = TestBed.createComponent(BadgeTestApp);
2120
testComponent = fixture.debugElement.componentInstance;
2221
fixture.detectChanges();
22+
23+
badgeDebugElement = fixture.debugElement.query(By.directive(MatBadge));
24+
badgeNativeElement = badgeDebugElement.nativeElement;
25+
}));
26+
27+
it('should update the badge based on attribute', () => {
28+
let badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content')!;
29+
30+
expect(badgeContentDebugElement.textContent).toContain('1');
31+
32+
testComponent.badgeContent = '22';
33+
fixture.detectChanges();
34+
35+
badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content')!;
36+
expect(badgeContentDebugElement.textContent).toContain('22');
2337
});
2438

25-
describe('MatBadge Text', () => {
26-
let badgeDebugElement: DebugElement;
39+
it('should apply class based on color attribute', () => {
40+
testComponent.badgeColor = 'primary';
41+
fixture.detectChanges();
42+
expect(badgeNativeElement.classList.contains('mat-badge-primary')).toBe(true);
2743

28-
beforeEach(() => {
29-
badgeDebugElement = fixture.debugElement.query(By.directive(MatBadge));
30-
badgeNativeElement = badgeDebugElement.nativeElement;
31-
fixture.detectChanges();
32-
});
44+
testComponent.badgeColor = 'accent';
45+
fixture.detectChanges();
46+
expect(badgeNativeElement.classList.contains('mat-badge-accent')).toBe(true);
3347

34-
it('should update the badge based on attribute', () => {
35-
let badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content');
48+
testComponent.badgeColor = 'warn';
49+
fixture.detectChanges();
50+
expect(badgeNativeElement.classList.contains('mat-badge-warn')).toBe(true);
3651

37-
expect(badgeContentDebugElement.textContent).toContain('1');
52+
testComponent.badgeColor = undefined;
53+
fixture.detectChanges();
3854

39-
testComponent.badgeContent = '22';
40-
fixture.detectChanges();
55+
expect(badgeNativeElement.classList).not.toContain('mat-badge-accent');
56+
});
4157

42-
badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content');
43-
expect(badgeContentDebugElement.textContent).toContain('22');
44-
});
58+
it('should update the badge position on direction change', () => {
59+
expect(badgeNativeElement.classList.contains('mat-badge-above')).toBe(true);
60+
expect(badgeNativeElement.classList.contains('mat-badge-after')).toBe(true);
4561

46-
it('should apply class based on color attribute', () => {
47-
testComponent.badgeColor = 'primary';
48-
fixture.detectChanges();
49-
expect(badgeNativeElement.classList.contains('mat-badge-primary')).toBe(true);
62+
testComponent.badgeDirection = 'below before';
63+
fixture.detectChanges();
5064

51-
testComponent.badgeColor = 'accent';
52-
fixture.detectChanges();
53-
expect(badgeNativeElement.classList.contains('mat-badge-accent')).toBe(true);
65+
expect(badgeNativeElement.classList.contains('mat-badge-below')).toBe(true);
66+
expect(badgeNativeElement.classList.contains('mat-badge-before')).toBe(true);
67+
});
5468

55-
testComponent.badgeColor = 'warn';
56-
fixture.detectChanges();
57-
expect(badgeNativeElement.classList.contains('mat-badge-warn')).toBe(true);
69+
it('should change visibility to hidden', () => {
70+
expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(false);
5871

59-
testComponent.badgeColor = undefined;
60-
fixture.detectChanges();
72+
testComponent.badgeHidden = true;
73+
fixture.detectChanges();
6174

62-
expect(badgeNativeElement.classList).not.toContain('mat-badge-accent');
63-
});
75+
expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(true);
76+
});
6477

65-
it('should update the badge position on direction change', () => {
66-
expect(badgeNativeElement.classList.contains('mat-badge-above')).toBe(true);
67-
expect(badgeNativeElement.classList.contains('mat-badge-after')).toBe(true);
78+
it('should change badge sizes', () => {
79+
expect(badgeNativeElement.classList.contains('mat-badge-medium')).toBe(true);
6880

69-
testComponent.badgeDirection = 'below before';
70-
fixture.detectChanges();
81+
testComponent.badgeSize = 'small';
82+
fixture.detectChanges();
7183

72-
expect(badgeNativeElement.classList.contains('mat-badge-below')).toBe(true);
73-
expect(badgeNativeElement.classList.contains('mat-badge-before')).toBe(true);
74-
});
84+
expect(badgeNativeElement.classList.contains('mat-badge-small')).toBe(true);
7585

76-
it('should change visibility to hidden', () => {
77-
expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(false);
86+
testComponent.badgeSize = 'large';
87+
fixture.detectChanges();
7888

79-
testComponent.badgeHidden = true;
80-
fixture.detectChanges();
89+
expect(badgeNativeElement.classList.contains('mat-badge-large')).toBe(true);
90+
});
8191

82-
expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(true);
83-
});
92+
it('should change badge overlap', () => {
93+
expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(false);
8494

85-
it('should change badge sizes', () => {
86-
expect(badgeNativeElement.classList.contains('mat-badge-medium')).toBe(true);
95+
testComponent.badgeOverlap = true;
96+
fixture.detectChanges();
8797

88-
testComponent.badgeSize = 'small';
89-
fixture.detectChanges();
98+
expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(true);
99+
});
90100

91-
expect(badgeNativeElement.classList.contains('mat-badge-small')).toBe(true);
101+
it('should toggle `aria-describedby` depending on whether the badge has a description', () => {
102+
const badgeContent = badgeNativeElement.querySelector('.mat-badge-content')!;
92103

93-
testComponent.badgeSize = 'large';
94-
fixture.detectChanges();
104+
expect(badgeContent.getAttribute('aria-describedby')).toBeFalsy();
95105

96-
expect(badgeNativeElement.classList.contains('mat-badge-large')).toBe(true);
97-
});
106+
testComponent.badgeDescription = 'Describing a badge';
107+
fixture.detectChanges();
98108

99-
it('should change badge overlap', () => {
100-
expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(false);
109+
expect(badgeContent.getAttribute('aria-describedby')).toBeTruthy();
101110

102-
testComponent.badgeOverlap = true;
103-
fixture.detectChanges();
111+
testComponent.badgeDescription = '';
112+
fixture.detectChanges();
104113

105-
expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(true);
106-
});
114+
expect(badgeContent.getAttribute('aria-describedby')).toBeFalsy();
107115
});
108116

109117
});
110118

111119
/** Test component that contains a MatBadge. */
112120
@Component({
113-
selector: 'test-app',
114121
template: `
115122
<span [matBadge]="badgeContent"
116123
[matBadgeColor]="badgeColor"
117124
[matBadgePosition]="badgeDirection"
118125
[matBadgeHidden]="badgeHidden"
119126
[matBadgeSize]="badgeSize"
120-
[matBadgeOverlap]="badgeOverlap">
127+
[matBadgeOverlap]="badgeOverlap"
128+
[matBadgeDescription]="badgeDescription">
121129
home
122130
</span>
123131
`
124132
})
125-
class BadgeWithTextContent {
133+
class BadgeTestApp {
126134
badgeColor: ThemePalette;
127135
badgeContent = '1';
128136
badgeDirection = 'above after';
129137
badgeHidden = false;
130138
badgeSize = 'medium';
131139
badgeOverlap = false;
140+
badgeDescription: string;
132141
}

src/lib/badge/badge.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Directive, Input, ElementRef, Inject, Optional, NgZone} from '@angular/core';
9+
import {Directive, Input, ElementRef, Inject, Optional, NgZone, OnDestroy} from '@angular/core';
1010
import {coerceBooleanProperty} from '@angular/cdk/coercion';
1111
import {ThemePalette} from '@angular/material/core';
1212
import {AriaDescriber} from '@angular/cdk/a11y';
@@ -33,7 +33,7 @@ export type MatBadgeSize = 'small' | 'medium' | 'large';
3333
'[class.mat-badge-hidden]': 'hidden',
3434
},
3535
})
36-
export class MatBadge {
36+
export class MatBadge implements OnDestroy {
3737

3838
/** The color of the badge. Can be `primary`, `accent`, or `warn`. */
3939
@Input('matBadgeColor')
@@ -70,11 +70,11 @@ export class MatBadge {
7070
/** Message used to describe the decorated element via aria-describedby */
7171
@Input('matBadgeDescription')
7272
get description(): string { return this._description; }
73-
set description(val: string) {
74-
if (this._description) {
75-
this._updateHostAriaDescription(val, this._description);
73+
set description(newDescription: string) {
74+
if (newDescription !== this._description) {
75+
this._updateHostAriaDescription(newDescription, this._description);
76+
this._description = newDescription;
7677
}
77-
this._description = val;
7878
}
7979
private _description: string;
8080

@@ -110,6 +110,12 @@ export class MatBadge {
110110
return this.position.indexOf('before') === -1;
111111
}
112112

113+
ngOnDestroy() {
114+
if (this.description && this._badgeElement) {
115+
this._ariaDescriber.removeDescription(this._badgeElement, this.description);
116+
}
117+
}
118+
113119
/** Injects a span element into the DOM with the content. */
114120
private _updateTextContent(): HTMLSpanElement {
115121
if (!this._badgeElement) {
@@ -150,11 +156,17 @@ export class MatBadge {
150156
}
151157

152158
/** Sets the aria-label property on the element */
153-
private _updateHostAriaDescription(val: string, prevVal: string): void {
159+
private _updateHostAriaDescription(newDescription: string, oldDescription: string): void {
154160
// ensure content available before setting label
155161
const content = this._updateTextContent();
156-
this._ariaDescriber.removeDescription(content, prevVal);
157-
this._ariaDescriber.describe(content, val);
162+
163+
if (oldDescription) {
164+
this._ariaDescriber.removeDescription(content, oldDescription);
165+
}
166+
167+
if (newDescription) {
168+
this._ariaDescriber.describe(content, newDescription);
169+
}
158170
}
159171

160172
/** Adds css theme class given the color to the component host */

0 commit comments

Comments
 (0)