Skip to content

Commit

Permalink
fix(cdk/dialog): default aria-modal to false (#30411)
Browse files Browse the repository at this point in the history
Having `aria-modal="true"` appears to hide other overlay-based components like `mat-select` from assistive technology. These changes set the default to `false` since the attribute is redundant anyway, because the dialog marks all outside content as `aria-hidden`.
  • Loading branch information
crisbeto authored Jan 29, 2025
1 parent 86ad515 commit c1ff40f
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 11 deletions.
8 changes: 6 additions & 2 deletions src/cdk/dialog/dialog-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,12 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
/** Dialog label applied via `aria-label` */
ariaLabel?: string | null = null;

/** Whether this is a modal dialog. Used to set the `aria-modal` attribute. */
ariaModal?: boolean = true;
/**
* Whether this is a modal dialog. Used to set the `aria-modal` attribute. Off by default,
* because it can interfere with other overlay-based components (e.g. `mat-select`) and because
* it is redundant since the dialog marks all outside content as `aria-hidden` anyway.
*/
ariaModal?: boolean = false;

/**
* Where the dialog should focus on open.
Expand Down
15 changes: 13 additions & 2 deletions src/cdk/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,18 @@ describe('Dialog', () => {
viewContainerFixture.detectChanges();
let dialogContainerElement = overlayContainerElement.querySelector('cdk-dialog-container')!;
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('true');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('false');
});

it('should be able to set aria-modal', () => {
dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef,
ariaModal: true,
});
viewContainerFixture.detectChanges();

const container = overlayContainerElement.querySelector('cdk-dialog-container')!;
expect(container.getAttribute('aria-modal')).toBe('true');
});

it('should open a dialog with a template', () => {
Expand All @@ -117,7 +128,7 @@ describe('Dialog', () => {

let dialogContainerElement = overlayContainerElement.querySelector('cdk-dialog-container')!;
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('true');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('false');

dialogRef.close();
});
Expand Down
8 changes: 6 additions & 2 deletions src/material/bottom-sheet/bottom-sheet-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,12 @@ export class MatBottomSheetConfig<D = any> {
/** Aria label to assign to the bottom sheet element. */
ariaLabel?: string | null = null;

/** Whether this is a modal bottom sheet. Used to set the `aria-modal` attribute. */
ariaModal?: boolean = true;
/**
* Whether this is a modal dialog. Used to set the `aria-modal` attribute. Off by default,
* because it can interfere with other overlay-based components (e.g. `mat-select`) and because
* it is redundant since the dialog marks all outside content as `aria-hidden` anyway.
*/
ariaModal?: boolean = false;

/**
* Whether the bottom sheet should close when the user goes backwards/forwards in history.
Expand Down
12 changes: 11 additions & 1 deletion src/material/bottom-sheet/bottom-sheet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,17 @@ describe('MatBottomSheet', () => {

const containerElement = overlayContainerElement.querySelector('mat-bottom-sheet-container')!;
expect(containerElement.getAttribute('role')).toBe('dialog');
expect(containerElement.getAttribute('aria-modal')).toBe('true');
expect(containerElement.getAttribute('aria-modal')).toBe('false');
});

it('should be able to set aria-modal', () => {
bottomSheet.open(PizzaMsg, {
ariaModal: true,
});
viewContainerFixture.detectChanges();

const container = overlayContainerElement.querySelector('mat-bottom-sheet-container')!;
expect(container.getAttribute('aria-modal')).toBe('true');
});

it('should close a bottom sheet via the escape key', fakeAsync(() => {
Expand Down
8 changes: 6 additions & 2 deletions src/material/dialog/dialog-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,12 @@ export class MatDialogConfig<D = any> {
/** Aria label to assign to the dialog element. */
ariaLabel?: string | null = null;

/** Whether this is a modal dialog. Used to set the `aria-modal` attribute. */
ariaModal?: boolean = true;
/**
* Whether this is a modal dialog. Used to set the `aria-modal` attribute. Off by default,
* because it can interfere with other overlay-based components (e.g. `mat-select`) and because
* it is redundant since the dialog marks all outside content as `aria-hidden` anyway.
*/
ariaModal?: boolean = false;

/**
* Where the dialog should focus on open.
Expand Down
15 changes: 13 additions & 2 deletions src/material/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,18 @@ describe('MatDialog', () => {
viewContainerFixture.detectChanges();
let dialogContainerElement = overlayContainerElement.querySelector('mat-dialog-container')!;
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('true');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('false');
});

it('should be able to set aria-modal', () => {
dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef,
ariaModal: true,
});
viewContainerFixture.detectChanges();

const container = overlayContainerElement.querySelector('mat-dialog-container')!;
expect(container.getAttribute('aria-modal')).toBe('true');
});

it('should open a dialog with a template', () => {
Expand All @@ -134,7 +145,7 @@ describe('MatDialog', () => {

let dialogContainerElement = overlayContainerElement.querySelector('mat-dialog-container')!;
expect(dialogContainerElement.getAttribute('role')).toBe('dialog');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('true');
expect(dialogContainerElement.getAttribute('aria-modal')).toBe('false');

dialogRef.close();
});
Expand Down

0 comments on commit c1ff40f

Please sign in to comment.