From 97c65b3c721253549e588cb48886800560836c23 Mon Sep 17 00:00:00 2001 From: Vicki League Date: Wed, 4 Feb 2026 10:26:38 -0500 Subject: [PATCH] [PM-31384] Prevent dialog header from stealing focus from autofocus inputs (#18657) --- .../vault-item-dialog.component.spec.ts | 2 +- .../vault-item-dialog.component.ts | 2 +- .../src/dialog/dialog/dialog.component.ts | 72 +++++++++++++------ .../src/input/autofocus.directive.ts | 2 + 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts index 276c0c2e6a3..08931c68900 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts @@ -363,7 +363,7 @@ describe("VaultItemDialogComponent", () => { }); it("refocuses the dialog header", async () => { - const focusOnHeaderSpy = jest.spyOn(component["dialogComponent"](), "focusOnHeader"); + const focusOnHeaderSpy = jest.spyOn(component["dialogComponent"](), "handleAutofocus"); await component["changeMode"]("view"); diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts index ef861b7cab3..df73aacfdde 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts @@ -692,7 +692,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { this.dialogContent().nativeElement.parentElement.scrollTop = 0; // Refocus on title element, the built-in focus management of the dialog only works for the initial open. - this.dialogComponent().focusOnHeader(); + this.dialogComponent().handleAutofocus(); // Update the URL query params to reflect the new mode. await this.router.navigate([], { diff --git a/libs/components/src/dialog/dialog/dialog.component.ts b/libs/components/src/dialog/dialog/dialog.component.ts index 2ce19a9f9e0..63fbb69399d 100644 --- a/libs/components/src/dialog/dialog/dialog.component.ts +++ b/libs/components/src/dialog/dialog/dialog.component.ts @@ -12,9 +12,10 @@ import { computed, signal, AfterViewInit, + NgZone, } from "@angular/core"; import { toObservable } from "@angular/core/rxjs-interop"; -import { combineLatest, switchMap } from "rxjs"; +import { combineLatest, firstValueFrom, switchMap } from "rxjs"; import { I18nPipe } from "@bitwarden/ui-common"; @@ -65,6 +66,9 @@ const drawerSizeToWidth = { }) export class DialogComponent implements AfterViewInit { private readonly destroyRef = inject(DestroyRef); + private readonly ngZone = inject(NgZone); + private readonly el = inject(ElementRef); + private readonly dialogHeader = viewChild.required>("dialogHeader"); private readonly scrollableBody = viewChild.required(CdkScrollable); @@ -144,10 +148,6 @@ export class DialogComponent implements AfterViewInit { return [...baseClasses, this.width(), ...sizeClasses, ...animationClasses]; }); - ngAfterViewInit() { - this.focusOnHeader(); - } - handleEsc(event: Event) { if (!this.dialogRef?.disableClose) { this.dialogRef?.close(); @@ -159,24 +159,54 @@ export class DialogComponent implements AfterViewInit { this.animationCompleted.set(true); } - /** - * Moves focus to the dialog header element. - * This is done automatically when the dialog is opened but can be called manually - * when the contents of the dialog change and focus should be reset. - */ - focusOnHeader(): void { + async ngAfterViewInit() { /** - * Wait a tick for any focus management to occur on the trigger element before moving focus to - * the dialog header. We choose the dialog header because it is always present, unlike possible - * interactive elements. - * - * We are doing this manually instead of using `cdkTrapFocusAutoCapture` and `cdkFocusInitial` - * because we need this delay behavior. + * Wait for the zone to stabilize before performing any focus behaviors. This ensures that all + * child elements are rendered and stable. */ - const headerFocusTimeout = setTimeout(() => { - this.dialogHeader().nativeElement.focus(); - }, 0); + if (this.ngZone.isStable) { + this.handleAutofocus(); + } else { + await firstValueFrom(this.ngZone.onStable); + this.handleAutofocus(); + } + } - this.destroyRef.onDestroy(() => clearTimeout(headerFocusTimeout)); + /** + * Ensure that the user's focus is in the dialog by autofocusing the appropriate element. + * + * If there is a descendant of the dialog with the AutofocusDirective applied, we defer to that. + * If not, we want to fallback to a default behavior of focusing the dialog's header element. We + * choose the dialog header as the default fallback for dialog focus because it is always present, + * unlike possible interactive elements. + */ + handleAutofocus() { + /** + * Angular's contentChildren query cannot see into the internal templates of child components. + * We need to use a regular DOM query instead to see if there are descendants using the + * AutofocusDirective. + */ + const dialogRef = this.el.nativeElement; + // Must match selectors of AutofocusDirective + const autofocusDescendants = dialogRef.querySelectorAll("[appAutofocus], [bitAutofocus]"); + const hasAutofocusDescendants = autofocusDescendants.length > 0; + + if (!hasAutofocusDescendants) { + /** + * Wait a tick for any focus management to occur on the trigger element before moving focus + * to the dialog header. + * + * We are doing this manually instead of using Angular's built-in focus management + * directives (`cdkTrapFocusAutoCapture` and `cdkFocusInitial`) because we need this delay + * behavior. + * + * And yes, we need the timeout even though we are already waiting for ngZone to stabilize. + */ + const headerFocusTimeout = setTimeout(() => { + this.dialogHeader().nativeElement.focus(); + }, 0); + + this.destroyRef.onDestroy(() => clearTimeout(headerFocusTimeout)); + } } } diff --git a/libs/components/src/input/autofocus.directive.ts b/libs/components/src/input/autofocus.directive.ts index a4791a51f01..bffac8eb757 100644 --- a/libs/components/src/input/autofocus.directive.ts +++ b/libs/components/src/input/autofocus.directive.ts @@ -22,6 +22,8 @@ import { FocusableElement } from "../shared/focusable-element"; * * If the component provides the `FocusableElement` interface, the `focus` * method will be called. Otherwise, the native element will be focused. + * + * If selector changes, `dialog.component.ts` must also be updated */ @Directive({ selector: "[appAutofocus], [bitAutofocus]",