fix(dialog): aria-label not being removed if text content changes after init

Currently we set an `aria-label` on the `mat-dialog-close` button if it's an icon button or it doesn't have text, however it wasn't accounting for the case where the text comes in at a later point.

Fixes #15542.

Author: crisbeto

src/lib/dialog/BUILD.bazel

@@ -19,6 +19,7 @@ ng_module(
     "//src/cdk/keycodes",
     "//src/cdk/overlay",
     "//src/cdk/portal",
+    "//src/cdk/observers",
     "//src/lib/core",
   ],
 )
@@ -50,6 +51,7 @@ ng_test_library(
     "//src/cdk/overlay",
     "//src/cdk/scrolling",
     "//src/cdk/testing",
+    "//src/cdk/observers",
     ":dialog",
   ]
 )

src/lib/dialog/dialog-content-directives.ts

@@ -14,7 +14,13 @@ import {
   Optional,
   SimpleChanges,
   ElementRef,
+  ChangeDetectorRef,
+  NgZone,
+  OnDestroy,
 } from '@angular/core';
+import {ContentObserver} from '@angular/cdk/observers';
+import {of as observableOf, Subscription} from 'rxjs';
+import {startWith} from 'rxjs/operators';
 import {MatDialog} from './dialog';
 import {MatDialogRef} from './dialog-ref';
 
@@ -33,7 +39,7 @@ let dialogElementUid = 0;
     'type': 'button', // Prevents accidental form submits.
   }
 })
-export class MatDialogClose implements OnInit, OnChanges {
+export class MatDialogClose implements OnInit, OnChanges, OnDestroy {
   /** Screenreader label for the button. */
   @Input('aria-label') ariaLabel: string = 'Close dialog';
 
@@ -48,10 +54,21 @@ export class MatDialogClose implements OnInit, OnChanges {
    */
   _hasAriaLabel?: boolean;
 
+  /** Subscription to changes in the button's content. */
+  private _contentChanges = Subscription.EMPTY;
+
   constructor(
     @Optional() public dialogRef: MatDialogRef<any>,
     private _elementRef: ElementRef<HTMLElement>,
-    private _dialog: MatDialog) {}
+    private _dialog: MatDialog,
+
+    /**
+     * @deprecated @breaking-change 9.0.0
+     * _contentObserver, _ngZone and _changeDetectorRef parameters to be made required.
+     */
+    private _contentObserver?: ContentObserver,
+    private _ngZone?: NgZone,
+    private _changeDetectorRef?: ChangeDetectorRef) {}
 
   ngOnInit() {
     if (!this.dialogRef) {
@@ -69,8 +86,23 @@ export class MatDialogClose implements OnInit, OnChanges {
       if (element.hasAttribute('mat-icon-button')) {
         this._hasAriaLabel = true;
       } else {
-        const buttonTextContent = element.textContent;
-        this._hasAriaLabel = !buttonTextContent || buttonTextContent.trim().length === 0;
+        // @breaking-change 9.0.0 Remove null checks for _contentObserver, _ngZone and
+        // _changeDetectorRef once they are made into required parameters.
+        const contentChangesStream = this._contentObserver ?
+          this._contentObserver.observe(this._elementRef) : observableOf<MutationRecord[]>();
+
+        // Toggle whether the button should have an aria-label, based on its content.
+        this._contentChanges = contentChangesStream.pipe(startWith(null)).subscribe(() => {
+          const buttonTextContent = element.textContent;
+          this._hasAriaLabel = !buttonTextContent || buttonTextContent.trim().length === 0;
+
+          // The content observer runs outside the `NgZone` so we need to bring it back in.
+          if (this._ngZone && this._changeDetectorRef) {
+            this._ngZone.run(() => {
+              this._changeDetectorRef!.markForCheck();
+            });
+          }
+        });
       }
     }
   }
@@ -87,6 +119,10 @@ export class MatDialogClose implements OnInit, OnChanges {
       this._hasAriaLabel = !!changes['ariaLabel'].currentValue;
     }
   }
+
+  ngOnDestroy() {
+    this._contentChanges.unsubscribe();
+  }
 }
 
 /**

src/lib/dialog/dialog.spec.ts

@@ -36,6 +36,7 @@ import {
   MAT_DIALOG_DEFAULT_OPTIONS
 } from './index';
 import {Subject} from 'rxjs';
+import {MutationObserverFactory} from '@angular/cdk/observers';
 
 
 describe('MatDialog', () => {
@@ -47,15 +48,30 @@ describe('MatDialog', () => {
   let testViewContainerRef: ViewContainerRef;
   let viewContainerFixture: ComponentFixture<ComponentWithChildViewContainer>;
   let mockLocation: SpyLocation;
+  let mutationCallbacks: Function[];
 
   beforeEach(fakeAsync(() => {
+    mutationCallbacks = [];
     TestBed.configureTestingModule({
       imports: [MatDialogModule, DialogTestModule],
       providers: [
         {provide: Location, useClass: SpyLocation},
         {provide: ScrollDispatcher, useFactory: () => ({
           scrolled: () => scrolledSubject.asObservable()
         })},
+        {
+          provide: MutationObserverFactory,
+          useValue: {
+            create: (callback: Function) => {
+              mutationCallbacks.push(callback);
+
+              return {
+                observe: () => {},
+                disconnect: () => {}
+              };
+            }
+          }
+        }
       ],
     });
 
@@ -1148,6 +1164,14 @@ describe('MatDialog', () => {
         expect(button.getAttribute('aria-label')).toBeTruthy();
       }));
 
+      it('should not have an aria-label if a button has bound text', fakeAsync(() => {
+        let button = overlayContainerElement.querySelector('.close-with-text-binding')!;
+        mutationCallbacks.forEach(callback => callback());
+        viewContainerFixture.detectChanges();
+
+        expect(button.getAttribute('aria-label')).toBeFalsy();
+      }));
+
       it('should not have an aria-label if a button has text', fakeAsync(() => {
         let button = overlayContainerElement.querySelector('[mat-dialog-close]')!;
         expect(button.getAttribute('aria-label')).toBeFalsy();
@@ -1511,6 +1535,7 @@ class PizzaMsg {
       <button class="close-without-text" mat-dialog-close></button>
       <button class="close-icon-button" mat-icon-button mat-dialog-close>exit</button>
       <button class="close-with-true" [mat-dialog-close]="true">Close and return true</button>
+      <button class="close-with-text-binding" mat-dialog-close>{{closeButtonText}}</button>
       <button
         class="close-with-aria-label"
         aria-label="Best close button ever"
@@ -1519,7 +1544,9 @@ class PizzaMsg {
     </mat-dialog-actions>
   `
 })
-class ContentElementDialog {}
+class ContentElementDialog {
+  closeButtonText = 'Close';
+}
 
 @Component({
   template: `
@@ -1531,6 +1558,7 @@ class ContentElementDialog {}
         <button class="close-without-text" mat-dialog-close></button>
         <button class="close-icon-button" mat-icon-button mat-dialog-close>exit</button>
         <button class="close-with-true" [mat-dialog-close]="true">Close and return true</button>
+        <button class="close-with-text-binding" mat-dialog-close>{{closeButtonText}}</button>
         <button
           class="close-with-aria-label"
           aria-label="Best close button ever"
@@ -1542,6 +1570,7 @@ class ContentElementDialog {}
 })
 class ComponentWithContentElementTemplateRef {
   @ViewChild(TemplateRef) templateRef: TemplateRef<any>;
+  closeButtonText = 'Close';
 }
 
 @Component({

tools/public_api_guard/lib/dialog.d.ts

@@ -44,14 +44,16 @@ export declare const matDialogAnimations: {
     readonly slideDialog: AnimationTriggerMetadata;
 };
 
-export declare class MatDialogClose implements OnInit, OnChanges {
+export declare class MatDialogClose implements OnInit, OnChanges, OnDestroy {
     _hasAriaLabel?: boolean;
     _matDialogClose: any;
     ariaLabel: string;
     dialogRef: MatDialogRef<any>;
     dialogResult: any;
-    constructor(dialogRef: MatDialogRef<any>, _elementRef: ElementRef<HTMLElement>, _dialog: MatDialog);
+    constructor(dialogRef: MatDialogRef<any>, _elementRef: ElementRef<HTMLElement>, _dialog: MatDialog,
+    _contentObserver?: ContentObserver | undefined, _ngZone?: NgZone | undefined, _changeDetectorRef?: ChangeDetectorRef | undefined);
     ngOnChanges(changes: SimpleChanges): void;
+    ngOnDestroy(): void;
     ngOnInit(): void;
 }