Skip to content

Commit 8f247b0

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 - Avoid opening multiple of the same dialog before animations complete by returning the previous `MatDialogRef` - update tests to use different dialog components when they need to open multiple dialogs quickly Fixes angular#21840
1 parent 56a30d0 commit 8f247b0

File tree

8 files changed

+114
-43
lines changed

8 files changed

+114
-43
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: 31 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();
@@ -1835,12 +1835,26 @@ class ComponentWithTemplateRef {
18351835
}
18361836
}
18371837

1838-
/** Simple component for testing ComponentPortal. */
1838+
/** Simple components for testing ComponentPortal and multiple dialogs. */
18391839
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
18401840
class PizzaMsg {
1841-
constructor(
1842-
public dialogRef: MatDialogRef<PizzaMsg>, public dialogInjector: Injector,
1843-
public directionality: Directionality) {}
1841+
constructor(public dialogRef: MatDialogRef<PizzaMsg>,
1842+
public dialogInjector: Injector,
1843+
public directionality: Directionality) {}
1844+
}
1845+
1846+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1847+
class PizzaMsgTwo {
1848+
constructor(public dialogRef: MatDialogRef<PizzaMsgTwo>,
1849+
public dialogInjector: Injector,
1850+
public directionality: Directionality) {}
1851+
}
1852+
1853+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1854+
class PizzaMsgThree {
1855+
constructor(public dialogRef: MatDialogRef<PizzaMsgThree>,
1856+
public dialogInjector: Injector,
1857+
public directionality: Directionality) {}
18441858
}
18451859

18461860
@Component({
@@ -1911,6 +1925,8 @@ const TEST_DIRECTIVES = [
19111925
ComponentWithChildViewContainer,
19121926
ComponentWithTemplateRef,
19131927
PizzaMsg,
1928+
PizzaMsgTwo,
1929+
PizzaMsgThree,
19141930
DirectiveWithViewContainer,
19151931
ComponentWithOnPushViewContainer,
19161932
ContentElementDialog,
@@ -1928,6 +1944,8 @@ const TEST_DIRECTIVES = [
19281944
ComponentWithChildViewContainer,
19291945
ComponentWithTemplateRef,
19301946
PizzaMsg,
1947+
PizzaMsgTwo,
1948+
PizzaMsgThree,
19311949
ContentElementDialog,
19321950
DialogWithInjectedData,
19331951
DialogWithoutFocusableElements,

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import {
1414
AnimationTriggerMetadata,
1515
} from '@angular/animations';
1616

17+
export const _transitionTime = 150;
18+
1719
/**
1820
* Animations used by MatDialog.
1921
* @docs-private
@@ -28,7 +30,7 @@ export const matDialogAnimations: {
2830
// decimate the animation performance. Leaving it as `none` solves both issues.
2931
state('void, exit', style({opacity: 0, transform: 'scale(0.7)'})),
3032
state('enter', style({transform: 'none'})),
31-
transition('* => enter', animate('150ms cubic-bezier(0, 0, 0.2, 1)',
33+
transition('* => enter', animate(`${_transitionTime}ms cubic-bezier(0, 0, 0.2, 1)`,
3234
style({transform: 'none', opacity: 1}))),
3335
transition('* => void, * => exit',
3436
animate('75ms cubic-bezier(0.4, 0.0, 0.2, 1)', style({opacity: 0}))),

‎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: 31 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;
@@ -1927,14 +1927,28 @@ class ComponentWithTemplateRef {
19271927
}
19281928
}
19291929

1930-
/** Simple component for testing ComponentPortal. */
1930+
/** Simple components for testing ComponentPortal and multiple dialogs. */
19311931
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
19321932
class PizzaMsg {
19331933
constructor(public dialogRef: MatDialogRef<PizzaMsg>,
19341934
public dialogInjector: Injector,
19351935
public directionality: Directionality) {}
19361936
}
19371937

1938+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1939+
class PizzaMsgTwo {
1940+
constructor(public dialogRef: MatDialogRef<PizzaMsgTwo>,
1941+
public dialogInjector: Injector,
1942+
public directionality: Directionality) {}
1943+
}
1944+
1945+
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
1946+
class PizzaMsgThree {
1947+
constructor(public dialogRef: MatDialogRef<PizzaMsgThree>,
1948+
public dialogInjector: Injector,
1949+
public directionality: Directionality) {}
1950+
}
1951+
19381952
@Component({
19391953
template: `
19401954
<h1 mat-dialog-title>This is the title</h1>
@@ -2004,6 +2018,8 @@ const TEST_DIRECTIVES = [
20042018
ComponentWithChildViewContainer,
20052019
ComponentWithTemplateRef,
20062020
PizzaMsg,
2021+
PizzaMsgTwo,
2022+
PizzaMsgThree,
20072023
DirectiveWithViewContainer,
20082024
ComponentWithOnPushViewContainer,
20092025
ContentElementDialog,
@@ -2021,6 +2037,8 @@ const TEST_DIRECTIVES = [
20212037
ComponentWithChildViewContainer,
20222038
ComponentWithTemplateRef,
20232039
PizzaMsg,
2040+
PizzaMsgTwo,
2041+
PizzaMsgThree,
20242042
ContentElementDialog,
20252043
DialogWithInjectedData,
20262044
DialogWithoutFocusableElements,

0 commit comments

Comments
 (0)