diff --git a/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.spec.ts b/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.spec.ts index eda84265e90..22eb12e3d05 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.spec.ts @@ -84,11 +84,12 @@ describe("VaultListItemsContainerComponent", () => { { provide: CipherService, useValue: mock() }, { provide: Router, useValue: { navigate: jest.fn() } }, { provide: PlatformUtilsService, useValue: { getAutofillKeyboardShortcut: () => "" } }, - { provide: DialogService, useValue: mock() }, { provide: PasswordRepromptService, useValue: mock() }, ], schemas: [CUSTOM_ELEMENTS_SCHEMA], - }).compileComponents(); + }) + .overrideProvider(DialogService, { useValue: mock() }) + .compileComponents(); fixture = TestBed.createComponent(VaultListItemsContainerComponent); component = fixture.componentInstance; diff --git a/apps/browser/src/vault/popup/settings/archive.component.spec.ts b/apps/browser/src/vault/popup/settings/archive.component.spec.ts index bbb61b57a84..810ab85c4c6 100644 --- a/apps/browser/src/vault/popup/settings/archive.component.spec.ts +++ b/apps/browser/src/vault/popup/settings/archive.component.spec.ts @@ -62,7 +62,6 @@ describe("ArchiveComponent", () => { useValue: { hasOrganizations, organizations$: () => of([]) }, }, { provide: CollectionService, useValue: { decryptedCollections$ } }, - { provide: DialogService, useValue: mock() }, { provide: CipherService, useValue: mock() }, { provide: CipherArchiveService, @@ -99,7 +98,9 @@ describe("ArchiveComponent", () => { }, }, ], - }).compileComponents(); + }) + .overrideProvider(DialogService, { useValue: mock() }) + .compileComponents(); fixture = TestBed.createComponent(ArchiveComponent); component = fixture.componentInstance; diff --git a/apps/desktop/src/autofill/modal/credentials/fido2-vault.component.spec.ts b/apps/desktop/src/autofill/modal/credentials/fido2-vault.component.spec.ts index 70ef4461f6a..2a5da1b766b 100644 --- a/apps/desktop/src/autofill/modal/credentials/fido2-vault.component.spec.ts +++ b/apps/desktop/src/autofill/modal/credentials/fido2-vault.component.spec.ts @@ -10,6 +10,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { DialogService } from "@bitwarden/components"; import { PasswordRepromptService } from "@bitwarden/vault"; import { DesktopSettingsService } from "../../../platform/services/desktop-settings.service"; @@ -65,7 +66,9 @@ describe("Fido2VaultComponent", () => { { provide: I18nService, useValue: mockI18nService }, ], schemas: [NO_ERRORS_SCHEMA], - }).compileComponents(); + }) + .overrideProvider(DialogService, { useValue: mock() }) + .compileComponents(); fixture = TestBed.createComponent(Fido2VaultComponent); component = fixture.componentInstance; 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 3409fa9b8d3..0293245c733 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 @@ -86,7 +86,6 @@ describe("VaultItemDialogComponent", () => { { provide: I18nService, useValue: { t: (key: string) => key } }, { provide: DIALOG_DATA, useValue: { ...baseParams } }, { provide: DialogRef, useValue: {} }, - { provide: DialogService, useValue: {} }, { provide: ToastService, useValue: { @@ -173,7 +172,9 @@ describe("VaultItemDialogComponent", () => { { provide: SyncService, useValue: {} }, { provide: CipherRiskService, useValue: {} }, ], - }).compileComponents(); + }) + .overrideProvider(DialogService, { useValue: {} }) + .compileComponents(); fixture = TestBed.createComponent(TestVaultItemDialogComponent); component = fixture.componentInstance; diff --git a/apps/web/src/app/vault/individual-vault/add-edit-v2.component.spec.ts b/apps/web/src/app/vault/individual-vault/add-edit-v2.component.spec.ts index 3eecd4ee549..1f3f285d153 100644 --- a/apps/web/src/app/vault/individual-vault/add-edit-v2.component.spec.ts +++ b/apps/web/src/app/vault/individual-vault/add-edit-v2.component.spec.ts @@ -87,7 +87,6 @@ describe("AddEditComponentV2", () => { { provide: DIALOG_DATA, useValue: mockParams }, { provide: DialogRef, useValue: dialogRef }, { provide: I18nService, useValue: { t: jest.fn().mockReturnValue("login") } }, - { provide: DialogService, useValue: dialogService }, { provide: CipherService, useValue: cipherService }, { provide: MessagingService, useValue: messagingService }, { provide: OrganizationService, useValue: organizationService }, @@ -105,7 +104,9 @@ describe("AddEditComponentV2", () => { }, { provide: AccountService, useValue: accountService }, ], - }).compileComponents(); + }) + .overrideProvider(DialogService, { useValue: dialogService }) + .compileComponents(); fixture = TestBed.createComponent(AddEditComponentV2); component = fixture.componentInstance; diff --git a/libs/components/src/dialog/dialog.service.spec.ts b/libs/components/src/dialog/dialog.service.spec.ts new file mode 100644 index 00000000000..b4c40c066b3 --- /dev/null +++ b/libs/components/src/dialog/dialog.service.spec.ts @@ -0,0 +1,115 @@ +import { Dialog as CdkDialog } from "@angular/cdk/dialog"; +import { ChangeDetectionStrategy, Component } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { provideRouter } from "@angular/router"; +import { RouterTestingHarness } from "@angular/router/testing"; +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject } from "rxjs"; + +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; + +import { DialogService } from "./dialog.service"; +import { DrawerService } from "./drawer.service"; + +@Component({ + selector: "test-drawer", + template: "", + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class TestDrawerComponent {} + +@Component({ + selector: "test-initial-route", + template: "

Initial Route

", + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class InitialRouteComponent {} + +@Component({ + selector: "test-other-route", + template: "

Other Route

", + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class OtherRouteComponent {} + +describe("DialogService", () => { + let service: DialogService; + let drawerService: MockProxy; + let cdkDialog: MockProxy; + let routerHarness: RouterTestingHarness; + let authStatus$: BehaviorSubject; + + beforeEach(async () => { + drawerService = mock(); + cdkDialog = mock(); + authStatus$ = new BehaviorSubject(AuthenticationStatus.Unlocked); + + TestBed.configureTestingModule({ + providers: [ + DialogService, + { provide: DrawerService, useValue: drawerService }, + { provide: CdkDialog, useValue: cdkDialog }, + { + provide: AuthService, + useValue: { + getAuthStatus: () => authStatus$, + }, + }, + provideRouter([ + { path: "", component: InitialRouteComponent }, + { path: "other-route", component: OtherRouteComponent }, + { path: "another-route", component: OtherRouteComponent }, + ]), + ], + }); + + routerHarness = await RouterTestingHarness.create(); + // Navigate to the initial route to set up the router state + await routerHarness.navigateByUrl("/"); + + service = TestBed.inject(DialogService); + }); + + describe("close drawer on navigation", () => { + it("closes the drawer when navigating to a different route with closeOnNavigation enabled", async () => { + service.openDrawer(TestDrawerComponent, { closeOnNavigation: true }); + + await routerHarness.navigateByUrl("/other-route"); + + expect(drawerService.close).toHaveBeenCalled(); + }); + + it("does not close the drawer when navigating if closeOnNavigation is disabled", async () => { + service.openDrawer(TestDrawerComponent, { closeOnNavigation: false }); + + await routerHarness.navigateByUrl("/other-route"); + + expect(drawerService.close).not.toHaveBeenCalled(); + }); + + it("does not close the drawer when only query params change", async () => { + service.openDrawer(TestDrawerComponent, { closeOnNavigation: true }); + + await routerHarness.navigateByUrl("/?foo=bar"); + + expect(drawerService.close).not.toHaveBeenCalled(); + }); + + it("closes the drawer when the path changes but query params remain", async () => { + service.openDrawer(TestDrawerComponent, { closeOnNavigation: true }); + + await routerHarness.navigateByUrl("/other-route?foo=bar"); + + expect(drawerService.close).toHaveBeenCalled(); + }); + + it("does not close the drawer by default when closeOnNavigation is not specified", async () => { + service.openDrawer(TestDrawerComponent); + + await routerHarness.navigateByUrl("/other-route"); + + expect(drawerService.close).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/libs/components/src/dialog/dialog.service.ts b/libs/components/src/dialog/dialog.service.ts index d4cff75eddd..09aac449d6d 100644 --- a/libs/components/src/dialog/dialog.service.ts +++ b/libs/components/src/dialog/dialog.service.ts @@ -10,7 +10,17 @@ import { ComponentPortal, Portal } from "@angular/cdk/portal"; import { Injectable, Injector, TemplateRef, inject } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router } from "@angular/router"; -import { filter, firstValueFrom, map, Observable, Subject, switchMap, take } from "rxjs"; +import { + distinctUntilChanged, + filter, + firstValueFrom, + map, + Observable, + startWith, + Subject, + switchMap, + take, +} from "rxjs"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -61,7 +71,14 @@ export abstract class DialogRef implements Pick< export type DialogConfig = Pick< CdkDialogConfig, - "data" | "disableClose" | "ariaModal" | "positionStrategy" | "height" | "width" | "restoreFocus" + | "data" + | "disableClose" + | "ariaModal" + | "positionStrategy" + | "height" + | "width" + | "restoreFocus" + | "closeOnNavigation" >; /** @@ -136,7 +153,11 @@ class DrawerDialogRef implements DialogRef { /** The portal containing the drawer */ portal?: Portal; - constructor(private drawerService: DrawerService) {} + constructor( + private drawerService: DrawerService, + /** Whether to close this drawer when navigating to a different route */ + readonly closeOnNavigation = false, + ) {} close(result?: R, _options?: DialogCloseOptions): void { if (this.disableClose) { @@ -187,7 +208,7 @@ export class DialogService { private dialog = inject(CdkDialog); private drawerService = inject(DrawerService); private injector = inject(Injector); - private router = inject(Router, { optional: true }); + private router = inject(Router); private authService = inject(AuthService, { optional: true }); private backDropClasses = ["tw-fixed", "tw-bg-black", "tw-bg-opacity-30", "tw-inset-0"]; @@ -210,6 +231,24 @@ export class DialogService { ) .subscribe(() => this.closeAll()); } + + /** + * Close the active drawer on route navigation if configured. + * Note: CDK dialogs have their own `closeOnNavigation` config option, + * but drawers use a custom implementation that requires manual cleanup. + */ + if (this.router) { + this.router.events + .pipe( + filter((event): event is NavigationEnd => event instanceof NavigationEnd), + map((event) => event.urlAfterRedirects.split("?")[0]), + startWith(this.router.url.split("?")[0]), + distinctUntilChanged(), + filter(() => this.activeDrawer?.closeOnNavigation === true), + takeUntilDestroyed(), + ) + .subscribe(() => this.closeDrawer()); + } } open( @@ -235,6 +274,7 @@ export class DialogService { backdropClass: this.backDropClasses, scrollStrategy: this.defaultScrollStrategy, positionStrategy: config?.positionStrategy ?? new ResponsivePositionStrategy(), + closeOnNavigation: config?.closeOnNavigation, injector, ...config, }; @@ -258,7 +298,7 @@ export class DialogService { * This is also circular. When creating the DrawerDialogRef, we do not yet have a portal instance to provide to the injector. * Similar to `this.open`, we get around this with mutability. */ - this.activeDrawer = new DrawerDialogRef(this.drawerService); + this.activeDrawer = new DrawerDialogRef(this.drawerService, config?.closeOnNavigation ?? false); const portal = new ComponentPortal( component, null, diff --git a/libs/components/src/dialog/dialogs.mdx b/libs/components/src/dialog/dialogs.mdx index 2b8afb06783..c2e674273ca 100644 --- a/libs/components/src/dialog/dialogs.mdx +++ b/libs/components/src/dialog/dialogs.mdx @@ -28,7 +28,16 @@ For non-blocking, supplementary content, open dialogs as a ### Closing Drawers on Navigation When using drawers, you may want to close them automatically when the user navigates to another page -to prevent the drawer from persisting across route changes. To implement this functionality: +to prevent the drawer from persisting across route changes. In most cases you can achieve this by +using the `closeOnNavigation` option when opening the drawer. + +```ts +this.dialogService.open(MyDialogComponent, { closeOnNavigation: true }); +``` + +In some cases you may want more control of when the drawer is closed, such as if you have nested +routes and want the drawer to persist across some route changes but not others. In this case, you +can manually close the drawer in `ngOnDestroy`. To do this: 1. Store a reference to the dialog when opening it 2. Implement `OnDestroy` and close the dialog in `ngOnDestroy` @@ -52,8 +61,6 @@ export class MyComponent implements OnDestroy { } ``` -This ensures drawers are closed when the component is destroyed during navigation. - ## Placement Dialogs should be centered vertically and horizontally on screen. Dialogs height should expand to