From c880ef5d4747c9e9212c104f5569bcdd1612e831 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 4 Jun 2024 17:30:23 -0700 Subject: [PATCH] fix(material/dialog): Make autofocus work with animations disabled --- src/cdk/dialog/dialog-container.ts | 39 +++++++++++++++++++++---- src/material/dialog/dialog-container.ts | 8 ----- src/material/dialog/dialog.spec.ts | 11 ++++--- 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/cdk/dialog/dialog-container.ts b/src/cdk/dialog/dialog-container.ts index 964bca1e4d01..aeb1c9a742ba 100644 --- a/src/cdk/dialog/dialog-container.ts +++ b/src/cdk/dialog/dialog-container.ts @@ -31,11 +31,13 @@ import { ElementRef, EmbeddedViewRef, Inject, + Injector, NgZone, OnDestroy, Optional, ViewChild, ViewEncapsulation, + afterNextRender, inject, } from '@angular/core'; import {DialogConfig} from './dialog-config'; @@ -102,6 +104,10 @@ export class CdkDialogContainer protected readonly _changeDetectorRef = inject(ChangeDetectorRef); + private _injector = inject(Injector); + + private _isDestroyed = false; + constructor( protected _elementRef: ElementRef, protected _focusTrapFactory: FocusTrapFactory, @@ -150,6 +156,7 @@ export class CdkDialogContainer } ngOnDestroy() { + this._isDestroyed = true; this._restoreFocus(); } @@ -246,13 +253,18 @@ export class CdkDialogContainer * cannot be moved then focus will go to the dialog container. */ protected _trapFocus() { + if (this._isDestroyed) { + return; + } + const element = this._elementRef.nativeElement; // If were to attempt to focus immediately, then the content of the dialog would not yet be // ready in instances where change detection has to run first. To deal with this, we simply // wait for the microtask queue to be empty when setting focus when autoFocus isn't set to // dialog. If the element inside the dialog can't be focused, then the container is focused // so the user can't tab into other elements behind it. - switch (this._config.autoFocus) { + const autoFocus = this._config.autoFocus; + switch (autoFocus) { case false: case 'dialog': // Ensure that focus is on the dialog container. It's possible that a different @@ -260,9 +272,14 @@ export class CdkDialogContainer // https://github.com/angular/components/issues/16215. Note that we only want to do this // if the focus isn't inside the dialog already, because it's possible that the consumer // turned off `autoFocus` in order to move focus themselves. - if (!this._containsFocus()) { - element.focus(); - } + afterNextRender( + () => { + if (!this._containsFocus()) { + element.focus(); + } + }, + {injector: this._injector}, + ); break; case true: case 'first-tabbable': @@ -275,10 +292,20 @@ export class CdkDialogContainer }); break; case 'first-heading': - this._focusByCssSelector('h1, h2, h3, h4, h5, h6, [role="heading"]'); + afterNextRender( + () => { + this._focusByCssSelector('h1, h2, h3, h4, h5, h6, [role="heading"]'); + }, + {injector: this._injector}, + ); break; default: - this._focusByCssSelector(this._config.autoFocus!); + afterNextRender( + () => { + this._focusByCssSelector(autoFocus!); + }, + {injector: this._injector}, + ); break; } } diff --git a/src/material/dialog/dialog-container.ts b/src/material/dialog/dialog-container.ts index b8d37b98c038..5b4ef223e287 100644 --- a/src/material/dialog/dialog-container.ts +++ b/src/material/dialog/dialog-container.ts @@ -94,8 +94,6 @@ export class MatDialogContainer extends CdkDialogContainer impl /** Current timer for dialog animations. */ private _animationTimer: ReturnType | null = null; - private _isDestroyed = false; - constructor( elementRef: ElementRef, focusTrapFactory: FocusTrapFactory, @@ -264,10 +262,6 @@ export class MatDialogContainer extends CdkDialogContainer impl * be called by sub-classes that use different animation implementations. */ protected _openAnimationDone(totalTime: number) { - if (this._isDestroyed) { - return; - } - if (this._config.delayFocusTrap) { this._trapFocus(); } @@ -281,8 +275,6 @@ export class MatDialogContainer extends CdkDialogContainer impl if (this._animationTimer !== null) { clearTimeout(this._animationTimer); } - - this._isDestroyed = true; } override attachComponentPortal(portal: ComponentPortal): ComponentRef { diff --git a/src/material/dialog/dialog.spec.ts b/src/material/dialog/dialog.spec.ts index 2ac6b19d431e..d5474b4e26d1 100644 --- a/src/material/dialog/dialog.spec.ts +++ b/src/material/dialog/dialog.spec.ts @@ -30,8 +30,8 @@ import { ViewEncapsulation, createNgModuleRef, forwardRef, - signal, provideZoneChangeDetection, + signal, } from '@angular/core'; import { ComponentFixture, @@ -1041,7 +1041,8 @@ describe('MDC-based MatDialog', () => { }); viewContainerFixture.detectChanges(); - flushMicrotasks(); + flush(); + viewContainerFixture.detectChanges(); let backdrop = overlayContainerElement.querySelector( '.cdk-overlay-backdrop', @@ -1209,7 +1210,8 @@ describe('MDC-based MatDialog', () => { }); viewContainerFixture.detectChanges(); - flushMicrotasks(); + flush(); + viewContainerFixture.detectChanges(); let container = overlayContainerElement.querySelector( '.mat-mdc-dialog-container', @@ -1245,7 +1247,8 @@ describe('MDC-based MatDialog', () => { }); viewContainerFixture.detectChanges(); - flushMicrotasks(); + flush(); + viewContainerFixture.detectChanges(); let firstParagraph = overlayContainerElement.querySelector( 'p[tabindex="-1"]',