Skip to content

fix(material/dialog): improve screen reader support when opened #23228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix(material/dialog): improve screen reader support when opened #23228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/components-examples/material/dialog/dialog-harness/dialog-harness-example.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {NoopAnimationsModule} from '@angular/platform-browser/animations';

describe('DialogHarnessExample', () => {
let fixture: ComponentFixture<DialogHarnessExample>;
let fixtureTwo: ComponentFixture<DialogHarnessExample>;
let loader: HarnessLoader;

beforeAll(() => {
Expand All @@ -27,6 +28,8 @@ describe('DialogHarnessExample', () => {
}).compileComponents();
fixture = TestBed.createComponent(DialogHarnessExample);
fixture.detectChanges();
fixtureTwo = TestBed.createComponent(DialogHarnessExample);
fixtureTwo.detectChanges();
loader = TestbedHarnessEnvironment.documentRootLoader(fixture);
}));

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

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

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

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

it('should be able to close dialog', async () => {
fixture.componentInstance.open({disableClose: true});
fixture.componentInstance.open();
fixtureTwo.componentInstance.open();
let dialogs = await loader.getAllHarnesses(MatDialogHarness);

expect(dialogs.length).toBe(2);
Expand Down
68 changes: 47 additions & 21 deletions src/material-experimental/mdc-dialog/dialog.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,8 +615,8 @@ describe('MDC-based MatDialog', () => {

it('should close all of the dialogs', fakeAsync(() => {
dialog.open(PizzaMsg);
dialog.open(PizzaMsg);
dialog.open(PizzaMsg);
dialog.open(PizzaMsgTwo);
dialog.open(PizzaMsgThree);

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

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

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

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

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

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

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

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

it('should close all of the dialogs when the injectable is destroyed', fakeAsync(() => {
dialog.open(PizzaMsg);
dialog.open(PizzaMsg);
dialog.open(PizzaMsg);
dialog.open(PizzaMsgTwo);
dialog.open(PizzaMsgThree);

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

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

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

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

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

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

expect(one.id).toBeTruthy();
expect(two.id).toBeTruthy();
Expand Down Expand Up @@ -1188,7 +1188,7 @@ describe('MDC-based MatDialog', () => {
expect(document.activeElement!.id)
.not.toBe(
'dialog-trigger',
'Expcted the focus not to have changed before the animation finishes.');
'Expected the focus not to have changed before the animation finishes.');

flushMicrotasks();
viewContainerFixture.detectChanges();
Expand Down Expand Up @@ -1247,7 +1247,8 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
expect(lastFocusOrigin!)
.withContext('Expected the trigger button to be blurred').toBeNull();

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

Expand All @@ -1256,7 +1257,8 @@ describe('MDC-based MatDialog', () => {
tick(500);

expect(lastFocusOrigin!)
.toBe('keyboard', 'Expected the trigger button to be focused via keyboard');
.withContext('Expected the trigger button to be focused via keyboard')
.toBe('keyboard');

focusMonitor.stopMonitoring(button);
document.body.removeChild(button);
Expand All @@ -1281,7 +1283,8 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
expect(lastFocusOrigin!)
.withContext('Expected the trigger button to be blurred').toBeNull();

const backdrop =
overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement;
Expand All @@ -1291,7 +1294,8 @@ describe('MDC-based MatDialog', () => {
tick(500);

expect(lastFocusOrigin!)
.toBe('mouse', 'Expected the trigger button to be focused via mouse');
.withContext('Expected the trigger button to be focused via mouse')
.toBe('mouse');

focusMonitor.stopMonitoring(button);
document.body.removeChild(button);
Expand All @@ -1317,7 +1321,8 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
expect(lastFocusOrigin!)
.withContext('Expected the trigger button to be blurred').toBeNull();

const closeButton =
overlayContainerElement.querySelector('button[mat-dialog-close]') as HTMLElement;
Expand All @@ -1330,7 +1335,8 @@ describe('MDC-based MatDialog', () => {
tick(500);

expect(lastFocusOrigin!)
.toBe('keyboard', 'Expected the trigger button to be focused via keyboard');
.withContext('Expected the trigger button to be focused via keyboard')
.toBe('keyboard');

focusMonitor.stopMonitoring(button);
document.body.removeChild(button);
Expand All @@ -1355,7 +1361,8 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
expect(lastFocusOrigin!).toBeNull('Expected the trigger button to be blurred');
expect(lastFocusOrigin!)
.withContext('Expected the trigger button to be blurred').toBeNull();

const closeButton =
overlayContainerElement.querySelector('button[mat-dialog-close]') as HTMLElement;
Expand All @@ -1369,7 +1376,8 @@ describe('MDC-based MatDialog', () => {
tick(500);

expect(lastFocusOrigin!)
.toBe('mouse', 'Expected the trigger button to be focused via mouse');
.withContext('Expected the trigger button to be focused via mouse')
.toBe('mouse');

focusMonitor.stopMonitoring(button);
document.body.removeChild(button);
Expand Down Expand Up @@ -1959,12 +1967,26 @@ class ComponentWithTemplateRef {
}
}

/** Simple component for testing ComponentPortal. */
/** Simple components for testing ComponentPortal and multiple dialogs. */
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
class PizzaMsg {
constructor(
public dialogRef: MatDialogRef<PizzaMsg>, public dialogInjector: Injector,
public directionality: Directionality) {}
constructor(public dialogRef: MatDialogRef<PizzaMsg>,
public dialogInjector: Injector,
public directionality: Directionality) {}
}

@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
class PizzaMsgTwo {
constructor(public dialogRef: MatDialogRef<PizzaMsgTwo>,
public dialogInjector: Injector,
public directionality: Directionality) {}
}

@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
class PizzaMsgThree {
constructor(public dialogRef: MatDialogRef<PizzaMsgThree>,
public dialogInjector: Injector,
public directionality: Directionality) {}
}

@Component({
Expand Down Expand Up @@ -2035,6 +2057,8 @@ const TEST_DIRECTIVES = [
ComponentWithChildViewContainer,
ComponentWithTemplateRef,
PizzaMsg,
PizzaMsgTwo,
PizzaMsgThree,
DirectiveWithViewContainer,
ComponentWithOnPushViewContainer,
ContentElementDialog,
Expand All @@ -2052,6 +2076,8 @@ const TEST_DIRECTIVES = [
ComponentWithChildViewContainer,
ComponentWithTemplateRef,
PizzaMsg,
PizzaMsgTwo,
PizzaMsgThree,
ContentElementDialog,
DialogWithInjectedData,
DialogWithoutFocusableElements,
Expand Down
8 changes: 7 additions & 1 deletion src/material/dialog/dialog-animations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import {
AnimationTriggerMetadata,
} from '@angular/animations';

/**
* Animation transition time used by MatDialog.
* @docs-private
*/
export const _transitionTime = 150;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crisbeto I didn't want to copy this value all over the place. Instead I defined this const here, but I'm not confident that this is the right way to do it in this repo. Can you provide some guidance here please?


/**
* Animations used by MatDialog.
* @docs-private
Expand All @@ -28,7 +34,7 @@ export const matDialogAnimations: {
// decimate the animation performance. Leaving it as `none` solves both issues.
state('void, exit', style({opacity: 0, transform: 'scale(0.7)'})),
state('enter', style({transform: 'none'})),
transition('* => enter', animate('150ms cubic-bezier(0, 0, 0.2, 1)',
transition('* => enter', animate(`${_transitionTime}ms cubic-bezier(0, 0, 0.2, 1)`,
style({transform: 'none', opacity: 1}))),
transition('* => void, * => exit',
animate('75ms cubic-bezier(0.4, 0.0, 0.2, 1)', style({opacity: 0}))),
Expand Down
11 changes: 7 additions & 4 deletions src/material/dialog/dialog-container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
// Save the previously focused element. This element will be re-focused
// when the dialog closes.
this._capturePreviouslyFocusedElement();
// Move focus onto the dialog immediately in order to prevent the user
// from accidentally opening multiple dialogs at the same time.
this._focusDialogContainer();
}

/**
Expand Down Expand Up @@ -218,7 +215,13 @@ export abstract class _MatDialogContainerBase extends BasePortalOutlet {
break;
case true:
case 'first-tabbable':
this._focusTrap.focusInitialElementWhenReady();
this._focusTrap.focusInitialElementWhenReady().then(focusedSuccessfully => {
// If we weren't able to find a focusable element in the dialog, then focus the dialog
// container instead.
if (!focusedSuccessfully) {
this._focusDialogContainer();
}
});
break;
case 'first-heading':
this._focusByCssSelector('h1, h2, h3, h4, h5, h6, [role="heading"]');
Expand Down
Loading