[PM-23375] Replace drawer with dialog (#17176)

bitwarden_license/bit-common/src/dirt/reports/risk-insights/models/drawer-models.types.ts

@@ -1,14 +1,15 @@
 import { MemberDetails } from "./report-models";
 
 // -------------------- Drawer and UI Models --------------------
-// FIXME: update to use a const object instead of a typescript enum
-// eslint-disable-next-line @bitwarden/platform/no-enums
-export enum DrawerType {
-  None = 0,
-  AppAtRiskMembers = 1,
-  OrgAtRiskMembers = 2,
-  OrgAtRiskApps = 3,
-}
+
+export const DrawerType = {
+  None: 0,
+  AppAtRiskMembers: 1,
+  OrgAtRiskMembers: 2,
+  OrgAtRiskApps: 3,
+} as const;
+
+export type DrawerType = (typeof DrawerType)[keyof typeof DrawerType];
 
 export type DrawerDetails = {
   open: boolean;

bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.html

@@ -109,112 +109,4 @@
       </div>
     }
   }
-
-  @if (dataService.drawerDetails$ | async; as drawerDetails) {
-    <bit-drawer style="width: 30%" [(open)]="isDrawerOpen" (openChange)="dataService.closeDrawer()">
-      <ng-container *ngIf="dataService.isActiveDrawerType(drawerTypes.OrgAtRiskMembers)">
-        <bit-drawer-header
-          title="{{ 'atRiskMembersWithCount' | i18n: drawerDetails.atRiskMemberDetails.length }}"
-        >
-        </bit-drawer-header>
-        <bit-drawer-body>
-          <span bitTypography="body1" class="tw-text-muted tw-text-sm">{{
-            (drawerDetails.atRiskMemberDetails.length > 0
-              ? "atRiskMembersDescription"
-              : "atRiskMembersDescriptionNone"
-            ) | i18n
-          }}</span>
-          <ng-container *ngIf="drawerDetails.atRiskMemberDetails.length > 0">
-            <button
-              bitLink
-              type="button"
-              class="tw-my-4 tw-font-bold tw-block"
-              (click)="downloadAtRiskMembers()"
-            >
-              <i class="bwi bwi-download tw-mr-1"></i>
-              {{ "downloadCSV" | i18n }}
-            </button>
-            <div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted">
-              <div bitTypography="body2" class="tw-text-sm tw-font-medium">
-                {{ "email" | i18n }}
-              </div>
-              <div bitTypography="body2" class="tw-text-sm tw-font-medium">
-                {{ "atRiskPasswords" | i18n }}
-              </div>
-            </div>
-            <ng-container *ngFor="let member of drawerDetails.atRiskMemberDetails">
-              <div class="tw-flex tw-justify-between tw-mt-2">
-                <div>{{ member.email }}</div>
-                <div>{{ member.atRiskPasswordCount }}</div>
-              </div>
-            </ng-container>
-          </ng-container>
-        </bit-drawer-body>
-      </ng-container>
-
-      @if (dataService.isActiveDrawerType(drawerTypes.AppAtRiskMembers)) {
-        <bit-drawer-header title="{{ drawerDetails.appAtRiskMembers.applicationName }}">
-        </bit-drawer-header>
-        <bit-drawer-body>
-          <div bitTypography="body1" class="tw-mb-2">
-            {{ "atRiskMembersWithCount" | i18n: drawerDetails.appAtRiskMembers.members.length }}
-          </div>
-          <div bitTypography="body1" class="tw-text-muted tw-text-sm tw-mb-2">
-            {{
-              (drawerDetails.appAtRiskMembers.members.length > 0
-                ? "atRiskMembersDescriptionWithApp"
-                : "atRiskMembersDescriptionWithAppNone"
-              ) | i18n: drawerDetails.appAtRiskMembers.applicationName
-            }}
-          </div>
-          <div class="tw-mt-1">
-            <ng-container *ngFor="let member of drawerDetails.appAtRiskMembers.members">
-              <div>{{ member.email }}</div>
-            </ng-container>
-          </div>
-        </bit-drawer-body>
-      }
-
-      @if (dataService.isActiveDrawerType(drawerTypes.OrgAtRiskApps)) {
-        <bit-drawer-header
-          title="{{ 'atRiskApplicationsWithCount' | i18n: drawerDetails.atRiskAppDetails.length }}"
-        >
-        </bit-drawer-header>
-
-        <bit-drawer-body>
-          <span bitTypography="body2" class="tw-text-muted tw-text-sm">{{
-            (drawerDetails.atRiskAppDetails.length > 0
-              ? "atRiskApplicationsDescription"
-              : "atRiskApplicationsDescriptionNone"
-            ) | i18n
-          }}</span>
-          <ng-container *ngIf="drawerDetails.atRiskAppDetails.length > 0">
-            <button
-              bitLink
-              type="button"
-              class="tw-my-4 tw-font-bold tw-block"
-              (click)="downloadAtRiskApplications()"
-            >
-              <i class="bwi bwi-download tw-mr-1"></i>
-              {{ "downloadCSV" | i18n }}
-            </button>
-            <div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted">
-              <div bitTypography="body2" class="tw-text-sm tw-font-medium">
-                {{ "application" | i18n }}
-              </div>
-              <div bitTypography="body2" class="tw-text-sm tw-font-medium">
-                {{ "atRiskPasswords" | i18n }}
-              </div>
-            </div>
-            <ng-container *ngFor="let app of drawerDetails.atRiskAppDetails">
-              <div class="tw-flex tw-justify-between tw-mt-2">
-                <div>{{ app.applicationName }}</div>
-                <div>{{ app.atRiskPasswordCount }}</div>
-              </div>
-            </ng-container>
-          </ng-container>
-        </bit-drawer-body>
-      }
-    </bit-drawer>
-  }
 </ng-container>

bitwarden_license/bit-web/src/app/dirt/access-intelligence/risk-insights.component.ts

@@ -1,10 +1,17 @@
 import { animate, style, transition, trigger } from "@angular/animations";
 import { CommonModule } from "@angular/common";
-import { Component, DestroyRef, OnDestroy, OnInit, inject } from "@angular/core";
+import {
+  Component,
+  DestroyRef,
+  OnDestroy,
+  OnInit,
+  inject,
+  ChangeDetectionStrategy,
+} from "@angular/core";
 import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
 import { ActivatedRoute, Router } from "@angular/router";
 import { combineLatest, EMPTY, firstValueFrom } from "rxjs";
-import { map, tap } from "rxjs/operators";
+import { distinctUntilChanged, map, tap } from "rxjs/operators";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import {
@@ -21,9 +28,8 @@ import { OrganizationId } from "@bitwarden/common/types/guid";
 import {
   AsyncActionsModule,
   ButtonModule,
-  DrawerBodyComponent,
-  DrawerComponent,
-  DrawerHeaderComponent,
+  DialogRef,
+  DialogService,
   TabsModule,
 } from "@bitwarden/components";
 import { ExportHelper } from "@bitwarden/vault-export-core";
@@ -36,11 +42,11 @@ import { CriticalApplicationsComponent } from "./critical-applications/critical-
 import { EmptyStateCardComponent } from "./empty-state-card.component";
 import { RiskInsightsTabType } from "./models/risk-insights.models";
 import { PageLoadingComponent } from "./shared/page-loading.component";
+import { RiskInsightsDrawerDialogComponent } from "./shared/risk-insights-drawer-dialog.component";
 import { ApplicationsLoadingComponent } from "./shared/risk-insights-loading.component";
 
-// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
-// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
 @Component({
+  changeDetection: ChangeDetectionStrategy.OnPush,
   templateUrl: "./risk-insights.component.html",
   imports: [
     AllApplicationsComponent,
@@ -52,9 +58,6 @@ import { ApplicationsLoadingComponent } from "./shared/risk-insights-loading.com
     JslibModule,
     HeaderModule,
     TabsModule,
-    DrawerComponent,
-    DrawerBodyComponent,
-    DrawerHeaderComponent,
     AllActivityComponent,
     ApplicationsLoadingComponent,
     PageLoadingComponent,
@@ -70,7 +73,6 @@ import { ApplicationsLoadingComponent } from "./shared/risk-insights-loading.com
 })
 export class RiskInsightsComponent implements OnInit, OnDestroy {
   private destroyRef = inject(DestroyRef);
-  private _isDrawerOpen: boolean = false;
   protected ReportStatusEnum = ReportStatus;
 
   tabIndex: RiskInsightsTabType = RiskInsightsTabType.AllApps;
@@ -94,6 +96,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy {
   protected emptyStateVideoSrc: string | null = "/videos/risk-insights-mark-as-critical.mp4";
 
   protected IMPORT_ICON = "bwi bwi-download";
+  protected currentDialogRef: DialogRef<unknown, RiskInsightsDrawerDialogComponent> | null = null;
 
   // TODO: See https://github.com/bitwarden/clients/pull/16832#discussion_r2474523235
 
@@ -103,6 +106,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy {
     private configService: ConfigService,
     protected dataService: RiskInsightsDataService,
     protected i18nService: I18nService,
+    protected dialogService: DialogService,
     private fileDownloadService: FileDownloadService,
     private logService: LogService,
   ) {
@@ -151,14 +155,32 @@ export class RiskInsightsComponent implements OnInit, OnDestroy {
 
     // Subscribe to drawer state changes
     this.dataService.drawerDetails$
-      .pipe(takeUntilDestroyed(this.destroyRef))
+      .pipe(
+        distinctUntilChanged(
+          (prev, curr) =>
+            prev.activeDrawerType === curr.activeDrawerType && prev.invokerId === curr.invokerId,
+        ),
+        takeUntilDestroyed(this.destroyRef),
+      )
       .subscribe((details) => {
-        this._isDrawerOpen = details.open;
+        if (details.activeDrawerType !== DrawerType.None) {
+          this.currentDialogRef = this.dialogService.openDrawer(RiskInsightsDrawerDialogComponent, {
+            data: details,
+          });
+        } else {
+          this.currentDialogRef?.close();
+        }
       });
+
+    // if any dialogs are open close it
+    // this happens when navigating between orgs
+    // or just navigating away from the page and back
+    this.currentDialogRef?.close();
   }
 
   ngOnDestroy(): void {
     this.dataService.destroy();
+    this.currentDialogRef?.close();
   }
 
   /**
@@ -179,35 +201,7 @@ export class RiskInsightsComponent implements OnInit, OnDestroy {
     });
 
     // close drawer when tabs are changed
-    this.dataService.closeDrawer();
-  }
-
-  // Get a list of drawer types
-  get drawerTypes(): typeof DrawerType {
-    return DrawerType;
-  }
-
-  /**
-   * Special case getter for syncing drawer state from service to component.
-   * This allows the template to use two-way binding while staying reactive.
-   */
-  get isDrawerOpen() {
-    return this._isDrawerOpen;
-  }
-
-  /**
-   * Special case setter for syncing drawer state from component to service.
-   * When the drawer component closes the drawer, this syncs the state back to the service.
-   */
-  set isDrawerOpen(value: boolean) {
-    if (this._isDrawerOpen !== value) {
-      this._isDrawerOpen = value;
-
-      // Close the drawer in the service if the drawer component closed the drawer
-      if (!value) {
-        this.dataService.closeDrawer();
-      }
-    }
+    this.currentDialogRef?.close();
   }
 
   // Empty state methods

bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.html

@@ -0,0 +1,98 @@
+@if (isActiveDrawerType(drawerTypes.OrgAtRiskMembers)) {
+  <bit-dialog dialogSize="large" disablePadding="false">
+    <ng-container bitDialogTitle>
+      <span>{{
+        "atRiskMembersWithCount" | i18n: drawerDetails.atRiskMemberDetails?.length ?? 0
+      }}</span>
+    </ng-container>
+    <ng-container bitDialogContent>
+      <span bitTypography="body1" class="tw-text-muted tw-text-sm">{{
+        (drawerDetails.atRiskMemberDetails?.length > 0
+          ? "atRiskMembersDescription"
+          : "atRiskMembersDescriptionNone"
+        ) | i18n
+      }}</span>
+
+      @if (drawerDetails.atRiskMemberDetails?.length > 0) {
+        <ng-container>
+          <div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted">
+            <div bitTypography="body2" class="tw-text-sm tw-font-bold">
+              {{ "email" | i18n }}
+            </div>
+            <div bitTypography="body2" class="tw-text-sm tw-font-bold">
+              {{ "atRiskPasswords" | i18n }}
+            </div>
+          </div>
+          @for (member of drawerDetails.atRiskMemberDetails; track member.email) {
+            <div class="tw-flex tw-justify-between tw-mt-2">
+              <div>{{ member.email }}</div>
+              <div>{{ member.atRiskPasswordCount }}</div>
+            </div>
+          }
+        </ng-container>
+      }
+    </ng-container>
+  </bit-dialog>
+}
+
+@if (isActiveDrawerType(drawerTypes.AppAtRiskMembers)) {
+  <bit-dialog dialogSize="large" disablePadding="false">
+    <ng-container bitDialogTitle>
+      <span>{{ drawerDetails.appAtRiskMembers?.applicationName }}</span>
+    </ng-container>
+    <ng-container bitDialogContent>
+      <div bitTypography="body1" class="tw-mb-2">
+        {{ "atRiskMembersWithCount" | i18n: drawerDetails.appAtRiskMembers?.members.length }}
+      </div>
+      <div bitTypography="body1" class="tw-text-muted tw-text-sm tw-mb-2">
+        {{
+          (drawerDetails.appAtRiskMembers?.members.length > 0
+            ? "atRiskMembersDescriptionWithApp"
+            : "atRiskMembersDescriptionWithAppNone"
+          ) | i18n: drawerDetails.appAtRiskMembers?.applicationName
+        }}
+      </div>
+      <div class="tw-mt-1">
+        @for (member of drawerDetails.appAtRiskMembers?.members; track $index) {
+          <div>{{ member.email }}</div>
+        }
+      </div>
+    </ng-container>
+  </bit-dialog>
+}
+
+@if (isActiveDrawerType(drawerTypes.OrgAtRiskApps)) {
+  <bit-dialog dialogSize="large" disablePadding="false">
+    <ng-container bitDialogTitle>
+      <span>{{
+        "atRiskApplicationsWithCount" | i18n: drawerDetails.atRiskAppDetails?.length ?? 0
+      }}</span>
+    </ng-container>
+    <ng-container bitDialogContent>
+      <span bitTypography="body1" class="tw-text-muted tw-text-sm">{{
+        (drawerDetails.atRiskAppDetails?.length > 0
+          ? "atRiskApplicationsDescription"
+          : "atRiskApplicationsDescriptionNone"
+        ) | i18n
+      }}</span>
+      @if (drawerDetails.atRiskAppDetails?.length > 0) {
+        <ng-container>
+          <div class="tw-flex tw-justify-between tw-mt-2 tw-text-muted">
+            <div bitTypography="body2" class="tw-text-sm tw-font-bold">
+              {{ "application" | i18n }}
+            </div>
+            <div bitTypography="body2" class="tw-text-sm tw-font-bold">
+              {{ "atRiskPasswords" | i18n }}
+            </div>
+          </div>
+          @for (app of drawerDetails.atRiskAppDetails; track app.applicationName) {
+            <div class="tw-flex tw-justify-between tw-mt-2">
+              <div>{{ app.applicationName }}</div>
+              <div>{{ app.atRiskPasswordCount }}</div>
+            </div>
+          }
+        </ng-container>
+      }
+    </ng-container>
+  </bit-dialog>
+}

bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.spec.ts

@@ -0,0 +1,96 @@
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { BrowserAnimationsModule } from "@angular/platform-browser/animations";
+import { mock } from "jest-mock-extended";
+
+import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights";
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { DIALOG_DATA } from "@bitwarden/components";
+import { I18nPipe } from "@bitwarden/ui-common";
+
+import { RiskInsightsDrawerDialogComponent } from "./risk-insights-drawer-dialog.component";
+
+beforeAll(() => {
+  // Mock element.animate for jsdom
+  // the animate function is not available in jsdom, so we provide a mock implementation
+  // This is necessary for tests that rely on animations
+  // This mock does not perform any actual animations, it just provides a structure that allows tests
+  // to run without throwing errors related to missing animate function
+  if (!HTMLElement.prototype.animate) {
+    HTMLElement.prototype.animate = function () {
+      return {
+        play: () => {},
+        pause: () => {},
+        finish: () => {},
+        cancel: () => {},
+        reverse: () => {},
+        addEventListener: () => {},
+        removeEventListener: () => {},
+        dispatchEvent: () => false,
+        onfinish: null,
+        oncancel: null,
+        startTime: 0,
+        currentTime: 0,
+        playbackRate: 1,
+        playState: "idle",
+        replaceState: "active",
+        effect: null,
+        finished: Promise.resolve(),
+        id: "",
+        remove: () => {},
+        timeline: null,
+        ready: Promise.resolve(),
+      } as unknown as Animation;
+    };
+  }
+});
+
+describe("RiskInsightsDrawerDialogComponent", () => {
+  let component: RiskInsightsDrawerDialogComponent;
+  let fixture: ComponentFixture<RiskInsightsDrawerDialogComponent>;
+  const mockI18nService = mock<I18nService>();
+  const drawerDetails: DrawerDetails = {
+    open: true,
+    invokerId: "test-invoker",
+    activeDrawerType: DrawerType.None,
+    atRiskMemberDetails: [],
+    appAtRiskMembers: null,
+    atRiskAppDetails: null,
+  };
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      imports: [RiskInsightsDrawerDialogComponent, BrowserAnimationsModule],
+      providers: [
+        { provide: DIALOG_DATA, useValue: drawerDetails },
+        { provide: I18nPipe, useValue: mock<I18nPipe>() },
+        { provide: I18nService, useValue: mockI18nService },
+      ],
+    }).compileComponents();
+
+    fixture = TestBed.createComponent(RiskInsightsDrawerDialogComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  it("should create", () => {
+    expect(component).toBeTruthy();
+  });
+
+  describe("drawerTypes getter", () => {
+    it("should return DrawerType enum", () => {
+      expect(component.drawerTypes).toBe(DrawerType);
+    });
+  });
+
+  describe("isActiveDrawerType", () => {
+    it("should return true if type matches activeDrawerType", () => {
+      component.drawerDetails.activeDrawerType = DrawerType.None;
+      expect(component.isActiveDrawerType(DrawerType.None)).toBeTruthy();
+    });
+
+    it("should return false if type does not match activeDrawerType", () => {
+      component.drawerDetails.activeDrawerType = DrawerType.None;
+      expect(component.isActiveDrawerType(DrawerType.AppAtRiskMembers)).toBeFalsy();
+    });
+  });
+});

bitwarden_license/bit-web/src/app/dirt/access-intelligence/shared/risk-insights-drawer-dialog.component.ts

@@ -0,0 +1,23 @@
+import { Component, ChangeDetectionStrategy, Inject } from "@angular/core";
+
+import { DrawerDetails, DrawerType } from "@bitwarden/bit-common/dirt/reports/risk-insights";
+import { DIALOG_DATA } from "@bitwarden/components";
+import { SharedModule } from "@bitwarden/web-vault/app/shared";
+
+@Component({
+  imports: [SharedModule],
+  templateUrl: "./risk-insights-drawer-dialog.component.html",
+  changeDetection: ChangeDetectionStrategy.OnPush,
+})
+export class RiskInsightsDrawerDialogComponent {
+  constructor(@Inject(DIALOG_DATA) public drawerDetails: DrawerDetails) {}
+
+  // Get a list of drawer types
+  get drawerTypes(): typeof DrawerType {
+    return DrawerType;
+  }
+
+  isActiveDrawerType(type: DrawerType): boolean {
+    return this.drawerDetails.activeDrawerType === type;
+  }
+}