[PM-17663] Fix extension Fill button display on page load (#15359)

* [PM-17663] Convert vault-list-items-container inputs to signals - Cleaned up some grouping logic - Cleaned up strict null checks and removed eslint comment * [PM-17663] Prefer undefined over null * [PM-17663] Fix flashing Fill buttons
2025-12-10 21:33:27 +00:00 · 2025-07-02 13:19:17 -07:00
parent 023b057f3e
commit ece5ebe844
4 changed files with 94 additions and 105 deletions
--- a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html
+++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html
@@ -4,7 +4,7 @@
  [title]="((currentURIIsBlocked$ | async) ? 'itemSuggestions' : 'autofillSuggestions') | i18n"
  [showRefresh]="showRefresh"
  (onRefresh)="refreshCurrentTab()"
-  [description]="(showEmptyAutofillTip$ | async) ? ('autofillSuggestionsTip' | i18n) : null"
+  [description]="(showEmptyAutofillTip$ | async) ? ('autofillSuggestionsTip' | i18n) : undefined"
  showAutofillButton
  [disableDescriptionMargin]="showEmptyAutofillTip$ | async"
  [primaryActionAutofill]="clickItemsToAutofillVaultView$ | async"
--- a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts
+++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts
@@ -1,7 +1,7 @@
 import { CommonModule } from "@angular/common";
 import { Component } from "@angular/core";
 import { toSignal } from "@angular/core/rxjs-interop";
-import { combineLatest, map, Observable } from "rxjs";
+import { combineLatest, map, Observable, startWith } from "rxjs";

 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service";
@@ -41,7 +41,9 @@ export class AutofillVaultListItemsComponent {

  /** Flag indicating whether the login item should automatically autofill when clicked  */
  protected clickItemsToAutofillVaultView$: Observable<boolean> =
-    this.vaultSettingsService.clickItemsToAutofillVaultView$;
+    this.vaultSettingsService.clickItemsToAutofillVaultView$.pipe(
+      startWith(true), // Start with true to avoid flashing the fill button on first load
+    );

  protected groupByType = toSignal(
    this.vaultPopupItemsService.hasFilterApplied$.pipe(map((hasFilter) => !hasFilter)),
@@ -74,9 +76,7 @@ export class AutofillVaultListItemsComponent {
    private vaultPopupItemsService: VaultPopupItemsService,
    private vaultPopupAutofillService: VaultPopupAutofillService,
    private vaultSettingsService: VaultSettingsService,
-  ) {
-    // TODO: Migrate logic to show Autofill policy toast PM-8144
-  }
+  ) {}

  /**
   * Refreshes the current tab to re-populate the autofill ciphers.
--- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html
+++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html
@@ -1,8 +1,8 @@
 <bit-section
-  *ngIf="cipherGroups$().length > 0 || description"
-  [disableMargin]="disableSectionMargin"
+  *ngIf="cipherGroups().length > 0 || description()"
+  [disableMargin]="disableSectionMargin()"
 >
-  <ng-container *ngIf="collapsibleKey">
+  <ng-container *ngIf="collapsibleKey()">
    <button
      class="tw-group/vault-section-header hover:tw-bg-primary-100 tw-rounded-md tw-pl-1 tw-w-full tw-border-x-0 tw-border-t-0 tw-border-b tw-border-solid focus-visible:tw-outline-none focus-visible:tw-ring-inset focus-visible:tw-ring-2 focus-visible:tw-ring-primary-600"
      [ngClass]="{
@@ -22,7 +22,7 @@
    </bit-disclosure>
  </ng-container>

-  <ng-container *ngIf="!collapsibleKey">
+  <ng-container *ngIf="!collapsibleKey()">
    <div class="tw-pl-1">
      <ng-container *ngTemplateOutlet="sectionHeader"></ng-container>
    </div>
@@ -34,10 +34,10 @@
 <ng-template #sectionHeader>
  <bit-section-header class="tw-p-0.5 -tw-mx-0.5">
    <h2 bitTypography="h6">
-      {{ title }}
+      {{ title() }}
    </h2>
    <button
-      *ngIf="showRefresh"
+      *ngIf="showRefresh()"
      bitIconButton="bwi-refresh"
      type="button"
      size="small"
@@ -48,13 +48,13 @@
      <span
        [ngClass]="{
          'group-hover/vault-section-header:tw-hidden group-focus-visible/vault-section-header:tw-hidden':
-            collapsibleKey && sectionOpenState(),
-          'tw-hidden': collapsibleKey && !sectionOpenState(),
+            collapsibleKey() && sectionOpenState(),
+          'tw-hidden': collapsibleKey() && !sectionOpenState(),
        }"
      >
        {{ ciphers().length }}
      </span>
-      <span class="tw-pr-1" *ngIf="collapsibleKey">
+      <span class="tw-pr-1" *ngIf="collapsibleKey()">
        <i
          class="bwi tw-text-main"
          [ngClass]="{
@@ -71,18 +71,18 @@

 <ng-template #descriptionText>
  <div
-    *ngIf="description"
+    *ngIf="description()"
    class="tw-text-muted tw-px-1 tw-mb-2"
-    [ngClass]="{ '!tw-mb-0': disableDescriptionMargin }"
+    [ngClass]="{ '!tw-mb-0': disableDescriptionMargin() }"
    bitTypography="body2"
  >
-    {{ description }}
+    {{ description() }}
  </div>
 </ng-template>

 <ng-template #itemGroup>
  <bit-item-group>
-    <ng-container *ngFor="let group of cipherGroups$()">
+    <ng-container *ngFor="let group of cipherGroups()">
      <ng-container *ngIf="group.subHeaderKey">
        <h3 class="tw-text-muted tw-text-xs tw-font-semibold tw-pl-1 tw-mb-1 bit-compact:tw-m-0">
          {{ group.subHeaderKey | i18n }}
@@ -97,7 +97,7 @@
            (click)="primaryActionOnSelect(cipher)"
            (dblclick)="launchCipher(cipher)"
            [appA11yTitle]="
-              cipherItemTitleKey(cipher) | async | i18n: cipher.name : cipher.login.username
+              cipherItemTitleKey()(cipher) | i18n: cipher.name : cipher.login.username
            "
            class="{{ itemHeightClass }}"
          >
@@ -109,7 +109,7 @@
              *ngIf="cipher.organizationId"
              slot="default-trailing"
              appOrgIcon
-              [tierType]="cipher.organization.productTierType"
+              [tierType]="cipher.organization!.productTierType"
              [size]="'small'"
              [appA11yTitle]="orgIconTooltip(cipher)"
            ></i>
@@ -122,7 +122,7 @@
          </button>

          <ng-container slot="end">
-            <bit-item-action *ngIf="!(hideAutofillButton$ | async)">
+            <bit-item-action *ngIf="!hideAutofillButton()">
              <button
                type="button"
                bitBadge
@@ -134,7 +134,7 @@
                {{ "fill" | i18n }}
              </button>
            </bit-item-action>
-            <bit-item-action *ngIf="!showAutofillButton && cipher.canLaunch">
+            <bit-item-action *ngIf="!showAutofillButton() && cipher.canLaunch">
              <button
                type="button"
                bitIconButton="bwi-external-link"
@@ -147,8 +147,8 @@
            <app-item-copy-actions [cipher]="cipher"></app-item-copy-actions>
            <app-item-more-options
              [cipher]="cipher"
-              [hideAutofillOptions]="hideAutofillOptions$ | async"
-              [showViewOption]="primaryActionAutofill"
+              [hideAutofillOptions]="hideAutofillMenuOptions()"
+              [showViewOption]="primaryActionAutofill()"
            ></app-item-more-options>
          </ng-container>
        </bit-item>
--- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts
+++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts
@@ -1,5 +1,3 @@
-// FIXME: Update this file to be type safe and remove this and next line
-// @ts-strict-ignore
 import { CdkVirtualScrollViewport, ScrollingModule } from "@angular/cdk/scrolling";
 import { CommonModule } from "@angular/common";
 import {
@@ -8,18 +6,17 @@ import {
  Component,
  EventEmitter,
  inject,
-  Input,
  Output,
  Signal,
  signal,
  ViewChild,
  computed,
-  OnInit,
  ChangeDetectionStrategy,
  input,
 } from "@angular/core";
+import { toSignal } from "@angular/core/rxjs-interop";
 import { Router } from "@angular/router";
-import { firstValueFrom, Observable, map } from "rxjs";
+import { firstValueFrom, map } from "rxjs";

 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -53,7 +50,10 @@ import {
 import { BrowserApi } from "../../../../../platform/browser/browser-api";
 import BrowserPopupUtils from "../../../../../platform/browser/browser-popup-utils";
 import { VaultPopupAutofillService } from "../../../services/vault-popup-autofill.service";
-import { VaultPopupSectionService } from "../../../services/vault-popup-section.service";
+import {
+  VaultPopupSectionService,
+  PopupSectionOpen,
+} from "../../../services/vault-popup-section.service";
 import { PopupCipherView } from "../../../views/popup-cipher.view";
 import { ItemCopyActionsComponent } from "../item-copy-action/item-copy-actions.component";
 import { ItemMoreOptionsComponent } from "../item-more-options/item-more-options.component";
@@ -81,17 +81,25 @@ import { ItemMoreOptionsComponent } from "../item-more-options/item-more-options
  templateUrl: "vault-list-items-container.component.html",
  changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
+export class VaultListItemsContainerComponent implements AfterViewInit {
  private compactModeService = inject(CompactModeService);
  private vaultPopupSectionService = inject(VaultPopupSectionService);

-  @ViewChild(CdkVirtualScrollViewport, { static: false }) viewPort: CdkVirtualScrollViewport;
-  @ViewChild(DisclosureComponent) disclosure: DisclosureComponent;
+  @ViewChild(CdkVirtualScrollViewport, { static: false }) viewPort!: CdkVirtualScrollViewport;
+  @ViewChild(DisclosureComponent) disclosure!: DisclosureComponent;

  /**
   * Indicates whether the section should be open or closed if collapsibleKey is provided
   */
-  protected sectionOpenState: Signal<boolean> | undefined;
+  protected sectionOpenState: Signal<boolean> = computed(() => {
+    if (!this.collapsibleKey()) {
+      return true;
+    }
+
+    return (
+      this.vaultPopupSectionService.getOpenDisplayStateForSection(this.collapsibleKey()!)() ?? true
+    );
+  });

  /**
   * The class used to set the height of a bit item's inner content.
@@ -115,7 +123,7 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
   * Timeout used to add a small delay when selecting a cipher to allow for double click to launch
   * @private
   */
-  private viewCipherTimeout: number | null;
+  private viewCipherTimeout?: number;

  ciphers = input<PopupCipherView[]>([]);

@@ -123,31 +131,33 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
   * If true, we will group ciphers by type (Login, Card, Identity)
   * within subheadings in a single container, converted to a WritableSignal.
   */
-  groupByType = input<boolean>(false);
+  groupByType = input<boolean | undefined>(false);

  /**
   * Computed signal for a grouped list of ciphers with an optional header
   */
-  cipherGroups$ = computed<
+  cipherGroups = computed<
    {
-      subHeaderKey?: string | null;
+      subHeaderKey?: string;
      ciphers: PopupCipherView[];
    }[]
  >(() => {
-    const groups: { [key: string]: CipherView[] } = {};
+    // Not grouping by type, return a single group with all ciphers
+    if (!this.groupByType()) {
+      return [{ ciphers: this.ciphers() }];
+    }
+
+    const groups: Record<string, PopupCipherView[]> = {};

    this.ciphers().forEach((cipher) => {
-      let groupKey;
-
-      if (this.groupByType()) {
-        switch (cipher.type) {
-          case CipherType.Card:
-            groupKey = "cards";
-            break;
-          case CipherType.Identity:
-            groupKey = "identities";
-            break;
-        }
+      let groupKey = "all";
+      switch (cipher.type) {
+        case CipherType.Card:
+          groupKey = "cards";
+          break;
+        case CipherType.Identity:
+          groupKey = "identities";
+          break;
      }

      if (!groups[groupKey]) {
@@ -157,17 +167,16 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
      groups[groupKey].push(cipher);
    });

-    return Object.keys(groups).map((key) => ({
-      subHeaderKey: this.groupByType ? key : "",
-      ciphers: groups[key],
+    return Object.entries(groups).map(([key, ciphers]) => ({
+      subHeaderKey: key != "all" ? key : undefined,
+      ciphers: ciphers,
    }));
  });

  /**
   * Title for the vault list item section.
   */
-  @Input()
-  title: string;
+  title = input<string | undefined>(undefined);

  /**
   * Optionally allow the items to be collapsed.
@@ -175,21 +184,18 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
   * The key must be added to the state definition in `vault-popup-section.service.ts` since the
   * collapsed state is stored locally.
   */
-  @Input()
-  collapsibleKey: "favorites" | "allItems" | undefined;
+  collapsibleKey = input<keyof PopupSectionOpen | undefined>(undefined);

  /**
   * Optional description for the vault list item section. Will be shown below the title even when
   * no ciphers are available.
   */
-  @Input()
-  description: string;
+  description = input<string | undefined>(undefined);

  /**
   * Option to show a refresh button in the section header.
   */
-  @Input({ transform: booleanAttribute })
-  showRefresh: boolean;
+  showRefresh = input(false, { transform: booleanAttribute });

  /**
   * Event emitted when the refresh button is clicked.
@@ -200,66 +206,61 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
  /**
   * Flag indicating that the current tab location is blocked
   */
-  currentURIIsBlocked$: Observable<boolean> =
-    this.vaultPopupAutofillService.currentTabIsOnBlocklist$;
+  currentURIIsBlocked = toSignal(this.vaultPopupAutofillService.currentTabIsOnBlocklist$);

  /**
   * Resolved i18n key to use for suggested cipher items
   */
-  cipherItemTitleKey = (cipher: CipherView) =>
-    this.currentURIIsBlocked$.pipe(
-      map((uriIsBlocked) => {
-        const hasUsername = cipher.login?.username != null;
-        const key = this.primaryActionAutofill && !uriIsBlocked ? "autofillTitle" : "viewItemTitle";
-        return hasUsername ? `${key}WithField` : key;
-      }),
-    );
+  cipherItemTitleKey = computed(() => {
+    return (cipher: CipherView) => {
+      const hasUsername = cipher.login?.username != null;
+      const key =
+        this.primaryActionAutofill() && !this.currentURIIsBlocked()
+          ? "autofillTitle"
+          : "viewItemTitle";
+      return hasUsername ? `${key}WithField` : key;
+    };
+  });

  /**
   * Option to show the autofill button for each item.
   */
-  @Input({ transform: booleanAttribute })
-  showAutofillButton: boolean;
+  showAutofillButton = input(false, { transform: booleanAttribute });

  /**
   * Flag indicating whether the suggested cipher item autofill button should be shown or not
   */
-  hideAutofillButton$ = this.currentURIIsBlocked$.pipe(
-    map((uriIsBlocked) => !this.showAutofillButton || uriIsBlocked || this.primaryActionAutofill),
+  hideAutofillButton = computed(
+    () => !this.showAutofillButton() || this.currentURIIsBlocked() || this.primaryActionAutofill(),
  );

  /**
-   * Flag indicating whether the cipher item autofill options should be shown or not
+   * Flag indicating whether the cipher item autofill menu options should be shown or not
   */
-  hideAutofillOptions$: Observable<boolean> = this.currentURIIsBlocked$.pipe(
-    map((uriIsBlocked) => uriIsBlocked || this.showAutofillButton),
-  );
+  hideAutofillMenuOptions = computed(() => this.currentURIIsBlocked() || this.showAutofillButton());

  /**
   * Option to perform autofill operation as the primary action for autofill suggestions.
   */
-  @Input({ transform: booleanAttribute })
-  primaryActionAutofill: boolean;
+  primaryActionAutofill = input(false, { transform: booleanAttribute });

  /**
   * Remove the bottom margin from the bit-section in this component
   * (used for containers at the end of the page where bottom margin is not needed)
   */
-  @Input({ transform: booleanAttribute })
-  disableSectionMargin: boolean = false;
+  disableSectionMargin = input(false, { transform: booleanAttribute });

  /**
   * Remove the description margin
   */
-  @Input({ transform: booleanAttribute })
-  disableDescriptionMargin: boolean = false;
+  disableDescriptionMargin = input(false, { transform: booleanAttribute });

  /**
   * The tooltip text for the organization icon for ciphers that belong to an organization.
   * @param cipher
   */
  orgIconTooltip(cipher: PopupCipherView) {
-    if (cipher.collectionIds.length > 1) {
+    if (cipher.collectionIds.length > 1 || !cipher.collections) {
      return this.i18nService.t("nCollections", cipher.collectionIds.length);
    }

@@ -279,16 +280,6 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
    private accountService: AccountService,
  ) {}

-  ngOnInit(): void {
-    if (!this.collapsibleKey) {
-      return;
-    }
-
-    this.sectionOpenState = this.vaultPopupSectionService.getOpenDisplayStateForSection(
-      this.collapsibleKey,
-    );
-  }
-
  async ngAfterViewInit() {
    const autofillShortcut = await this.platformUtilsService.getAutofillKeyboardShortcut();

@@ -301,10 +292,8 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
    }
  }

-  async primaryActionOnSelect(cipher: CipherView) {
-    const isBlocked = await firstValueFrom(this.currentURIIsBlocked$);
-
-    return this.primaryActionAutofill && !isBlocked
+  primaryActionOnSelect(cipher: CipherView) {
+    return this.primaryActionAutofill() && !this.currentURIIsBlocked()
      ? this.doAutofill(cipher)
      : this.onViewCipher(cipher);
  }
@@ -320,7 +309,7 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
    // If there is a view action pending, clear it
    if (this.viewCipherTimeout != null) {
      window.clearTimeout(this.viewCipherTimeout);
-      this.viewCipherTimeout = null;
+      this.viewCipherTimeout = undefined;
    }

    const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
@@ -363,7 +352,7 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
          });
        } finally {
          // Ensure the timeout is always cleared
-          this.viewCipherTimeout = null;
+          this.viewCipherTimeout = undefined;
        }
      },
      cipher.canLaunch ? 200 : 0,
@@ -374,12 +363,12 @@ export class VaultListItemsContainerComponent implements OnInit, AfterViewInit {
   * Update section open/close state based on user action
   */
  async toggleSectionOpen() {
-    if (!this.collapsibleKey) {
+    if (!this.collapsibleKey()) {
      return;
    }

    await this.vaultPopupSectionService.updateSectionOpenStoredState(
-      this.collapsibleKey,
+      this.collapsibleKey()!,
      this.disclosure.open,
    );
  }