Skip to content

fix(cdk/dialog): avoid setting aria-hidden before focus has moved #31030

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.

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

Merged
merged 1 commit into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 14 additions & 8 deletions goldens/cdk/dialog/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { ViewContainerRef } from '@angular/core';
export type AutoFocusTarget = 'dialog' | 'first-tabbable' | 'first-heading';

// @public
export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends BasePortalOutlet implements OnDestroy {
export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends BasePortalOutlet implements DialogContainer, OnDestroy {
constructor(...args: unknown[]);
// (undocumented)
_addAriaLabelledBy(id: string): void;
Expand All @@ -62,6 +62,8 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends B
// (undocumented)
protected _focusTrapFactory: FocusTrapFactory;
// (undocumented)
_focusTrapped: Observable<void>;
// (undocumented)
ngOnDestroy(): void;
// (undocumented)
protected _ngZone: NgZone;
Expand Down Expand Up @@ -111,7 +113,7 @@ export interface DialogCloseOptions {
}

// @public
export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet = BasePortalOutlet> {
export class DialogConfig<D = unknown, R = unknown, C extends DialogContainer = BasePortalOutlet> {
ariaDescribedBy?: string | null;
ariaLabel?: string | null;
ariaLabelledBy?: string | null;
Expand Down Expand Up @@ -149,6 +151,13 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
width?: string;
}

// @public
export type DialogContainer = BasePortalOutlet & {
_focusTrapped?: Observable<void>;
_closeInteractionType?: FocusOrigin;
_recaptureFocus?: () => void;
};

// @public (undocumented)
export class DialogModule {
// (undocumented)
Expand All @@ -161,19 +170,16 @@ export class DialogModule {

// @public
export class DialogRef<R = unknown, C = unknown> {
constructor(overlayRef: OverlayRef, config: DialogConfig<any, DialogRef<R, C>, BasePortalOutlet>);
constructor(overlayRef: OverlayRef, config: DialogConfig<any, DialogRef<R, C>, DialogContainer>);
addPanelClass(classes: string | string[]): this;
readonly backdropClick: Observable<MouseEvent>;
close(result?: R, options?: DialogCloseOptions): void;
readonly closed: Observable<R | undefined>;
readonly componentInstance: C | null;
readonly componentRef: ComponentRef<C> | null;
// (undocumented)
readonly config: DialogConfig<any, DialogRef<R, C>, BasePortalOutlet>;
readonly containerInstance: BasePortalOutlet & {
_closeInteractionType?: FocusOrigin;
_recaptureFocus?: () => void;
};
readonly config: DialogConfig<any, DialogRef<R, C>, DialogContainer>;
readonly containerInstance: DialogContainer;
disableClose: boolean | undefined;
readonly id: string;
readonly keydownEvents: Observable<KeyboardEvent>;
Expand Down
11 changes: 10 additions & 1 deletion src/cdk/dialog/dialog-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,25 @@
import {ViewContainerRef, Injector, StaticProvider, Type} from '@angular/core';
import {Direction} from '../bidi';
import {PositionStrategy, ScrollStrategy} from '../overlay';
import {Observable} from 'rxjs';
import {BasePortalOutlet} from '../portal';
import {FocusOrigin} from '../a11y';

/** Options for where to set focus to automatically on dialog open */
export type AutoFocusTarget = 'dialog' | 'first-tabbable' | 'first-heading';

/** Valid ARIA roles for a dialog. */
export type DialogRole = 'dialog' | 'alertdialog';

/** Component that can be used as the container for the dialog. */
export type DialogContainer = BasePortalOutlet & {
_focusTrapped?: Observable<void>;
_closeInteractionType?: FocusOrigin;
_recaptureFocus?: () => void;
};

/** Configuration for opening a modal dialog. */
export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet = BasePortalOutlet> {
export class DialogConfig<D = unknown, R = unknown, C extends DialogContainer = BasePortalOutlet> {
/**
* Where the attached component should live in Angular's *logical* component tree.
* This affects what is available for injection and the change detection order for the
Expand Down
16 changes: 9 additions & 7 deletions src/cdk/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ import {
inject,
DOCUMENT,
} from '@angular/core';
import {DialogConfig} from './dialog-config';
import {DialogConfig, DialogContainer} from './dialog-config';
import {Observable, Subject} from 'rxjs';

export function throwDialogContentAlreadyAttachedError() {
throw Error('Attempting to attach dialog content after content is already attached');
Expand Down Expand Up @@ -71,7 +72,7 @@ export function throwDialogContentAlreadyAttachedError() {
})
export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
extends BasePortalOutlet
implements OnDestroy
implements DialogContainer, OnDestroy
{
protected _elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
protected _focusTrapFactory = inject(FocusTrapFactory);
Expand All @@ -80,13 +81,16 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
protected _ngZone = inject(NgZone);
private _focusMonitor = inject(FocusMonitor);
private _renderer = inject(Renderer2);

protected readonly _changeDetectorRef = inject(ChangeDetectorRef);
private _injector = inject(Injector);
private _platform = inject(Platform);
protected _document = inject(DOCUMENT, {optional: true})!;

/** The portal outlet inside of this container into which the dialog content will be loaded. */
@ViewChild(CdkPortalOutlet, {static: true}) _portalOutlet: CdkPortalOutlet;

_focusTrapped: Observable<void> = new Subject<void>();

/** The class that traps and manages focus within the dialog. */
private _focusTrap: FocusTrap | null = null;

Expand All @@ -108,10 +112,6 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
*/
_ariaLabelledByQueue: string[] = [];

protected readonly _changeDetectorRef = inject(ChangeDetectorRef);

private _injector = inject(Injector);

private _isDestroyed = false;

constructor(...args: unknown[]);
Expand Down Expand Up @@ -156,6 +156,7 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
}

ngOnDestroy() {
(this._focusTrapped as Subject<void>).complete();
this._isDestroyed = true;
this._restoreFocus();
}
Expand Down Expand Up @@ -291,6 +292,7 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
this._focusByCssSelector(this._config.autoFocus!, options);
break;
}
(this._focusTrapped as Subject<void>).next();
},
{injector: this._injector},
);
Expand Down
14 changes: 5 additions & 9 deletions src/cdk/dialog/dialog-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
import {OverlayRef} from '../overlay';
import {ESCAPE, hasModifierKey} from '../keycodes';
import {Observable, Subject, Subscription} from 'rxjs';
import {DialogConfig} from './dialog-config';
import {DialogConfig, DialogContainer} from './dialog-config';
import {FocusOrigin} from '../a11y';
import {BasePortalOutlet} from '../portal';
import {ComponentRef} from '@angular/core';

/** Additional options that can be passed in when closing a dialog. */
Expand All @@ -37,10 +36,7 @@ export class DialogRef<R = unknown, C = unknown> {
readonly componentRef: ComponentRef<C> | null;

/** Instance of the container that is rendering out the dialog content. */
readonly containerInstance: BasePortalOutlet & {
_closeInteractionType?: FocusOrigin;
_recaptureFocus?: () => void;
};
readonly containerInstance: DialogContainer;

/** Whether the user is allowed to close the dialog. */
disableClose: boolean | undefined;
Expand All @@ -65,7 +61,7 @@ export class DialogRef<R = unknown, C = unknown> {

constructor(
readonly overlayRef: OverlayRef,
readonly config: DialogConfig<any, DialogRef<R, C>, BasePortalOutlet>,
readonly config: DialogConfig<any, DialogRef<R, C>, DialogContainer>,
) {
this.disableClose = config.disableClose;
this.backdropClick = overlayRef.backdropClick();
Expand Down Expand Up @@ -114,7 +110,7 @@ export class DialogRef<R = unknown, C = unknown> {
closedSubject.next(result);
closedSubject.complete();
(this as {componentInstance: C}).componentInstance = (
this as {containerInstance: BasePortalOutlet}
this as {containerInstance: DialogContainer}
).containerInstance = null!;
}
}
Expand Down Expand Up @@ -149,7 +145,7 @@ export class DialogRef<R = unknown, C = unknown> {

/** Whether the dialog is allowed to close. */
private _canClose(result?: R): boolean {
const config = this.config as DialogConfig<unknown, unknown, BasePortalOutlet>;
const config = this.config as DialogConfig<unknown, unknown, DialogContainer>;

return (
!!this.containerInstance &&
Expand Down
34 changes: 21 additions & 13 deletions src/cdk/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
signal,
} from '@angular/core';
import {Observable, Subject, defer} from 'rxjs';
import {startWith} from 'rxjs/operators';
import {startWith, take} from 'rxjs/operators';
import {_IdGenerator} from '../a11y';
import {Direction, Directionality} from '../bidi';
import {
Expand All @@ -30,8 +30,8 @@ import {
OverlayContainer,
OverlayRef,
} from '../overlay';
import {BasePortalOutlet, ComponentPortal, TemplatePortal} from '../portal';
import {DialogConfig} from './dialog-config';
import {ComponentPortal, TemplatePortal} from '../portal';
import {DialogConfig, DialogContainer} from './dialog-config';
import {DialogRef} from './dialog-ref';

import {CdkDialogContainer} from './dialog-container';
Expand Down Expand Up @@ -141,14 +141,24 @@ export class Dialog implements OnDestroy {
const dialogRef = new DialogRef(overlayRef, config);
const dialogContainer = this._attachContainer(overlayRef, dialogRef, config);

(dialogRef as {containerInstance: BasePortalOutlet}).containerInstance = dialogContainer;
this._attachDialogContent(componentOrTemplateRef, dialogRef, dialogContainer, config);
(dialogRef as {containerInstance: DialogContainer}).containerInstance = dialogContainer;

// If this is the first dialog that we're opening, hide all the non-overlay content.
if (!this.openDialogs.length) {
this._hideNonDialogContentFromAssistiveTechnology();
// Resolve this ahead of time, because some internal apps
// mock it out and depend on it being synchronous.
const overlayContainer = this._overlayContainer.getContainerElement();

if (dialogContainer._focusTrapped) {
dialogContainer._focusTrapped.pipe(take(1)).subscribe(() => {
this._hideNonDialogContentFromAssistiveTechnology(overlayContainer);
});
} else {
this._hideNonDialogContentFromAssistiveTechnology(overlayContainer);
}
}

this._attachDialogContent(componentOrTemplateRef, dialogRef, dialogContainer, config);
(this.openDialogs as DialogRef<R, C>[]).push(dialogRef);
dialogRef.closed.subscribe(() => this._removeOpenDialog(dialogRef, true));
this.afterOpened.next(dialogRef);
Expand Down Expand Up @@ -233,14 +243,14 @@ export class Dialog implements OnDestroy {
overlay: OverlayRef,
dialogRef: DialogRef<R, C>,
config: DialogConfig<D, DialogRef<R, C>>,
): BasePortalOutlet {
): DialogContainer {
const userInjector = config.injector || config.viewContainerRef?.injector;
const providers: StaticProvider[] = [
{provide: DialogConfig, useValue: config},
{provide: DialogRef, useValue: dialogRef},
{provide: OverlayRef, useValue: overlay},
];
let containerType: Type<BasePortalOutlet>;
let containerType: Type<DialogContainer>;

if (config.container) {
if (typeof config.container === 'function') {
Expand Down Expand Up @@ -274,7 +284,7 @@ export class Dialog implements OnDestroy {
private _attachDialogContent<R, D, C>(
componentOrTemplateRef: ComponentType<C> | TemplateRef<C>,
dialogRef: DialogRef<R, C>,
dialogContainer: BasePortalOutlet,
dialogContainer: DialogContainer,
config: DialogConfig<D, DialogRef<R, C>>,
) {
if (componentOrTemplateRef instanceof TemplateRef) {
Expand Down Expand Up @@ -316,7 +326,7 @@ export class Dialog implements OnDestroy {
private _createInjector<R, D, C>(
config: DialogConfig<D, DialogRef<R, C>>,
dialogRef: DialogRef<R, C>,
dialogContainer: BasePortalOutlet,
dialogContainer: DialogContainer,
fallbackInjector: Injector | undefined,
): Injector {
const userInjector = config.injector || config.viewContainerRef?.injector;
Expand Down Expand Up @@ -379,9 +389,7 @@ export class Dialog implements OnDestroy {
}

/** Hides all of the content that isn't an overlay from assistive technology. */
private _hideNonDialogContentFromAssistiveTechnology() {
const overlayContainer = this._overlayContainer.getContainerElement();

private _hideNonDialogContentFromAssistiveTechnology(overlayContainer: HTMLElement) {
// Ensure that the overlay container is attached to the DOM.
if (overlayContainer.parentElement) {
const siblings = overlayContainer.parentElement.children;
Expand Down
Loading