1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-12 06:23:38 +00:00

[PM-31384] Prevent dialog header from stealing focus from autofocus inputs (#18657)

This commit is contained in:
Vicki League
2026-02-04 10:26:38 -05:00
committed by GitHub
parent 5bceadd29b
commit 97c65b3c72
4 changed files with 55 additions and 23 deletions

View File

@@ -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");

View File

@@ -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([], {

View File

@@ -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<ElementRef<HTMLHeadingElement>>("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));
}
}
}

View File

@@ -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]",