mirror of
https://github.com/bitwarden/browser
synced 2026-02-28 10:33:31 +00:00
[CL-1051] Generic closeOnNavigation for drawers (#19031)
Exposes closeOnNavigation from cdk config and implements a drawer close solution on navigation. More complex scenarios may still require custom handling. DialogService is referenced in imported components in some tests meaning we need to use overrideProvider rather than providers.
This commit is contained in:
115
libs/components/src/dialog/dialog.service.spec.ts
Normal file
115
libs/components/src/dialog/dialog.service.spec.ts
Normal file
@@ -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: "<h1>Initial Route</h1>",
|
||||
changeDetection: ChangeDetectionStrategy.OnPush,
|
||||
})
|
||||
class InitialRouteComponent {}
|
||||
|
||||
@Component({
|
||||
selector: "test-other-route",
|
||||
template: "<h1>Other Route</h1>",
|
||||
changeDetection: ChangeDetectionStrategy.OnPush,
|
||||
})
|
||||
class OtherRouteComponent {}
|
||||
|
||||
describe("DialogService", () => {
|
||||
let service: DialogService;
|
||||
let drawerService: MockProxy<DrawerService>;
|
||||
let cdkDialog: MockProxy<CdkDialog>;
|
||||
let routerHarness: RouterTestingHarness;
|
||||
let authStatus$: BehaviorSubject<AuthenticationStatus>;
|
||||
|
||||
beforeEach(async () => {
|
||||
drawerService = mock<DrawerService>();
|
||||
cdkDialog = mock<CdkDialog>();
|
||||
authStatus$ = new BehaviorSubject<AuthenticationStatus>(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();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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<R = unknown, C = unknown> implements Pick<
|
||||
|
||||
export type DialogConfig<D = unknown, R = unknown> = Pick<
|
||||
CdkDialogConfig<D, R>,
|
||||
"data" | "disableClose" | "ariaModal" | "positionStrategy" | "height" | "width" | "restoreFocus"
|
||||
| "data"
|
||||
| "disableClose"
|
||||
| "ariaModal"
|
||||
| "positionStrategy"
|
||||
| "height"
|
||||
| "width"
|
||||
| "restoreFocus"
|
||||
| "closeOnNavigation"
|
||||
>;
|
||||
|
||||
/**
|
||||
@@ -136,7 +153,11 @@ class DrawerDialogRef<R = unknown, C = unknown> implements DialogRef<R, C> {
|
||||
/** The portal containing the drawer */
|
||||
portal?: Portal<unknown>;
|
||||
|
||||
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<R = unknown, D = unknown, C = unknown>(
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user