[PM-27661] - Multiple URIs - Add a Collapse Button after clicking View All for Saved Websites (#17352)

* use signals. add toggleable list view. * use @for. remove redundant if statement * fix template variable name * clean up test setup * Update apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts Co-authored-by: Nik Gilmore <ngilmore@bitwarden.com> --------- Co-authored-by: Nik Gilmore <ngilmore@bitwarden.com>
2025-12-06 00:13:28 +00:00 · 2025-11-13 10:34:38 -08:00
parent 37c1a5ee17
commit 18c1d8b2d3
7 changed files with 94 additions and 118 deletions
--- a/apps/browser/src/_locales/en/messages.json
+++ b/apps/browser/src/_locales/en/messages.json
@@ -594,6 +594,9 @@
  "viewAll": {
    "message": "View all"
  },
+  "viewLess": {
+    "message": "View less"
+  },
  "viewLogin": {
    "message": "View login"
  },
--- a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html
+++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html
@@ -1,60 +1,60 @@
-<bit-dialog>
-  <span bitDialogTitle>{{ "confirmAutofill" | i18n }}</span>
+<bit-dialog [title]="'confirmAutofill' | i18n">
  <div bitDialogContent>
    <p bitTypography="body2">
      {{ "confirmAutofillDesc" | i18n }}
    </p>
-    @if (savedUrls.length === 1) {
+    @if (savedUrls().length === 1) {
      <p class="tw-text-muted tw-text-xs tw-uppercase tw-mt-4 tw-font-medium">
        {{ "savedWebsite" | i18n }}
      </p>
      <bit-callout [title]="null" type="success" icon="bwi-globe">
-        <div class="tw-font-mono tw-line-clamp-1 tw-break-all" [appA11yTitle]="savedUrls[0]">
-          {{ savedUrls[0] }}
+        <div class="tw-font-mono tw-line-clamp-1 tw-break-all" [appA11yTitle]="savedUrls()[0]">
+          {{ savedUrls()[0] }}
        </div>
      </bit-callout>
    }
-    @if (savedUrls.length > 1) {
+    @if (savedUrls().length > 1) {
      <div class="tw-flex tw-justify-between tw-items-center tw-mt-4 tw-mb-1 tw-pt-2">
        <p class="tw-text-muted tw-text-xs tw-uppercase tw-font-medium">
-          {{ "savedWebsites" | i18n: savedUrls.length }}
+          {{ "savedWebsites" | i18n: savedUrls().length }}
        </p>
        <button
-          *ngIf="!savedUrlsExpanded"
          type="button"
          bitLink
          class="tw-text-sm tw-font-medium tw-cursor-pointer"
-          (click)="viewAllSavedUrls()"
+          (click)="toggleSavedUrlExpandedState()"
        >
-          {{ "viewAll" | i18n }}
+          {{ (savedUrlsExpanded() ? "viewLess" : "viewAll") | i18n }}
        </button>
      </div>
-      <div class="tw-pt-2" [ngClass]="savedUrlsListClass">
-        <div class="-tw-mt-2" *ngFor="let url of savedUrls">
-          <bit-callout [title]="null" type="success" icon="bwi-globe">
-            <div class="tw-font-mono tw-line-clamp-1 tw-break-all" [appA11yTitle]="url">
-              {{ url }}
-            </div>
-          </bit-callout>
-        </div>
+      <div class="tw-pt-2" [ngClass]="savedUrlsListClass()">
+        @for (url of savedUrls(); track url) {
+          <div class="-tw-mt-2">
+            <bit-callout [title]="null" type="success" icon="bwi-globe">
+              <div class="tw-font-mono tw-line-clamp-1 tw-break-all" [appA11yTitle]="url">
+                {{ url }}
+              </div>
+            </bit-callout>
+          </div>
+        }
      </div>
    }
    <p class="tw-text-muted tw-text-xs tw-uppercase tw-mt-5 tw-font-medium">
      {{ "currentWebsite" | i18n }}
    </p>
    <bit-callout [title]="null" type="warning" icon="bwi-globe">
-      <div [appA11yTitle]="currentUrl" class="tw-font-mono tw-line-clamp-1 tw-break-all">
-        {{ currentUrl }}
+      <div [appA11yTitle]="currentUrl()" class="tw-font-mono tw-line-clamp-1 tw-break-all">
+        {{ currentUrl() }}
      </div>
    </bit-callout>
    <div class="tw-flex tw-justify-center tw-flex-col tw-gap-3 tw-mt-6">
-      @if (!viewOnly) {
+      @if (!viewOnly()) {
        <button type="button" bitButton buttonType="primary" (click)="autofillAndAddUrl()">
          {{ "autofillAndAddWebsite" | i18n }}
        </button>
      }
      <button type="button" bitButton buttonType="secondary" (click)="autofillOnly()">
-        {{ (viewOnly ? "autofill" : "autofillWithoutAdding") | i18n }}
+        {{ (viewOnly() ? "autofill" : "autofillWithoutAdding") | i18n }}
      </button>
      <button
        type="button"
--- a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts
+++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts
@@ -29,7 +29,11 @@ describe("AutofillConfirmationDialogComponent", () => {
    params?: AutofillConfirmationDialogParams;
    viewOnly?: boolean;
  }) {
-    const p = options?.params ?? params;
+    const base = options?.params ?? params;
+    const p: AutofillConfirmationDialogParams = {
+      ...base,
+      viewOnly: options?.viewOnly,
+    };

    TestBed.resetTestingModule();
    await TestBed.configureTestingModule({
@@ -46,12 +50,6 @@ describe("AutofillConfirmationDialogComponent", () => {

    const freshFixture = TestBed.createComponent(AutofillConfirmationDialogComponent);
    const freshInstance = freshFixture.componentInstance;
-
-    // If needed, set viewOnly BEFORE first detectChanges so initial render reflects it.
-    if (typeof options?.viewOnly !== "undefined") {
-      freshInstance.viewOnly = options.viewOnly;
-    }
-
    freshFixture.detectChanges();
    return { fixture: freshFixture, component: freshInstance };
  }
@@ -95,10 +93,8 @@ describe("AutofillConfirmationDialogComponent", () => {

  it("normalizes currentUrl and savedUrls via Utils.getHostname", () => {
    expect(Utils.getHostname).toHaveBeenCalledTimes(1 + (params.savedUrls?.length ?? 0));
-    // current
-    expect(component.currentUrl).toBe("example.com");
-    // saved
-    expect(component.savedUrls).toEqual([
+    expect(component.currentUrl()).toBe("example.com");
+    expect(component.savedUrls()).toEqual([
      "one.example.com",
      "two.example.com",
      "not-a-url.example",
@@ -115,30 +111,30 @@ describe("AutofillConfirmationDialogComponent", () => {

  it("emits Canceled on close()", () => {
    const spy = jest.spyOn(dialogRef, "close");
-    component["close"]();
+    (component as any)["close"]();
    expect(spy).toHaveBeenCalledWith(AutofillConfirmationDialogResult.Canceled);
  });

  it("emits AutofillAndUrlAdded on autofillAndAddUrl()", () => {
    const spy = jest.spyOn(dialogRef, "close");
-    component["autofillAndAddUrl"]();
+    (component as any)["autofillAndAddUrl"]();
    expect(spy).toHaveBeenCalledWith(AutofillConfirmationDialogResult.AutofillAndUrlAdded);
  });

  it("emits AutofilledOnly on autofillOnly()", () => {
    const spy = jest.spyOn(dialogRef, "close");
-    component["autofillOnly"]();
+    (component as any)["autofillOnly"]();
    expect(spy).toHaveBeenCalledWith(AutofillConfirmationDialogResult.AutofilledOnly);
  });

-  it("applies collapsed list gradient class by default, then clears it after viewAllSavedUrls()", () => {
-    const initial = component["savedUrlsListClass"];
+  it("applies collapsed list gradient class by default, then clears it after toggling", () => {
+    const initial = component.savedUrlsListClass();
    expect(initial).toContain("gradient");

-    component["viewAllSavedUrls"]();
+    component.toggleSavedUrlExpandedState();
    fixture.detectChanges();

-    const expanded = component["savedUrlsListClass"];
+    const expanded = component.savedUrlsListClass();
    expect(expanded).toBe("");
  });

@@ -149,37 +145,36 @@ describe("AutofillConfirmationDialogComponent", () => {
    };

    const { component: fresh } = await createFreshFixture({ params: newParams });
-    expect(fresh.savedUrls).toEqual([]);
-    expect(fresh.currentUrl).toBe("bitwarden.com");
+    expect(fresh.savedUrls()).toEqual([]);
+    expect(fresh.currentUrl()).toBe("bitwarden.com");
  });

-  it("handles undefined savedUrls by defaulting to [] and empty strings from Utils.getHostname", () => {
+  it("handles undefined savedUrls by defaulting to [] and empty strings from Utils.getHostname", async () => {
    const localParams: AutofillConfirmationDialogParams = {
      currentUrl: "https://sub.domain.tld/x",
    };

-    const local = new AutofillConfirmationDialogComponent(localParams as any, dialogRef);
-
-    expect(local.savedUrls).toEqual([]);
-    expect(local.currentUrl).toBe("sub.domain.tld");
+    const { component: local } = await createFreshFixture({ params: localParams });
+    expect(local.savedUrls()).toEqual([]);
+    expect(local.currentUrl()).toBe("sub.domain.tld");
  });

-  it("filters out falsy/invalid values from Utils.getHostname in savedUrls", () => {
-    (Utils.getHostname as jest.Mock).mockImplementationOnce(() => "example.com");
-    (Utils.getHostname as jest.Mock)
-      .mockImplementationOnce(() => "ok.example")
-      .mockImplementationOnce(() => "")
-      .mockImplementationOnce(() => undefined as unknown as string);
+  it("filters out falsy/invalid values from Utils.getHostname in savedUrls", async () => {
+    const hostSpy = jest.spyOn(Utils, "getHostname");
+    hostSpy.mockImplementationOnce(() => "example.com");
+    hostSpy.mockImplementationOnce(() => "ok.example");
+    hostSpy.mockImplementationOnce(() => "");
+    hostSpy.mockImplementationOnce(() => undefined as unknown as string);

    const edgeParams: AutofillConfirmationDialogParams = {
      currentUrl: "https://example.com",
      savedUrls: ["https://ok.example", "://bad", "%%%"],
    };

-    const edge = new AutofillConfirmationDialogComponent(edgeParams as any, dialogRef);
+    const { component: edge } = await createFreshFixture({ params: edgeParams });

-    expect(edge.currentUrl).toBe("example.com");
-    expect(edge.savedUrls).toEqual(["ok.example"]);
+    expect(edge.currentUrl()).toBe("example.com");
+    expect(edge.savedUrls()).toEqual(["ok.example"]);
  });

  it("renders one current-url callout and N saved-url callouts", () => {
@@ -196,7 +191,7 @@ describe("AutofillConfirmationDialogComponent", () => {
    expect(text).toContain("two.example.com");
  });

-  it("shows the 'view all' button when savedUrls > 1 and hides it after click", () => {
+  it("shows the 'view all' button when savedUrls > 1 and toggles the button text when clicked", () => {
    const findViewAll = () =>
      fixture.nativeElement.querySelector(
        "button.tw-text-sm.tw-font-medium.tw-cursor-pointer",
@@ -209,35 +204,31 @@ describe("AutofillConfirmationDialogComponent", () => {
    fixture.detectChanges();

    btn = findViewAll();
-    expect(btn).toBeFalsy();
-    expect(component.savedUrlsExpanded).toBe(true);
+    expect(btn!.textContent).toContain("viewLess");
+    expect(component.savedUrlsExpanded()).toBe(true);
  });

  it("shows autofillWithoutAdding text on autofill button when viewOnly is false", () => {
    fixture.detectChanges();
-
    const text = fixture.nativeElement.textContent as string;
    expect(text.includes("autofillWithoutAdding")).toBe(true);
  });

  it("does not show autofillWithoutAdding text on autofill button when viewOnly is true", async () => {
    const { fixture: vf } = await createFreshFixture({ viewOnly: true });
-
    const text = vf.nativeElement.textContent as string;
    expect(text.includes("autofillWithoutAdding")).toBe(false);
  });

  it("shows autofill and save button when viewOnly is false", () => {
-    component.viewOnly = false;
+    // default viewOnly is false
    fixture.detectChanges();
-
    const text = fixture.nativeElement.textContent as string;
    expect(text.includes("autofillAndAddWebsite")).toBe(true);
  });

  it("does not show autofill and save button when viewOnly is true", async () => {
    const { fixture: vf } = await createFreshFixture({ viewOnly: true });
-
    const text = vf.nativeElement.textContent as string;
    expect(text.includes("autofillAndAddWebsite")).toBe(false);
  });
--- a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.ts
+++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.ts
@@ -1,5 +1,5 @@
 import { CommonModule } from "@angular/common";
-import { ChangeDetectionStrategy, Component, Inject } from "@angular/core";
+import { ChangeDetectionStrategy, Component, computed, inject, signal } from "@angular/core";

 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -8,8 +8,8 @@ import {
  DIALOG_DATA,
  DialogConfig,
  DialogRef,
-  ButtonModule,
  DialogService,
+  ButtonModule,
  DialogModule,
  TypographyModule,
  CalloutComponent,
@@ -46,49 +46,37 @@ export type AutofillConfirmationDialogResultType = UnionOfValues<
  ],
 })
 export class AutofillConfirmationDialogComponent {
-  AutofillConfirmationDialogResult = AutofillConfirmationDialogResult;
+  private readonly params = inject<AutofillConfirmationDialogParams>(DIALOG_DATA);
+  private readonly dialogRef = inject(DialogRef<AutofillConfirmationDialogResultType>);

-  currentUrl: string = "";
-  savedUrls: string[] = [];
-  savedUrlsExpanded = false;
-  viewOnly: boolean = false;
+  readonly currentUrl = signal<string>(Utils.getHostname(this.params.currentUrl));
+  readonly savedUrls = signal<string[]>(
+    (this.params.savedUrls ?? []).map((u) => Utils.getHostname(u) ?? "").filter(Boolean),
+  );
+  readonly viewOnly = signal<boolean>(this.params.viewOnly ?? false);
+  readonly savedUrlsExpanded = signal<boolean>(false);

-  constructor(
-    @Inject(DIALOG_DATA) protected params: AutofillConfirmationDialogParams,
-    private dialogRef: DialogRef,
-  ) {
-    this.currentUrl = Utils.getHostname(params.currentUrl);
-    this.viewOnly = params.viewOnly ?? false;
-    this.savedUrls =
-      params.savedUrls?.map((url) => Utils.getHostname(url) ?? "").filter(Boolean) ?? [];
-  }
-
-  protected get savedUrlsListClass(): string {
-    return this.savedUrlsExpanded
+  readonly savedUrlsListClass = computed(() =>
+    this.savedUrlsExpanded()
      ? ""
-      : `tw-relative
-         tw-max-h-24
-         tw-overflow-hidden
-         after:tw-pointer-events-none after:tw-content-['']
-         after:tw-absolute after:tw-inset-x-0 after:tw-bottom-0
-         after:tw-h-8 after:tw-bg-gradient-to-t
-         after:tw-from-background after:tw-to-transparent
-    `;
+      : `tw-relative tw-max-h-24 tw-overflow-hidden after:tw-pointer-events-none
+         after:tw-content-[''] after:tw-absolute after:tw-inset-x-0 after:tw-bottom-0
+         after:tw-h-8 after:tw-bg-gradient-to-t after:tw-from-background after:tw-to-transparent`,
+  );
+
+  toggleSavedUrlExpandedState() {
+    this.savedUrlsExpanded.update((v) => !v);
  }

-  protected viewAllSavedUrls() {
-    this.savedUrlsExpanded = true;
-  }
-
-  protected close() {
+  close() {
    this.dialogRef.close(AutofillConfirmationDialogResult.Canceled);
  }

-  protected autofillAndAddUrl() {
+  autofillAndAddUrl() {
    this.dialogRef.close(AutofillConfirmationDialogResult.AutofillAndUrlAdded);
  }

-  protected autofillOnly() {
+  autofillOnly() {
    this.dialogRef.close(AutofillConfirmationDialogResult.AutofilledOnly);
  }

--- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html
+++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html
@@ -14,7 +14,7 @@
          {{ "autofill" | i18n }}
        </button>
        <!-- Autofill confirmation handles both 'autofill' and 'autofill and save' so no need to show both -->
-        @if (!(showAutofillConfirmation$ | async)) {
+        @if (!(autofillConfirmationFlagEnabled$ | async)) {
          <button
            type="button"
            bitMenuItem
@@ -52,7 +52,7 @@
      </a>
    </ng-container>
    @if (canArchive$ | async) {
-      <button type="button" bitMenuItem (click)="archive()" *ngIf="canArchive$ | async">
+      <button type="button" bitMenuItem (click)="archive()">
        {{ "archiveVerb" | i18n }}
      </button>
    }
--- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts
+++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts
@@ -2,6 +2,7 @@ import { CUSTOM_ELEMENTS_SCHEMA } from "@angular/core";
 import { ComponentFixture, TestBed, waitForAsync } from "@angular/core/testing";
 import { NoopAnimationsModule } from "@angular/platform-browser/animations";
 import { Router } from "@angular/router";
+import { mock } from "jest-mock-extended";
 import { BehaviorSubject, of } from "rxjs";

 import { CollectionService } from "@bitwarden/admin-console/common";
@@ -66,11 +67,6 @@ describe("ItemMoreOptionsComponent", () => {
    resolvedDefaultUriMatchStrategy$: uriMatchStrategy$.asObservable(),
  };

-  const hasSearchText$ = new BehaviorSubject(false);
-  const vaultPopupItemsService = {
-    hasSearchText$: hasSearchText$.asObservable(),
-  };
-
  const baseCipher = {
    id: "cipher-1",
    login: {
@@ -120,7 +116,7 @@ describe("ItemMoreOptionsComponent", () => {
        },
        {
          provide: VaultPopupItemsService,
-          useValue: vaultPopupItemsService,
+          useValue: mock<VaultPopupItemsService>({}),
        },
      ],
      schemas: [CUSTOM_ELEMENTS_SCHEMA],
@@ -153,7 +149,7 @@ describe("ItemMoreOptionsComponent", () => {
      expect(passwordRepromptService.passwordRepromptCheck).toHaveBeenCalledWith(baseCipher);
    });

-    it("calls the autofill service to autofill without showing the confirmation dialog when the feature flag is disabled or search text is not present", async () => {
+    it("calls the autofill service to autofill without showing the confirmation dialog when the feature flag is disabled", async () => {
      autofillSvc.currentAutofillTab$.next({ url: "https://page.example.com" });

      await component.doAutofill();
@@ -182,7 +178,7 @@ describe("ItemMoreOptionsComponent", () => {
    });

    it("does not show the exact match dialog when the default match strategy is Exact and autofill confirmation is not to be shown", async () => {
-      // autofill confirmation dialog is not shown when either the feature flag is disabled or search text is not present
+      // autofill confirmation dialog is not shown when either the feature flag is disabled
      uriMatchStrategy$.next(UriMatchStrategy.Exact);
      autofillSvc.currentAutofillTab$.next({ url: "https://page.example.com/path" });
      await component.doAutofill();
@@ -192,9 +188,8 @@ describe("ItemMoreOptionsComponent", () => {

    describe("autofill confirmation dialog", () => {
      beforeEach(() => {
-        // autofill confirmation dialog is shown when feature flag is enabled and search text is present
+        // autofill confirmation dialog is shown when feature flag is enabled
        featureFlag$.next(true);
-        hasSearchText$.next(true);
        uriMatchStrategy$.next(UriMatchStrategy.Domain);
        passwordRepromptService.passwordRepromptCheck.mockResolvedValue(true);
      });
@@ -208,7 +203,7 @@ describe("ItemMoreOptionsComponent", () => {
        expect(passwordRepromptService.passwordRepromptCheck).toHaveBeenCalledWith(baseCipher);
      });

-      it("opens the autofill confirmation dialog with filtered saved URLs when the feature flag is enabled and search text is present", async () => {
+      it("opens the autofill confirmation dialog with filtered saved URLs when the feature flag is enabled", async () => {
        autofillSvc.currentAutofillTab$.next({ url: "https://page.example.com/path" });
        const openSpy = mockConfirmDialogResult(AutofillConfirmationDialogResult.Canceled);

@@ -216,8 +211,8 @@ describe("ItemMoreOptionsComponent", () => {

        expect(openSpy).toHaveBeenCalledTimes(1);
        const args = openSpy.mock.calls[0][1];
-        expect(args.data.currentUrl).toBe("https://page.example.com/path");
-        expect(args.data.savedUrls).toEqual([
+        expect(args.data?.currentUrl).toBe("https://page.example.com/path");
+        expect(args.data?.savedUrls).toEqual([
          "https://one.example.com",
          "https://two.example.com/a",
        ]);
--- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts
+++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts
@@ -84,10 +84,9 @@ export class ItemMoreOptionsComponent {

  protected autofillAllowed$ = this.vaultPopupAutofillService.autofillAllowed$;

-  protected showAutofillConfirmation$ = combineLatest([
-    this.configService.getFeatureFlag$(FeatureFlag.AutofillConfirmation),
-    this.vaultPopupItemsService.hasSearchText$,
-  ]).pipe(map(([isFeatureFlagEnabled, hasSearchText]) => isFeatureFlagEnabled && hasSearchText));
+  protected autofillConfirmationFlagEnabled$ = this.configService
+    .getFeatureFlag$(FeatureFlag.AutofillConfirmation)
+    .pipe(map((isFeatureFlagEnabled) => isFeatureFlagEnabled));

  protected uriMatchStrategy$ = this.domainSettingsService.resolvedDefaultUriMatchStrategy$;

@@ -210,7 +209,7 @@ export class ItemMoreOptionsComponent {
    const cipherHasAllExactMatchLoginUris =
      uris.length > 0 && uris.every((u) => u.uri && u.match === UriMatchStrategy.Exact);

-    const showAutofillConfirmation = await firstValueFrom(this.showAutofillConfirmation$);
+    const showAutofillConfirmation = await firstValueFrom(this.autofillConfirmationFlagEnabled$);
    const uriMatchStrategy = await firstValueFrom(this.uriMatchStrategy$);

    if (