From 4e84016fe41eee38b84508cc1b7321bf7f552229 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 29 Mar 2019 21:13:33 +0100 Subject: [PATCH] fix(dialog): remove default aria-label from mat-dialog-close Currently we add an `aria-label` by default to close buttons that don't have text. This is unreliable, because text can come in at any time and it takes a lot of code to keep track of it. Furthermore, the default text is in English which means that sites in different languages will have to override it anyway. These changes remove the default `aria-label` and add some guidance to the docs on how to deal with labelling the close button. Fixes #15542. --- src/lib/dialog/dialog-content-directives.ts | 28 +++------------------ src/lib/dialog/dialog.md | 4 +++ src/lib/dialog/dialog.spec.ts | 19 -------------- tools/public_api_guard/lib/dialog.d.ts | 1 - 4 files changed, 7 insertions(+), 45 deletions(-) diff --git a/src/lib/dialog/dialog-content-directives.ts b/src/lib/dialog/dialog-content-directives.ts index edca30a06c2b..5dec4d9dca9c 100644 --- a/src/lib/dialog/dialog-content-directives.ts +++ b/src/lib/dialog/dialog-content-directives.ts @@ -29,25 +29,19 @@ let dialogElementUid = 0; exportAs: 'matDialogClose', host: { '(click)': 'dialogRef.close(dialogResult)', - '[attr.aria-label]': '_hasAriaLabel ? ariaLabel : null', + '[attr.aria-label]': 'ariaLabel || null', 'type': 'button', // Prevents accidental form submits. } }) export class MatDialogClose implements OnInit, OnChanges { /** Screenreader label for the button. */ - @Input('aria-label') ariaLabel: string = 'Close dialog'; + @Input('aria-label') ariaLabel: string; /** Dialog close input. */ @Input('mat-dialog-close') dialogResult: any; @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, private _elementRef: ElementRef, @@ -62,30 +56,14 @@ 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) { - const proxiedChange = - changes['_matDialogClose'] || changes['_matDialogCloseResult']; + const proxiedChange = changes['_matDialogClose'] || changes['_matDialogCloseResult']; if (proxiedChange) { this.dialogResult = proxiedChange.currentValue; } - - if (changes['ariaLabel']) { - this._hasAriaLabel = !!changes['ariaLabel'].currentValue; - } } } diff --git a/src/lib/dialog/dialog.md b/src/lib/dialog/dialog.md index de198325bad3..9984e42e0145 100644 --- a/src/lib/dialog/dialog.md +++ b/src/lib/dialog/dialog.md @@ -154,6 +154,10 @@ a [focus trap](https://material.angular.io/cdk/a11y/overview#focustrap) to conta within itself. Once a dialog is closed, it will return focus to the element that was focused before the dialog was opened. +If you're adding a close button that doesn't have text (e.g. a purely icon-based button), make sure +that it has a meaningful `aria-label` so that users with assistive technology know what it is used +for. + #### Focus management By default, the first tabbable element within the dialog will receive focus upon open. This can be configured by setting the `cdkFocusInitial` attribute on another focusable element. diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index 246a1863ba0f..c6159bf2e844 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -1143,26 +1143,11 @@ 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]')!; @@ -1508,8 +1493,6 @@ class PizzaMsg { Lorem ipsum dolor sit amet. - - - -