From 93ce914f79eb77b5fc0dc7d7efb331126a8aace9 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Fri, 30 Jan 2026 10:10:26 -0600 Subject: [PATCH] [PM-30638] Cipher Add/Edit dialog focus (#18536) * allow exporting of the DialogComponent * focus on dialog header when switching modes * update to view child fixmes --- .../vault-item-dialog.component.spec.ts | 74 ++++++++++++++++++- .../vault-item-dialog.component.ts | 23 +++--- .../src/dialog/dialog/dialog.component.ts | 31 +++++--- libs/components/src/dialog/index.ts | 1 + 4 files changed, 106 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 9a048b7a8b3..276c0c2e6a3 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 @@ -104,7 +104,7 @@ describe("VaultItemDialogComponent", () => { getFeatureFlag$: () => of(false), }, }, - { provide: Router, useValue: {} }, + { provide: Router, useValue: { navigate: jest.fn() } }, { provide: ActivatedRoute, useValue: {} }, { provide: BillingAccountProfileStateService, @@ -356,4 +356,76 @@ describe("VaultItemDialogComponent", () => { }); }); }); + + describe("changeMode", () => { + beforeEach(() => { + component.setTestCipher({ type: CipherType.Login, id: "cipher-id" }); + }); + + it("refocuses the dialog header", async () => { + const focusOnHeaderSpy = jest.spyOn(component["dialogComponent"](), "focusOnHeader"); + + await component["changeMode"]("view"); + + expect(focusOnHeaderSpy).toHaveBeenCalled(); + }); + + describe("to view", () => { + beforeEach(() => { + component.setTestParams({ mode: "form" }); + fixture.detectChanges(); + }); + + it("sets mode to view", async () => { + await component["changeMode"]("view"); + + expect(component["params"].mode).toBe("view"); + }); + + it("updates the url", async () => { + const router = TestBed.inject(Router); + + await component["changeMode"]("view"); + + expect(router.navigate).toHaveBeenCalledWith([], { + queryParams: { action: "view", itemId: "cipher-id" }, + queryParamsHandling: "merge", + replaceUrl: true, + }); + }); + }); + + describe("to form", () => { + const waitForFormReady = async () => { + const changeModePromise = component["changeMode"]("form"); + + expect(component["loadForm"]).toBe(true); + + component["onFormReady"](); + await changeModePromise; + }; + + beforeEach(() => { + component.setTestParams({ mode: "view" }); + fixture.detectChanges(); + }); + + it("waits for form to be ready when switching to form mode", async () => { + await waitForFormReady(); + + expect(component["params"].mode).toBe("form"); + }); + + it("updates the url", async () => { + const router = TestBed.inject(Router); + await waitForFormReady(); + + expect(router.navigate).toHaveBeenCalledWith([], { + queryParams: { action: "edit", itemId: "cipher-id" }, + queryParamsHandling: "merge", + replaceUrl: true, + }); + }); + }); + }); }); 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 5d5e319c8af..90452ba573a 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 @@ -8,7 +8,7 @@ import { Inject, OnDestroy, OnInit, - ViewChild, + viewChild, } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { Router } from "@angular/router"; @@ -50,6 +50,7 @@ import { ItemModule, ToastService, CenterPositionStrategy, + DialogComponent, } from "@bitwarden/components"; import { AttachmentDialogCloseResult, @@ -163,14 +164,11 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { * Reference to the dialog content element. Used to scroll to the top of the dialog when switching modes. * @protected */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @ViewChild("dialogContent") - protected dialogContent: ElementRef; + protected readonly dialogContent = viewChild.required>("dialogContent"); - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @ViewChild(CipherFormComponent) cipherFormComponent!: CipherFormComponent; + private readonly cipherFormComponent = viewChild.required(CipherFormComponent); + + private readonly dialogComponent = viewChild(DialogComponent); /** * Tracks if the cipher was ever modified while the dialog was open. Used to ensure the dialog emits the correct result @@ -536,7 +534,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { updatedCipherView = await this.cipherService.decrypt(updatedCipher, activeUserId); } - this.cipherFormComponent.patchCipher((currentCipher) => { + this.cipherFormComponent().patchCipher((currentCipher) => { currentCipher.attachments = updatedCipherView.attachments; currentCipher.revisionDate = updatedCipherView.revisionDate; @@ -574,7 +572,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { return; } - this.cipherFormComponent.patchCipher((current) => { + this.cipherFormComponent().patchCipher((current) => { current.revisionDate = revisionDate; current.archivedDate = archivedDate; return current; @@ -691,7 +689,10 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { this.params.mode = mode; this.updateTitle(); // Scroll to the top of the dialog content when switching modes. - this.dialogContent.nativeElement.parentElement.scrollTop = 0; + 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(); // 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 f9073da2217..2ce19a9f9e0 100644 --- a/libs/components/src/dialog/dialog/dialog.component.ts +++ b/libs/components/src/dialog/dialog/dialog.component.ts @@ -145,6 +145,26 @@ export class DialogComponent implements AfterViewInit { }); ngAfterViewInit() { + this.focusOnHeader(); + } + + handleEsc(event: Event) { + if (!this.dialogRef?.disableClose) { + this.dialogRef?.close(); + event.stopPropagation(); + } + } + + onAnimationEnd() { + 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 { /** * 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 @@ -159,15 +179,4 @@ export class DialogComponent implements AfterViewInit { this.destroyRef.onDestroy(() => clearTimeout(headerFocusTimeout)); } - - handleEsc(event: Event) { - if (!this.dialogRef?.disableClose) { - this.dialogRef?.close(); - event.stopPropagation(); - } - } - - onAnimationEnd() { - this.animationCompleted.set(true); - } } diff --git a/libs/components/src/dialog/index.ts b/libs/components/src/dialog/index.ts index fb4c2721b81..ce41f7957f6 100644 --- a/libs/components/src/dialog/index.ts +++ b/libs/components/src/dialog/index.ts @@ -2,3 +2,4 @@ export * from "./dialog.module"; export * from "./simple-dialog/types"; export * from "./dialog.service"; export { DIALOG_DATA } from "@angular/cdk/dialog"; +export { DialogComponent } from "./dialog/dialog.component";