From ece5ebe844e3fd4e97210c196d5ff7d2c63682cb Mon Sep 17 00:00:00 2001
From: Shane Melton <smelton@bitwarden.com>
Date: Wed, 2 Jul 2025 13:19:17 -0700
Subject: [PATCH] [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
---
 .../autofill-vault-list-items.component.html  |   2 +-
 .../autofill-vault-list-items.component.ts    |  10 +-
 .../vault-list-items-container.component.html |  38 ++---
 .../vault-list-items-container.component.ts   | 149 ++++++++----------
 4 files changed, 94 insertions(+), 105 deletions(-)

diff --git 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
index 19f1668eba4..47ef0284d6a 100644
--- 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"
diff --git 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
index 47f104cd4d3..aa790d24ede 100644
--- 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.
diff --git 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
index 87d13d4d18a..8dca1f9e576 100644
--- 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>
diff --git 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
index 7c99e4e68b8..1105f929e04 100644
--- 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,
     );
   }