Skip to content

Commit

Permalink
fix(badge): incorrectly setting aria-describedby (#9957)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
crisbeto authored and jelbourn committed Feb 18, 2018
1 parent f30609c commit aed7e8a
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 78 deletions.
147 changes: 78 additions & 69 deletions src/lib/badge/badge.spec.ts
Original file line number Diff line number Diff line change
@@ -1,132 +1,141 @@
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
import {ComponentFixture, TestBed, fakeAsync} from '@angular/core/testing';
import {Component, DebugElement} from '@angular/core';
import {By} from '@angular/platform-browser';
import {MatBadge, MatBadgeModule} from './index';
import {ThemePalette} from '@angular/material/core';

describe('MatBadge', () => {
let fixture: ComponentFixture<any>;
let testComponent: BadgeWithTextContent;
let badgeNativeElement;
let testComponent: BadgeTestApp;
let badgeNativeElement: HTMLElement;
let badgeDebugElement: DebugElement;

beforeEach(async(() => {
beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
imports: [MatBadgeModule],
declarations: [BadgeWithTextContent],
declarations: [BadgeTestApp],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(BadgeWithTextContent);
fixture = TestBed.createComponent(BadgeTestApp);
testComponent = fixture.debugElement.componentInstance;
fixture.detectChanges();

badgeDebugElement = fixture.debugElement.query(By.directive(MatBadge));
badgeNativeElement = badgeDebugElement.nativeElement;
}));

it('should update the badge based on attribute', () => {
let badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content')!;

expect(badgeContentDebugElement.textContent).toContain('1');

testComponent.badgeContent = '22';
fixture.detectChanges();

badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content')!;
expect(badgeContentDebugElement.textContent).toContain('22');
});

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

beforeEach(() => {
badgeDebugElement = fixture.debugElement.query(By.directive(MatBadge));
badgeNativeElement = badgeDebugElement.nativeElement;
fixture.detectChanges();
});
testComponent.badgeColor = 'accent';
fixture.detectChanges();
expect(badgeNativeElement.classList.contains('mat-badge-accent')).toBe(true);

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

expect(badgeContentDebugElement.textContent).toContain('1');
testComponent.badgeColor = undefined;
fixture.detectChanges();

testComponent.badgeContent = '22';
fixture.detectChanges();
expect(badgeNativeElement.classList).not.toContain('mat-badge-accent');
});

badgeContentDebugElement = badgeNativeElement.querySelector('.mat-badge-content');
expect(badgeContentDebugElement.textContent).toContain('22');
});
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);

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

testComponent.badgeColor = 'accent';
fixture.detectChanges();
expect(badgeNativeElement.classList.contains('mat-badge-accent')).toBe(true);
expect(badgeNativeElement.classList.contains('mat-badge-below')).toBe(true);
expect(badgeNativeElement.classList.contains('mat-badge-before')).toBe(true);
});

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

testComponent.badgeColor = undefined;
fixture.detectChanges();
testComponent.badgeHidden = true;
fixture.detectChanges();

expect(badgeNativeElement.classList).not.toContain('mat-badge-accent');
});
expect(badgeNativeElement.classList.contains('mat-badge-hidden')).toBe(true);
});

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);
it('should change badge sizes', () => {
expect(badgeNativeElement.classList.contains('mat-badge-medium')).toBe(true);

testComponent.badgeDirection = 'below before';
fixture.detectChanges();
testComponent.badgeSize = 'small';
fixture.detectChanges();

expect(badgeNativeElement.classList.contains('mat-badge-below')).toBe(true);
expect(badgeNativeElement.classList.contains('mat-badge-before')).toBe(true);
});
expect(badgeNativeElement.classList.contains('mat-badge-small')).toBe(true);

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

testComponent.badgeHidden = true;
fixture.detectChanges();
expect(badgeNativeElement.classList.contains('mat-badge-large')).toBe(true);
});

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

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

testComponent.badgeSize = 'small';
fixture.detectChanges();
expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(true);
});

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

testComponent.badgeSize = 'large';
fixture.detectChanges();
expect(badgeContent.getAttribute('aria-describedby')).toBeFalsy();

expect(badgeNativeElement.classList.contains('mat-badge-large')).toBe(true);
});
testComponent.badgeDescription = 'Describing a badge';
fixture.detectChanges();

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

testComponent.badgeOverlap = true;
fixture.detectChanges();
testComponent.badgeDescription = '';
fixture.detectChanges();

expect(badgeNativeElement.classList.contains('mat-badge-overlap')).toBe(true);
});
expect(badgeContent.getAttribute('aria-describedby')).toBeFalsy();
});

});

/** Test component that contains a MatBadge. */
@Component({
selector: 'test-app',
template: `
<span [matBadge]="badgeContent"
[matBadgeColor]="badgeColor"
[matBadgePosition]="badgeDirection"
[matBadgeHidden]="badgeHidden"
[matBadgeSize]="badgeSize"
[matBadgeOverlap]="badgeOverlap">
[matBadgeOverlap]="badgeOverlap"
[matBadgeDescription]="badgeDescription">
home
</span>
`
})
class BadgeWithTextContent {
class BadgeTestApp {
badgeColor: ThemePalette;
badgeContent = '1';
badgeDirection = 'above after';
badgeHidden = false;
badgeSize = 'medium';
badgeOverlap = false;
badgeDescription: string;
}
30 changes: 21 additions & 9 deletions src/lib/badge/badge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

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

/** The color of the badge. Can be `primary`, `accent`, or `warn`. */
@Input('matBadgeColor')
Expand Down Expand Up @@ -70,11 +70,11 @@ export class MatBadge {
/** Message used to describe the decorated element via aria-describedby */
@Input('matBadgeDescription')
get description(): string { return this._description; }
set description(val: string) {
if (this._description) {
this._updateHostAriaDescription(val, this._description);
set description(newDescription: string) {
if (newDescription !== this._description) {
this._updateHostAriaDescription(newDescription, this._description);
this._description = newDescription;
}
this._description = val;
}
private _description: string;

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

ngOnDestroy() {
if (this.description && this._badgeElement) {
this._ariaDescriber.removeDescription(this._badgeElement, this.description);
}
}

/** Injects a span element into the DOM with the content. */
private _updateTextContent(): HTMLSpanElement {
if (!this._badgeElement) {
Expand Down Expand Up @@ -150,11 +156,17 @@ export class MatBadge {
}

/** Sets the aria-label property on the element */
private _updateHostAriaDescription(val: string, prevVal: string): void {
private _updateHostAriaDescription(newDescription: string, oldDescription: string): void {
// ensure content available before setting label
const content = this._updateTextContent();
this._ariaDescriber.removeDescription(content, prevVal);
this._ariaDescriber.describe(content, val);

if (oldDescription) {
this._ariaDescriber.removeDescription(content, oldDescription);
}

if (newDescription) {
this._ariaDescriber.describe(content, newDescription);
}
}

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

0 comments on commit aed7e8a

Please sign in to comment.