fix(dialog): don't assign aria-label to close button if button has text (#11093)

No longer adds the `aria-label` to the `mat-dialog-close` directive, if the element that it's being applied to has text. This prevents the label from being read instead of the button text.

Fixes #11084.

Author: crisbeto

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

@@ -29,7 +29,7 @@ let dialogElementUid = 0;
   exportAs: 'matDialogClose',
   host: {
     '(click)': 'dialogRef.close(dialogResult)',
-    '[attr.aria-label]': 'ariaLabel',
+    '[attr.aria-label]': '_hasAriaLabel ? ariaLabel : null',
     'type': 'button', // Prevents accidental form submits.
   }
 })
@@ -42,6 +42,12 @@ export class MatDialogClose implements OnInit, OnChanges {
 
   @Input('matDialogClose') _matDialogClose: any;
 
+  /**
+   * Whether the button should have an `aria-label`. Used for clearing the
+   * attribute to prevent it from being read instead of the button's text.
+   */
+  _hasAriaLabel?: boolean;
+
   constructor(
     @Optional() public dialogRef: MatDialogRef<any>,
     private _elementRef: ElementRef<HTMLElement>,
@@ -56,6 +62,17 @@ export class MatDialogClose implements OnInit, OnChanges {
       // be resolved at constructor time.
       this.dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
     }
+
+    if (typeof this._hasAriaLabel === 'undefined') {
+      const element = this._elementRef.nativeElement;
+
+      if (element.hasAttribute('mat-icon-button')) {
+        this._hasAriaLabel = true;
+      } else {
+        const buttonTextContent = element.textContent;
+        this._hasAriaLabel = !buttonTextContent || buttonTextContent.trim().length === 0;
+      }
+    }
   }
 
   ngOnChanges(changes: SimpleChanges) {
@@ -64,6 +81,10 @@ export class MatDialogClose implements OnInit, OnChanges {
     if (proxiedChange) {
       this.dialogResult = proxiedChange.currentValue;
     }
+
+    if (changes.ariaLabel) {
+      this._hasAriaLabel = !!changes.ariaLabel.currentValue;
+    }
   }
 }
 

src/lib/dialog/dialog.spec.ts

@@ -1143,11 +1143,26 @@ describe('MatDialog', () => {
         expect(overlayContainerElement.querySelectorAll('.mat-dialog-container').length).toBe(1);
       });
 
+      it('should set an aria-label on a button without text', fakeAsync(() => {
+        let button = overlayContainerElement.querySelector('.close-without-text')!;
+        expect(button.getAttribute('aria-label')).toBeTruthy();
+      }));
+
+      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();
+      }));
+
       it('should allow for a user-specified aria-label on the close button', fakeAsync(() => {
         let button = overlayContainerElement.querySelector('.close-with-aria-label')!;
         expect(button.getAttribute('aria-label')).toBe('Best close button ever');
       }));
 
+      it('should always have an aria-label on a mat-icon-button', fakeAsync(() => {
+        let button = overlayContainerElement.querySelector('.close-icon-button')!;
+        expect(button.getAttribute('aria-label')).toBeTruthy();
+      }));
+
       it('should override the "type" attribute of the close button', () => {
         let button = overlayContainerElement.querySelector('button[mat-dialog-close]')!;
 
@@ -1493,11 +1508,13 @@ class PizzaMsg {
     <mat-dialog-content>Lorem ipsum dolor sit amet.</mat-dialog-content>
     <mat-dialog-actions>
       <button mat-dialog-close>Close</button>
+      <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-aria-label"
         aria-label="Best close button ever"
-        [mat-dialog-close]="true">Close</button>
+        [mat-dialog-close]="true"></button>
       <div mat-dialog-close>Should not close</div>
     </mat-dialog-actions>
   `
@@ -1511,11 +1528,13 @@ class ContentElementDialog {}
       <mat-dialog-content>Lorem ipsum dolor sit amet.</mat-dialog-content>
       <mat-dialog-actions>
         <button mat-dialog-close>Close</button>
+        <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-aria-label"
           aria-label="Best close button ever"
-          [mat-dialog-close]="true">Close</button>
+          [mat-dialog-close]="true"></button>
         <div mat-dialog-close>Should not close</div>
       </mat-dialog-actions>
     </ng-template>

tools/public_api_guard/lib/dialog.d.ts

@@ -45,6 +45,7 @@ export declare const matDialogAnimations: {
 };
 
 export declare class MatDialogClose implements OnInit, OnChanges {
+    _hasAriaLabel?: boolean;
     _matDialogClose: any;
     ariaLabel: string;
     dialogRef: MatDialogRef<any>;