[PM-18946] Improve Vault loading experience (#13714)

* [PM-18946] Refactor loading$ in vault-v2. Update icon-component, and build-cipher-icon

apps/browser/src/autofill/background/overlay.background.spec.ts

@@ -969,7 +969,7 @@ describe("OverlayBackground", () => {
             icon: {
               fallbackImage: "",
               icon: "bwi-credit-card",
-              image: undefined,
+              image: null,
               imageEnabled: true,
             },
             id: "inline-menu-cipher-0",
@@ -1007,7 +1007,7 @@ describe("OverlayBackground", () => {
               icon: {
                 fallbackImage: "",
                 icon: "bwi-id-card",
-                image: undefined,
+                image: null,
                 imageEnabled: true,
               },
               id: "inline-menu-cipher-1",
@@ -1048,7 +1048,7 @@ describe("OverlayBackground", () => {
               icon: {
                 fallbackImage: "",
                 icon: "bwi-id-card",
-                image: undefined,
+                image: null,
                 imageEnabled: true,
               },
               id: "inline-menu-cipher-0",
@@ -1120,7 +1120,7 @@ describe("OverlayBackground", () => {
               icon: {
                 fallbackImage: "",
                 icon: "bwi-id-card",
-                image: undefined,
+                image: null,
                 imageEnabled: true,
               },
               id: "inline-menu-cipher-1",

apps/browser/src/autofill/deprecated/background/overlay.background.deprecated.spec.ts

@@ -355,7 +355,7 @@ describe("OverlayBackground", () => {
           icon: {
             fallbackImage: "",
             icon: "bwi-credit-card",
-            image: undefined,
+            image: null,
             imageEnabled: true,
           },
           id: "overlay-cipher-2",
@@ -370,7 +370,7 @@ describe("OverlayBackground", () => {
           icon: {
             fallbackImage: "",
             icon: "bwi-credit-card",
-            image: undefined,
+            image: null,
             imageEnabled: true,
           },
           id: "overlay-cipher-3",

apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html

@@ -28,10 +28,7 @@
   ></blocked-injection-banner>
 
   <!-- Show search & filters outside of the scroll area of the page -->
-  <ng-container
+  <ng-container slot="above-scroll-area" *ngIf="vaultState !== VaultStateEnum.Empty">
-    slot="above-scroll-area"
-    *ngIf="vaultState !== VaultStateEnum.Empty && !(loading$ | async)"
-  >
     <vault-at-risk-password-callout
       *appIfFeature="FeatureFlag.SecurityTasks"
     ></vault-at-risk-password-callout>

apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts

@@ -2,7 +2,6 @@ import { CdkVirtualScrollableElement, ScrollingModule } from "@angular/cdk/scrol
 import { CommonModule } from "@angular/common";
 import { AfterViewInit, Component, DestroyRef, OnDestroy, OnInit, ViewChild } from "@angular/core";
 import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
-import { RouterLink } from "@angular/router";
 import {
   combineLatest,
   filter,
@@ -12,29 +11,24 @@ import {
   shareReplay,
   switchMap,
   take,
+  startWith,
 } from "rxjs";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
-import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { CipherId, CollectionId, OrganizationId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherType } from "@bitwarden/common/vault/enums";
-import {
+import { ButtonModule, DialogService, Icons, NoItemsModule } from "@bitwarden/components";
-  BannerComponent,
-  ButtonModule,
-  DialogService,
-  Icons,
-  NoItemsModule,
-} from "@bitwarden/components";
 import { DecryptionFailureDialogComponent, VaultIcons } from "@bitwarden/vault";
 
 import { CurrentAccountComponent } from "../../../../auth/popup/account-switching/current-account.component";
 import { PopOutComponent } from "../../../../platform/popup/components/pop-out.component";
 import { PopupHeaderComponent } from "../../../../platform/popup/layout/popup-header.component";
 import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page.component";
+import { VaultPopupCopyButtonsService } from "../../services/vault-popup-copy-buttons.service";
 import { VaultPopupItemsService } from "../../services/vault-popup-items.service";
 import { VaultPopupListFiltersService } from "../../services/vault-popup-list-filters.service";
 import { VaultPopupScrollPositionService } from "../../services/vault-popup-scroll-position.service";
@@ -73,12 +67,9 @@ enum VaultState {
     AutofillVaultListItemsComponent,
     VaultListItemsContainerComponent,
     ButtonModule,
-    RouterLink,
     NewItemDropdownV2Component,
     ScrollingModule,
     VaultHeaderV2Component,
-    DecryptionFailureDialogComponent,
-    BannerComponent,
     AtRiskPasswordCalloutComponent,
     NewSettingsCalloutComponent,
   ],
@@ -93,9 +84,15 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy {
   protected remainingCiphers$ = this.vaultPopupItemsService.remainingCiphers$;
   protected allFilters$ = this.vaultPopupListFiltersService.allFilters$;
 
-  protected loading$ = combineLatest([this.vaultPopupItemsService.loading$, this.allFilters$]).pipe(
+  protected loading$ = combineLatest([
+    this.vaultPopupItemsService.loading$,
+    this.allFilters$,
+    // Added as a dependency to avoid flashing the copyActions on slower devices
+    this.vaultCopyButtonsService.showQuickCopyActions$,
+  ]).pipe(
     map(([itemsLoading, filters]) => itemsLoading || !filters),
     shareReplay({ bufferSize: 1, refCount: true }),
+    startWith(true),
   );
 
   protected newItemItemValues$: Observable<NewItemInitialValues> =
@@ -130,8 +127,7 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy {
     private destroyRef: DestroyRef,
     private cipherService: CipherService,
     private dialogService: DialogService,
-    private vaultProfileService: VaultProfileService,
+    private vaultCopyButtonsService: VaultPopupCopyButtonsService,
-    private vaultPageService: VaultPageService,
   ) {
     combineLatest([
       this.vaultPopupItemsService.emptyVault$,

apps/browser/src/vault/popup/services/vault-popup-copy-buttons.service.ts

@@ -1,5 +1,5 @@
 import { inject, Injectable } from "@angular/core";
-import { map, Observable } from "rxjs";
+import { map, Observable, shareReplay } from "rxjs";
 
 import {
   GlobalStateProvider,
@@ -31,6 +31,7 @@ export class VaultPopupCopyButtonsService {
 
   showQuickCopyActions$: Observable<boolean> = this.displayMode$.pipe(
     map((displayMode) => displayMode === "quick"),
+    shareReplay({ bufferSize: 1, refCount: true }),
   );
 
   async setShowQuickCopyActions(value: boolean) {

apps/web/src/app/vault/components/vault-items/vault-items.stories.ts

@@ -17,7 +17,10 @@ import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.servic
 import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
 import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
-import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
+import {
+  Environment,
+  EnvironmentService,
+} from "@bitwarden/common/platform/abstractions/environment.service";
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
 import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
 import { CipherType } from "@bitwarden/common/vault/enums";
@@ -53,6 +56,11 @@ export default {
             getIconsUrl() {
               return "";
             },
+            environment$: new BehaviorSubject({
+              getIconsUrl() {
+                return "";
+              },
+            } as Environment).asObservable(),
           } as Partial<EnvironmentService>,
         },
         {

libs/angular/src/vault/components/icon.component.html

@@ -2,16 +2,18 @@
   <ng-container *ngIf="data$ | async as data">
     <img
       [src]="data.image"
-      [appFallbackSrc]="data.fallbackImage"
       *ngIf="data.imageEnabled && data.image"
       class="tw-size-6 tw-rounded-md"
       alt=""
       decoding="async"
       loading="lazy"
+      [ngClass]="{ 'tw-invisible tw-absolute': !imageLoaded() }"
+      (load)="imageLoaded.set(true)"
+      (error)="imageLoaded.set(false)"
     />
     <i
       class="tw-w-6 tw-text-muted bwi bwi-lg {{ data.icon }}"
-      *ngIf="!data.imageEnabled || !data.image"
+      *ngIf="!data.imageEnabled || !data.image || !imageLoaded()"
     ></i>
   </ng-container>
 </div>

libs/angular/src/vault/components/icon.component.ts

@@ -1,18 +1,18 @@
-// FIXME: Update this file to be type safe and remove this and next line
+import { ChangeDetectionStrategy, Component, input, signal } from "@angular/core";
-// @ts-strict-ignore
+import { toObservable } from "@angular/core/rxjs-interop";
-import { ChangeDetectionStrategy, Component, Input, OnInit } from "@angular/core";
 import {
-  BehaviorSubject,
   combineLatest,
   distinctUntilChanged,
-  filter,
   map,
+  tap,
   Observable,
+  startWith,
+  pairwise,
 } from "rxjs";
 
 import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
-import { buildCipherIcon } from "@bitwarden/common/vault/icon/build-cipher-icon";
+import { buildCipherIcon, CipherIconDetails } from "@bitwarden/common/vault/icon/build-cipher-icon";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
 @Component({
@@ -20,33 +20,40 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
   templateUrl: "icon.component.html",
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class IconComponent implements OnInit {
+export class IconComponent {
-  @Input()
+  /**
-  set cipher(value: CipherView) {
+   * The cipher to display the icon for.
-    this.cipher$.next(value);
+   */
-  }
+  cipher = input.required<CipherView>();
 
-  protected data$: Observable<{
+  imageLoaded = signal(false);
-    imageEnabled: boolean;
-    image?: string;
-    fallbackImage: string;
-    icon?: string;
-  }>;
 
-  private cipher$ = new BehaviorSubject<CipherView>(undefined);
+  protected data$: Observable<CipherIconDetails>;
 
   constructor(
     private environmentService: EnvironmentService,
     private domainSettingsService: DomainSettingsService,
-  ) {}
+  ) {
-
+    const iconSettings$ = combineLatest([
-  async ngOnInit() {
-    this.data$ = combineLatest([
       this.environmentService.environment$.pipe(map((e) => e.getIconsUrl())),
       this.domainSettingsService.showFavicons$.pipe(distinctUntilChanged()),
-      this.cipher$.pipe(filter((c) => c !== undefined)),
     ]).pipe(
-      map(([iconsUrl, showFavicon, cipher]) => buildCipherIcon(iconsUrl, cipher, showFavicon)),
+      map(([iconsUrl, showFavicon]) => ({ iconsUrl, showFavicon })),
+      startWith({ iconsUrl: null, showFavicon: false }), // Start with a safe default to avoid flickering icons
+      distinctUntilChanged(),
+    );
+
+    this.data$ = combineLatest([iconSettings$, toObservable(this.cipher)]).pipe(
+      map(([{ iconsUrl, showFavicon }, cipher]) => buildCipherIcon(iconsUrl, cipher, showFavicon)),
+      startWith(null),
+      pairwise(),
+      tap(([prev, next]) => {
+        if (prev?.image !== next?.image) {
+          // The image changed, reset the loaded state to not show an empty icon
+          this.imageLoaded.set(false);
+        }
+      }),
+      map(([_, next]) => next!),
     );
   }
 }

libs/common/src/vault/icon/build-cipher-icon.spec.ts

@@ -89,7 +89,7 @@ describe("buildCipherIcon", () => {
 
       expect(iconDetails).toEqual({
         icon: "bwi-globe",
-        image: undefined,
+        image: null,
         fallbackImage: "",
         imageEnabled: false,
       });
@@ -102,7 +102,7 @@ describe("buildCipherIcon", () => {
 
       expect(iconDetails).toEqual({
         icon: "bwi-globe",
-        image: undefined,
+        image: null,
         fallbackImage: "",
         imageEnabled: true,
       });

libs/common/src/vault/icon/build-cipher-icon.ts

@@ -2,9 +2,23 @@ import { Utils } from "../../platform/misc/utils";
 import { CipherType } from "../enums/cipher-type";
 import { CipherView } from "../models/view/cipher.view";
 
-export function buildCipherIcon(iconsServerUrl: string, cipher: CipherView, showFavicon: boolean) {
+export interface CipherIconDetails {
-  let icon;
+  imageEnabled: boolean;
-  let image;
+  image: string | null;
+  /**
+   * @deprecated Fallback to `icon` instead which will default to "bwi-globe" if no other icon is applicable.
+   */
+  fallbackImage: string;
+  icon: string;
+}
+
+export function buildCipherIcon(
+  iconsServerUrl: string | null,
+  cipher: CipherView,
+  showFavicon: boolean,
+): CipherIconDetails {
+  let icon: string = "bwi-globe";
+  let image: string | null = null;
   let fallbackImage = "";
   const cardIcons: Record<string, string> = {
     Visa: "card-visa",
@@ -18,6 +32,10 @@ export function buildCipherIcon(iconsServerUrl: string, cipher: CipherView, show
     RuPay: "card-ru-pay",
   };
 
+  if (iconsServerUrl == null) {
+    showFavicon = false;
+  }
+
   switch (cipher.type) {
     case CipherType.Login:
       icon = "bwi-globe";
@@ -53,9 +71,7 @@ export function buildCipherIcon(iconsServerUrl: string, cipher: CipherView, show
           try {
             image = `${iconsServerUrl}/${Utils.getHostname(hostnameUri)}/icon.png`;
             fallbackImage = "images/bwi-globe.png";
-            // FIXME: Remove when updating file. Eslint update
+          } catch {
-            // eslint-disable-next-line @typescript-eslint/no-unused-vars
-          } catch (e) {
             // Ignore error since the fallback icon will be shown if image is null.
           }
         }