Skip to content

Commit 92a5dce

Browse files
SplaktarSplaktar
committed
fix(material/dialog): improve screen reader support when opened
- notify screen reader users that they have entered a dialog - previously only the focused element would be read i.e. "Close Button Press Search plus Space to activate" - now the screen reader user gets the normal dialog behavior, which is to read the dialog title, role, content, and then tell the user about the focused element - this matches the guidance here: https://www.w3.org/TR/wai-aria-practices-1.1/examples/dialog-modal/dialog.html - block the same dialog from opening again for 300ms to avoid multiple of the same dialog opening before animations complete and the focus is moved off of the trigger - update tests to use different dialog components when they need to open multiple dialogs Fixes angular#21840
1 parent bca634b commit 92a5dce

File tree

7 files changed

+125
-42
lines changed

7 files changed

+125
-42
lines changed

‎src/components-examples/material/dialog/dialog-harness/dialog-harness-example.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {NoopAnimationsModule} from '@angular/platform-browser/animations';
1212

1313
describe('DialogHarnessExample', () => {
1414
let fixture: ComponentFixture<DialogHarnessExample>;
15+
let fixtureTwo: ComponentFixture<DialogHarnessExample>;
1516
let loader: HarnessLoader;
1617

1718
beforeAll(() => {
@@ -27,6 +28,8 @@ describe('DialogHarnessExample', () => {
2728
}).compileComponents();
2829
fixture = TestBed.createComponent(DialogHarnessExample);
2930
fixture.detectChanges();
31+
fixtureTwo = TestBed.createComponent(DialogHarnessExample);
32+
fixtureTwo.detectChanges();
3033
loader = TestbedHarnessEnvironment.documentRootLoader(fixture);
3134
}));
3235

@@ -38,7 +41,7 @@ describe('DialogHarnessExample', () => {
3841

3942
it('should load harness for dialog with specific id', async () => {
4043
fixture.componentInstance.open({id: 'my-dialog'});
41-
fixture.componentInstance.open({id: 'other'});
44+
fixtureTwo.componentInstance.open({id: 'other'});
4245
let dialogs = await loader.getAllHarnesses(MatDialogHarness);
4346
expect(dialogs.length).toBe(2);
4447

@@ -48,7 +51,7 @@ describe('DialogHarnessExample', () => {
4851

4952
it('should be able to get role of dialog', async () => {
5053
fixture.componentInstance.open({role: 'alertdialog'});
51-
fixture.componentInstance.open({role: 'dialog'});
54+
fixtureTwo.componentInstance.open({role: 'dialog'});
5255
const dialogs = await loader.getAllHarnesses(MatDialogHarness);
5356
expect(await dialogs[0].getRole()).toBe('alertdialog');
5457
expect(await dialogs[1].getRole()).toBe('dialog');
@@ -57,7 +60,7 @@ describe('DialogHarnessExample', () => {
5760

5861
it('should be able to close dialog', async () => {
5962
fixture.componentInstance.open({disableClose: true});
60-
fixture.componentInstance.open();
63+
fixtureTwo.componentInstance.open();
6164
let dialogs = await loader.getAllHarnesses(MatDialogHarness);
6265

6366
expect(dialogs.length).toBe(2);

‎src/material-experimental/mdc-dialog/dialog.spec.ts

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -615,8 +615,8 @@ describe('MDC-based MatDialog', () => {
615615

616616
it('should close all of the dialogs', fakeAsync(() => {
617617
dialog.open(PizzaMsg);
618-
dialog.open(PizzaMsg);
619-
dialog.open(PizzaMsg);
618+
dialog.open(PizzaMsgTwo);
619+
dialog.open(PizzaMsgThree);
620620

621621
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(3);
622622

@@ -629,7 +629,7 @@ describe('MDC-based MatDialog', () => {
629629

630630
it('should close all dialogs when the user goes forwards/backwards in history', fakeAsync(() => {
631631
dialog.open(PizzaMsg);
632-
dialog.open(PizzaMsg);
632+
dialog.open(PizzaMsgTwo);
633633

634634
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
635635

@@ -642,7 +642,7 @@ describe('MDC-based MatDialog', () => {
642642

643643
it('should close all open dialogs when the location hash changes', fakeAsync(() => {
644644
dialog.open(PizzaMsg);
645-
dialog.open(PizzaMsg);
645+
dialog.open(PizzaMsgTwo);
646646

647647
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
648648

@@ -655,8 +655,8 @@ describe('MDC-based MatDialog', () => {
655655

656656
it('should close all of the dialogs when the injectable is destroyed', fakeAsync(() => {
657657
dialog.open(PizzaMsg);
658-
dialog.open(PizzaMsg);
659-
dialog.open(PizzaMsg);
658+
dialog.open(PizzaMsgTwo);
659+
dialog.open(PizzaMsgThree);
660660

661661
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(3);
662662

@@ -685,7 +685,7 @@ describe('MDC-based MatDialog', () => {
685685

686686
it('should allow the consumer to disable closing a dialog on navigation', fakeAsync(() => {
687687
dialog.open(PizzaMsg);
688-
dialog.open(PizzaMsg, {closeOnNavigation: false});
688+
dialog.open(PizzaMsgTwo, {closeOnNavigation: false});
689689

690690
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
691691

@@ -774,7 +774,7 @@ describe('MDC-based MatDialog', () => {
774774

775775
it('should assign a unique id to each dialog', () => {
776776
const one = dialog.open(PizzaMsg);
777-
const two = dialog.open(PizzaMsg);
777+
const two = dialog.open(PizzaMsgTwo);
778778

779779
expect(one.id).toBeTruthy();
780780
expect(two.id).toBeTruthy();
@@ -1064,7 +1064,7 @@ describe('MDC-based MatDialog', () => {
10641064
expect(document.activeElement!.id)
10651065
.not.toBe(
10661066
'dialog-trigger',
1067-
'Expcted the focus not to have changed before the animation finishes.');
1067+
'Expected the focus not to have changed before the animation finishes.');
10681068

10691069
flushMicrotasks();
10701070
viewContainerFixture.detectChanges();
@@ -1789,6 +1789,15 @@ describe('MDC-based MatDialog with animations enabled', () => {
17891789
flush();
17901790
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
17911791
}));
1792+
1793+
it('should not allow the same dialog to open again until 300ms have passed',
1794+
() => {
1795+
const openTwoDialogs = () => {
1796+
dialog.open(PizzaMsg);
1797+
dialog.open(PizzaMsg);
1798+
};
1799+
expect(openTwoDialogs).toThrowError(/Ignoring attempt to open dialog/);
1800+
});
17921801
});
17931802

17941803
@Directive({selector: 'dir-with-view-container'})
@@ -1835,12 +1844,26 @@ class ComponentWithTemplateRef {
18351844
}
18361845
}
18371846

1838-
/** Simple component for testing ComponentPortal. */
1847+
/** Simple components for testing ComponentPortal and multiple dialogs. */
18391848
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
18401849
class PizzaMsg {
1841-
constructor(
1842-
public dialogRef: MatDialogRef<PizzaMsg>, public dialogInjector: Injector,
1843-
public directionality: Directionality) {}
1850+
constructor(public dialogRef: MatDialogRef<PizzaMsg>,
1851+
public dialogInjector: Injector,
1852+
public directionality: Directionality) {}
1853+
}
1854+
1855+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1856+
class PizzaMsgTwo {
1857+
constructor(public dialogRef: MatDialogRef<PizzaMsgTwo>,
1858+
public dialogInjector: Injector,
1859+
public directionality: Directionality) {}
1860+
}
1861+
1862+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1863+
class PizzaMsgThree {
1864+
constructor(public dialogRef: MatDialogRef<PizzaMsgThree>,
1865+
public dialogInjector: Injector,
1866+
public directionality: Directionality) {}
18441867
}
18451868

18461869
@Component({
@@ -1911,6 +1934,8 @@ const TEST_DIRECTIVES = [
19111934
ComponentWithChildViewContainer,
19121935
ComponentWithTemplateRef,
19131936
PizzaMsg,
1937+
PizzaMsgTwo,
1938+
PizzaMsgThree,
19141939
DirectiveWithViewContainer,
19151940
ComponentWithOnPushViewContainer,
19161941
ContentElementDialog,
@@ -1928,6 +1953,8 @@ const TEST_DIRECTIVES = [
19281953
ComponentWithChildViewContainer,
19291954
ComponentWithTemplateRef,
19301955
PizzaMsg,
1956+
PizzaMsgTwo,
1957+
PizzaMsgThree,
19311958
ContentElementDialog,
19321959
DialogWithInjectedData,
19331960
DialogWithoutFocusableElements,

‎src/material/dialog/dialog-container.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,6 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
105105
// Save the previously focused element. This element will be re-focused
106106
// when the dialog closes.
107107
this._capturePreviouslyFocusedElement();
108-
// Move focus onto the dialog immediately in order to prevent the user
109-
// from accidentally opening multiple dialogs at the same time.
110-
this._focusDialogContainer();
111108
}
112109

113110
/**
@@ -154,7 +151,7 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
154151
const focusContainer = !this._config.autoFocus || !this._focusTrap.focusInitialElement();
155152

156153
if (focusContainer) {
157-
this._elementRef.nativeElement.focus();
154+
this._focusDialogContainer();
158155
}
159156
}
160157
}
@@ -165,14 +162,20 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
165162
// ready in instances where change detection has to run first. To deal with this, we simply
166163
// wait for the microtask queue to be empty.
167164
if (this._config.autoFocus) {
168-
this._focusTrap.focusInitialElementWhenReady();
165+
this._focusTrap.focusInitialElementWhenReady().then(focusedSuccessfully => {
166+
// If we weren't able to find a focusable element in the dialog, then focus the dialog
167+
// container instead.
168+
if (!focusedSuccessfully) {
169+
this._focusDialogContainer();
170+
}
171+
});
169172
} else if (!this._containsFocus()) {
170173
// Otherwise ensure that focus is on the dialog container. It's possible that a different
171174
// component tried to move focus while the open animation was running. See:
172175
// https://github.com/angular/components/issues/16215. Note that we only want to do this
173176
// if the focus isn't inside the dialog already, because it's possible that the consumer
174177
// turned off `autoFocus` in order to move focus themselves.
175-
this._elementRef.nativeElement.focus();
178+
this._focusDialogContainer();
176179
}
177180
}
178181

‎src/material/dialog/dialog.spec.ts

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -670,8 +670,8 @@ describe('MatDialog', () => {
670670

671671
it('should close all of the dialogs', fakeAsync(() => {
672672
dialog.open(PizzaMsg);
673-
dialog.open(PizzaMsg);
674-
dialog.open(PizzaMsg);
673+
dialog.open(PizzaMsgTwo);
674+
dialog.open(PizzaMsgThree);
675675

676676
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(3);
677677

@@ -697,7 +697,7 @@ describe('MatDialog', () => {
697697

698698
it('should close all dialogs when the user goes forwards/backwards in history', fakeAsync(() => {
699699
dialog.open(PizzaMsg);
700-
dialog.open(PizzaMsg);
700+
dialog.open(PizzaMsgTwo);
701701

702702
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
703703

@@ -710,7 +710,7 @@ describe('MatDialog', () => {
710710

711711
it('should close all open dialogs when the location hash changes', fakeAsync(() => {
712712
dialog.open(PizzaMsg);
713-
dialog.open(PizzaMsg);
713+
dialog.open(PizzaMsgTwo);
714714

715715
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
716716

@@ -723,8 +723,8 @@ describe('MatDialog', () => {
723723

724724
it('should close all of the dialogs when the injectable is destroyed', fakeAsync(() => {
725725
dialog.open(PizzaMsg);
726-
dialog.open(PizzaMsg);
727-
dialog.open(PizzaMsg);
726+
dialog.open(PizzaMsgTwo);
727+
dialog.open(PizzaMsgThree);
728728

729729
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(3);
730730

@@ -754,7 +754,7 @@ describe('MatDialog', () => {
754754

755755
it('should allow the consumer to disable closing a dialog on navigation', fakeAsync(() => {
756756
dialog.open(PizzaMsg);
757-
dialog.open(PizzaMsg, {closeOnNavigation: false});
757+
dialog.open(PizzaMsgTwo, {closeOnNavigation: false});
758758

759759
expect(overlayContainerElement.querySelectorAll('mat-dialog-container').length).toBe(2);
760760

@@ -849,7 +849,7 @@ describe('MatDialog', () => {
849849

850850
it('should assign a unique id to each dialog', () => {
851851
const one = dialog.open(PizzaMsg);
852-
const two = dialog.open(PizzaMsg);
852+
const two = dialog.open(PizzaMsgTwo);
853853

854854
expect(one.id).toBeTruthy();
855855
expect(two.id).toBeTruthy();
@@ -1227,7 +1227,7 @@ describe('MatDialog', () => {
12271227

12281228
tick(500);
12291229
viewContainerFixture.detectChanges();
1230-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1230+
expect(lastFocusOrigin!).toBe('program');
12311231

12321232
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
12331233

@@ -1260,7 +1260,7 @@ describe('MatDialog', () => {
12601260

12611261
tick(500);
12621262
viewContainerFixture.detectChanges();
1263-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1263+
expect(lastFocusOrigin!).toBe('program');
12641264

12651265
const backdrop = overlayContainerElement
12661266
.querySelector('.cdk-overlay-backdrop') as HTMLElement;
@@ -1296,7 +1296,7 @@ describe('MatDialog', () => {
12961296

12971297
tick(500);
12981298
viewContainerFixture.detectChanges();
1299-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1299+
expect(lastFocusOrigin!).toBe('program');
13001300

13011301
const closeButton = overlayContainerElement
13021302
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1333,7 +1333,7 @@ describe('MatDialog', () => {
13331333

13341334
tick(500);
13351335
viewContainerFixture.detectChanges();
1336-
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
1336+
expect(lastFocusOrigin!).toBe('program');
13371337

13381338
const closeButton = overlayContainerElement
13391339
.querySelector('button[mat-dialog-close]') as HTMLElement;
@@ -1881,6 +1881,15 @@ describe('MatDialog with animations enabled', () => {
18811881
flush();
18821882
expect(dialogRef.getState()).toBe(MatDialogState.CLOSED);
18831883
}));
1884+
1885+
it('should not allow the same dialog to open again until 300ms have passed',
1886+
() => {
1887+
const openTwoDialogs = () => {
1888+
dialog.open(PizzaMsg);
1889+
dialog.open(PizzaMsg);
1890+
};
1891+
expect(openTwoDialogs).toThrowError(/Ignoring attempt to open dialog/);
1892+
});
18841893
});
18851894

18861895
@Directive({selector: 'dir-with-view-container'})
@@ -1927,14 +1936,28 @@ class ComponentWithTemplateRef {
19271936
}
19281937
}
19291938

1930-
/** Simple component for testing ComponentPortal. */
1939+
/** Simple components for testing ComponentPortal and multiple dialogs. */
19311940
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
19321941
class PizzaMsg {
19331942
constructor(public dialogRef: MatDialogRef<PizzaMsg>,
19341943
public dialogInjector: Injector,
19351944
public directionality: Directionality) {}
19361945
}
19371946

1947+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1948+
class PizzaMsgTwo {
1949+
constructor(public dialogRef: MatDialogRef<PizzaMsgTwo>,
1950+
public dialogInjector: Injector,
1951+
public directionality: Directionality) {}
1952+
}
1953+
1954+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1955+
class PizzaMsgThree {
1956+
constructor(public dialogRef: MatDialogRef<PizzaMsgThree>,
1957+
public dialogInjector: Injector,
1958+
public directionality: Directionality) {}
1959+
}
1960+
19381961
@Component({
19391962
template: `
19401963
<h1 mat-dialog-title>This is the title</h1>
@@ -2004,6 +2027,8 @@ const TEST_DIRECTIVES = [
20042027
ComponentWithChildViewContainer,
20052028
ComponentWithTemplateRef,
20062029
PizzaMsg,
2030+
PizzaMsgTwo,
2031+
PizzaMsgThree,
20072032
DirectiveWithViewContainer,
20082033
ComponentWithOnPushViewContainer,
20092034
ContentElementDialog,
@@ -2021,6 +2046,8 @@ const TEST_DIRECTIVES = [
20212046
ComponentWithChildViewContainer,
20222047
ComponentWithTemplateRef,
20232048
PizzaMsg,
2049+
PizzaMsgTwo,
2050+
PizzaMsgThree,
20242051
ContentElementDialog,
20252052
DialogWithInjectedData,
20262053
DialogWithoutFocusableElements,

0 commit comments

Comments
 (0)