From d31965e41aac8a459e120ae1f1d96ad99b74a313 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Thu, 9 Sep 2021 19:44:28 -0700 Subject: [PATCH] fix(material/badge): correctly apply badge description 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. --- src/dev-app/badge/badge-demo.html | 103 ++++++-------- src/material/badge/badge.spec.ts | 85 +++++------ src/material/badge/badge.ts | 171 ++++++++++------------- tools/public_api_guard/material/badge.md | 13 +- 4 files changed, 160 insertions(+), 212 deletions(-) diff --git a/src/dev-app/badge/badge-demo.html b/src/dev-app/badge/badge-demo.html index 0ed1d00a007f..283dac9b7e51 100644 --- a/src/dev-app/badge/badge-demo.html +++ b/src/dev-app/badge/badge-demo.html @@ -1,71 +1,24 @@
-
-

Text

- - Hello - - - - Hello - - - - Hello - - - - Hello - - - - Hello - - - - Aria - - - - Hidden - - - - -
-

Buttons

- - - - - - - - - - - -
@@ -99,4 +52,38 @@

Size

+
+

Text

+ + Hello + + + + Hello + + + + Hello + + + + Hello + + + + Hello + + + + Aria + + + + Hidden + + + + +
+ diff --git a/src/material/badge/badge.spec.ts b/src/material/badge/badge.spec.ts index 08d2dbae2e90..40370370ad25 100644 --- 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; 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: ` - Notifications - ` + template: `Notifications`, }) class BadgeOnTemplate { } diff --git a/src/material/badge/badge.ts b/src/material/badge/badge.ts index 1e321f7b3340..019791ebdfa2 100644 --- a/src/material/badge/badge.ts +++ b/src/material/badge/badge.ts @@ -14,11 +14,10 @@ import { Inject, Input, NgZone, - OnChanges, OnDestroy, + OnInit, Optional, Renderer2, - SimpleChanges, } from '@angular/core'; import {CanDisable, mixinDisabled, ThemePalette} from '@angular/material/core'; import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; @@ -38,6 +37,8 @@ export type MatBadgePosition = /** Allowed size options for matBadgeSize */ export type MatBadgeSize = 'small' | 'medium' | 'large'; +const BADGE_CONTENT_CLASS = 'mat-badge-content'; + /** Directive to display a text badge. */ @Directive({ selector: '[matBadge]', @@ -52,14 +53,11 @@ export type MatBadgeSize = 'small' | 'medium' | 'large'; '[class.mat-badge-small]': 'size === "small"', '[class.mat-badge-medium]': 'size === "medium"', '[class.mat-badge-large]': 'size === "large"', - '[class.mat-badge-hidden]': 'hidden || !_hasContent', + '[class.mat-badge-hidden]': 'hidden || !content', '[class.mat-badge-disabled]': 'disabled', }, }) -export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, CanDisable { - /** Whether the badge has any content. */ - _hasContent = false; - +export class MatBadge extends _MatBadgeBase implements OnInit, OnDestroy, CanDisable { /** The color of the badge. Can be `primary`, `accent`, or `warn`. */ @Input('matBadgeColor') get color(): ThemePalette { return this._color; } @@ -84,22 +82,20 @@ export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, Can @Input('matBadgePosition') position: MatBadgePosition = 'above after'; /** The content for the badge */ - @Input('matBadge') content: string | number | undefined | null; + @Input('matBadge') + get content(): string | number | undefined | null { + return this._content; + } + set content(newContent: string | number | undefined | null) { + this._updateRenderedContent(newContent); + } + private _content: string | number | undefined | null; /** Message used to describe the decorated element via aria-describedby */ @Input('matBadgeDescription') get description(): string { return this._description; } set description(newDescription: string) { - if (newDescription !== this._description) { - const badgeElement = this._badgeElement; - this._updateHostAriaDescription(newDescription, this._description); - this._description = newDescription; - - if (badgeElement) { - newDescription ? badgeElement.setAttribute('aria-label', newDescription) : - badgeElement.removeAttribute('aria-label'); - } - } + this._updateHostAriaDescription(newDescription); } private _description: string; @@ -117,8 +113,12 @@ export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, Can /** Unique id for the badge */ _id: number = nextId++; + /** Visible badge element. */ private _badgeElement: HTMLElement | undefined; + /** Whether the OnInit lifecycle hook has run yet */ + private _isInitialized = false; + constructor( private _ngZone: NgZone, private _elementRef: ElementRef, @@ -145,70 +145,54 @@ export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, Can return this.position.indexOf('before') === -1; } - ngOnChanges(changes: SimpleChanges) { - const contentChange = changes['content']; - - if (contentChange) { - const value = contentChange.currentValue; - this._hasContent = value != null && `${value}`.trim().length > 0; - this._updateTextContent(); - } - } - - ngOnDestroy() { - const badgeElement = this._badgeElement; - - if (badgeElement) { - if (this.description) { - this._ariaDescriber.removeDescription(badgeElement, this.description); - } - - // When creating a badge through the Renderer, Angular will keep it in an index. - // We have to destroy it ourselves, otherwise it'll be retained in memory. - if (this._renderer.destroyNode) { - this._renderer.destroyNode(badgeElement); - } - } - } - /** - * Gets the element into which the badge's content is being rendered. - * Undefined if the element hasn't been created (e.g. if the badge doesn't have content). + * Gets the element into which the badge's content is being rendered. Undefined if the element + * hasn't been created (e.g. if the badge doesn't have content). */ getBadgeElement(): HTMLElement | undefined { return this._badgeElement; } - /** Injects a span element into the DOM with the content. */ - private _updateTextContent(): HTMLSpanElement { - if (!this._badgeElement) { + ngOnInit() { + // We may have server-side rendered badge that we need to clear. + // We need to do this in ngOnInit because the full content of the component + // on which the badge is attached won't necessarily be in the DOM until this point. + this._clearExistingBadges(); + + if (this.content && !this._badgeElement) { this._badgeElement = this._createBadgeElement(); - } else { - this._badgeElement.textContent = this._stringifyContent(); + this._updateRenderedContent(this.content); } - return this._badgeElement; + + this._isInitialized = true; + } + + ngOnDestroy() { + // ViewEngine only: when creating a badge through the Renderer, Angular remembers its index. + // We have to destroy it ourselves, otherwise it'll be retained in memory. + if (this._renderer.destroyNode) { + this._renderer.destroyNode(this._badgeElement); + } + + this._ariaDescriber.removeDescription(this._elementRef.nativeElement, this.description); } /** Creates the badge element */ private _createBadgeElement(): HTMLElement { const badgeElement = this._renderer.createElement('span'); const activeClass = 'mat-badge-active'; - const contentClass = 'mat-badge-content'; - // Clear any existing badges which may have persisted from a server-side render. - this._clearExistingBadges(contentClass); badgeElement.setAttribute('id', `mat-badge-content-${this._id}`); - badgeElement.classList.add(contentClass); - badgeElement.textContent = this._stringifyContent(); + + // The badge is aria-hidden because we don't want it to appear in the page's navigation + // flow. Instead, we use the badge to describe the decorated element with aria-describedby. + badgeElement.setAttribute('aria-hidden', 'true'); + badgeElement.classList.add(BADGE_CONTENT_CLASS); if (this._animationMode === 'NoopAnimations') { badgeElement.classList.add('_mat-animation-noopable'); } - if (this.description) { - badgeElement.setAttribute('aria-label', this.description); - } - this._elementRef.nativeElement.appendChild(badgeElement); // animate in after insertion @@ -225,56 +209,55 @@ export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, Can return badgeElement; } - /** Sets the aria-label property on the element */ - private _updateHostAriaDescription(newDescription: string, oldDescription: string): void { - // ensure content available before setting label - const content = this._updateTextContent(); + /** Update the text content of the badge element in the DOM, creating the element if necessary. */ + private _updateRenderedContent(newContent: string | number | undefined | null): void { + const newContentNormalized: string = `${newContent ?? ''}`.trim(); - if (oldDescription) { - this._ariaDescriber.removeDescription(content, oldDescription); + // Don't create the badge element if the directive isn't initialized because we want to + // append the badge element to the *end* of the host element's content for backwards + // compatibility. + if (this._isInitialized && newContentNormalized && !this._badgeElement) { + this._badgeElement = this._createBadgeElement(); } + if (this._badgeElement) { + this._badgeElement.textContent = newContentNormalized; + } + + this._content = newContentNormalized; + } + + /** Updates the host element's aria description via AriaDescriber. */ + private _updateHostAriaDescription(newDescription: string): void { + this._ariaDescriber.removeDescription(this._elementRef.nativeElement, this.description); if (newDescription) { - this._ariaDescriber.describe(content, newDescription); + this._ariaDescriber.describe(this._elementRef.nativeElement, newDescription); } + this._description = newDescription; } /** Adds css theme class given the color to the component host */ private _setColor(colorPalette: ThemePalette) { - if (colorPalette !== this._color) { - const classList = this._elementRef.nativeElement.classList; - if (this._color) { - classList.remove(`mat-badge-${this._color}`); - } - if (colorPalette) { - classList.add(`mat-badge-${colorPalette}`); - } + const classList = this._elementRef.nativeElement.classList; + classList.remove(`mat-badge-${this._color}`); + if (colorPalette) { + classList.add(`mat-badge-${colorPalette}`); } } /** Clears any existing badges that might be left over from server-side rendering. */ - private _clearExistingBadges(cssClass: string) { - const element = this._elementRef.nativeElement; - let childCount = element.children.length; - - // Use a reverse while, because we'll be removing elements from the list as we're iterating. - while (childCount--) { - const currentChild = element.children[childCount]; - - if (currentChild.classList.contains(cssClass)) { - element.removeChild(currentChild); + private _clearExistingBadges() { + // Only check direct children of this host element in order to avoid deleting + // any badges that might exist in descendant elements. + const badges = + this._elementRef.nativeElement.querySelectorAll(`:scope > .${BADGE_CONTENT_CLASS}`); + for (const badgeElement of Array.from(badges)) { + if (badgeElement !== this._badgeElement) { + badgeElement.remove(); } } } - /** Gets the string representation of the badge content. */ - private _stringifyContent(): string { - // Convert null and undefined to an empty string which is consistent - // with how Angular handles them in inside template interpolations. - const content = this.content; - return content == null ? '' : `${content}`; - } - static ngAcceptInputType_disabled: BooleanInput; static ngAcceptInputType_hidden: BooleanInput; static ngAcceptInputType_overlap: BooleanInput; diff --git a/tools/public_api_guard/material/badge.md b/tools/public_api_guard/material/badge.md index dc728879f0ea..237b86dcc659 100644 --- a/tools/public_api_guard/material/badge.md +++ b/tools/public_api_guard/material/badge.md @@ -14,22 +14,21 @@ import * as i0 from '@angular/core'; import * as i2 from '@angular/cdk/a11y'; import * as i3 from '@angular/material/core'; import { NgZone } from '@angular/core'; -import { OnChanges } from '@angular/core'; import { OnDestroy } from '@angular/core'; +import { OnInit } from '@angular/core'; import { Renderer2 } from '@angular/core'; -import { SimpleChanges } from '@angular/core'; import { ThemePalette } from '@angular/material/core'; // @public -export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, CanDisable { +export class MatBadge extends _MatBadgeBase implements OnInit, OnDestroy, CanDisable { constructor(_ngZone: NgZone, _elementRef: ElementRef, _ariaDescriber: AriaDescriber, _renderer: Renderer2, _animationMode?: string | undefined); get color(): ThemePalette; set color(value: ThemePalette); - content: string | number | undefined | null; + get content(): string | number | undefined | null; + set content(newContent: string | number | undefined | null); get description(): string; set description(newDescription: string); getBadgeElement(): HTMLElement | undefined; - _hasContent: boolean; get hidden(): boolean; set hidden(val: boolean); _id: number; @@ -42,9 +41,9 @@ export class MatBadge extends _MatBadgeBase implements OnDestroy, OnChanges, Can // (undocumented) static ngAcceptInputType_overlap: BooleanInput; // (undocumented) - ngOnChanges(changes: SimpleChanges): void; - // (undocumented) ngOnDestroy(): void; + // (undocumented) + ngOnInit(): void; get overlap(): boolean; set overlap(val: boolean); position: MatBadgePosition;