diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.html b/apps/browser/src/tools/popup/send-v2/send-v2.component.html index 0bcbd47a145..47ecd7564dc 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.html +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.html @@ -47,8 +47,8 @@ @if (showSkeletonsLoaders$ | async) { - + - + } diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.ts b/apps/browser/src/tools/popup/send-v2/send-v2.component.ts index 43a1119deca..e3baba53c42 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.ts +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.ts @@ -15,6 +15,8 @@ import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; +import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; +import { skeletonLoadingDelay } from "@bitwarden/common/vault/utils/skeleton-loading.operator"; import { ButtonModule, CalloutModule, @@ -95,8 +97,16 @@ export class SendV2Component implements OnDestroy { /** Skeleton Loading State */ protected showSkeletonsLoaders$ = combineLatest([ this.sendsLoading$, + this.searchService.isSendSearching$, this.skeletonFeatureFlag$, - ]).pipe(map(([loading, skeletonsEnabled]) => loading && skeletonsEnabled)); + ]).pipe( + map( + ([loading, cipherSearching, skeletonsEnabled]) => + (loading || cipherSearching) && skeletonsEnabled, + ), + distinctUntilChanged(), + skeletonLoadingDelay(), + ); protected title: string = "allSends"; protected noItemIcon = NoSendsIcon; @@ -110,6 +120,7 @@ export class SendV2Component implements OnDestroy { private policyService: PolicyService, private accountService: AccountService, private configService: ConfigService, + private searchService: SearchService, ) { combineLatest([ this.sendItemsService.emptyList$, diff --git a/apps/browser/src/vault/popup/components/vault-fade-in-out/vault-fade-in-out.component.html b/apps/browser/src/vault/popup/components/vault-fade-in-out/vault-fade-in-out.component.html new file mode 100644 index 00000000000..6dbc7430638 --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-fade-in-out/vault-fade-in-out.component.html @@ -0,0 +1 @@ + diff --git a/apps/browser/src/vault/popup/components/vault-fade-in-out/vault-fade-in-out.component.ts b/apps/browser/src/vault/popup/components/vault-fade-in-out/vault-fade-in-out.component.ts new file mode 100644 index 00000000000..a30a447833b --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-fade-in-out/vault-fade-in-out.component.ts @@ -0,0 +1,20 @@ +import { animate, style, transition, trigger } from "@angular/animations"; +import { ChangeDetectionStrategy, Component, HostBinding } from "@angular/core"; + +@Component({ + selector: "vault-fade-in-out", + templateUrl: "./vault-fade-in-out.component.html", + animations: [ + trigger("fadeInOut", [ + transition(":enter", [ + style({ opacity: 0 }), + animate("100ms ease-in", style({ opacity: 1 })), + ]), + transition(":leave", [animate("300ms ease-out", style({ opacity: 0 }))]), + ]), + ], + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class VaultFadeInOutComponent { + @HostBinding("@fadeInOut") fadeInOut = true; +} diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html index faaa6a40e98..7a5a99c8100 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html @@ -8,20 +8,32 @@ -
- - {{ "yourVaultIsEmpty" | i18n }} - -

{{ "emptyVaultDescription" | i18n }}

- - - {{ "newLogin" | i18n }} - - -
+ +
+ + {{ "yourVaultIsEmpty" | i18n }} + +

+ {{ "emptyVaultDescription" | i18n }} +

+ + + {{ "newLogin" | i18n }} + + +
+ + + @if (skeletonFeatureFlag$ | async) { + + + + } @else { + + } - - - - - + + + + + + + + + @if (skeletonFeatureFlag$ | async) { + + + + } @else { + + } @if (showSkeletonsLoaders$ | async) { diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts index 563ec5f9709..5563cd3033b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts @@ -23,6 +23,7 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; import { TaskService } from "@bitwarden/common/vault/tasks"; import { DialogService } from "@bitwarden/components"; @@ -259,6 +260,10 @@ describe("VaultV2Component", () => { getFeatureFlag$: (_: string) => of(false), }, }, + { + provide: SearchService, + useValue: { isCipherSearching$: of(false) }, + }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 499e9b76757..471e6e70601 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -32,8 +32,10 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherId, CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { UnionOfValues } from "@bitwarden/common/vault/types/union-of-values"; +import { skeletonLoadingDelay } from "@bitwarden/common/vault/utils/skeleton-loading.operator"; import { ButtonModule, DialogService, @@ -54,6 +56,7 @@ import { VaultPopupListFiltersService } from "../../services/vault-popup-list-fi import { VaultPopupLoadingService } from "../../services/vault-popup-loading.service"; import { VaultPopupScrollPositionService } from "../../services/vault-popup-scroll-position.service"; import { AtRiskPasswordCalloutComponent } from "../at-risk-callout/at-risk-password-callout.component"; +import { VaultFadeInOutComponent } from "../vault-fade-in-out/vault-fade-in-out.component"; import { VaultFadeInOutSkeletonComponent } from "../vault-fade-in-out-skeleton/vault-fade-in-out-skeleton.component"; import { VaultLoadingSkeletonComponent } from "../vault-loading-skeleton/vault-loading-skeleton.component"; @@ -100,6 +103,7 @@ type VaultState = UnionOfValues; TypographyModule, VaultLoadingSkeletonComponent, VaultFadeInOutSkeletonComponent, + VaultFadeInOutComponent, ], }) export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { @@ -129,7 +133,7 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { }), ); - private skeletonFeatureFlag$ = this.configService.getFeatureFlag$( + protected skeletonFeatureFlag$ = this.configService.getFeatureFlag$( FeatureFlag.VaultLoadingSkeletons, ); @@ -183,9 +187,18 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { map(([loading, skeletonsEnabled]) => loading && !skeletonsEnabled), ); - /** When true, show skeleton loading state */ - protected showSkeletonsLoaders$ = combineLatest([this.loading$, this.skeletonFeatureFlag$]).pipe( - map(([loading, skeletonsEnabled]) => loading && skeletonsEnabled), + /** When true, show skeleton loading state with debouncing to prevent flicker */ + protected showSkeletonsLoaders$ = combineLatest([ + this.loading$, + this.searchService.isCipherSearching$, + this.skeletonFeatureFlag$, + ]).pipe( + map( + ([loading, cipherSearching, skeletonsEnabled]) => + (loading || cipherSearching) && skeletonsEnabled, + ), + distinctUntilChanged(), + skeletonLoadingDelay(), ); protected newItemItemValues$: Observable = @@ -228,6 +241,7 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { private liveAnnouncer: LiveAnnouncer, private i18nService: I18nService, private configService: ConfigService, + private searchService: SearchService, ) { combineLatest([ this.vaultPopupItemsService.emptyVault$, diff --git a/libs/common/src/vault/abstractions/search.service.ts b/libs/common/src/vault/abstractions/search.service.ts index 233dee9ec75..29575ec3af9 100644 --- a/libs/common/src/vault/abstractions/search.service.ts +++ b/libs/common/src/vault/abstractions/search.service.ts @@ -6,6 +6,9 @@ import { CipherView } from "../models/view/cipher.view"; import { CipherViewLike } from "../utils/cipher-view-like-utils"; export abstract class SearchService { + abstract isCipherSearching$: Observable; + abstract isSendSearching$: Observable; + abstract indexedEntityId$(userId: UserId): Observable; abstract clearIndex(userId: UserId): Promise; diff --git a/libs/common/src/vault/services/search.service.ts b/libs/common/src/vault/services/search.service.ts index 80fddda42d5..0b34bd3863f 100644 --- a/libs/common/src/vault/services/search.service.ts +++ b/libs/common/src/vault/services/search.service.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import * as lunr from "lunr"; -import { Observable, firstValueFrom, map } from "rxjs"; +import { BehaviorSubject, Observable, firstValueFrom, map } from "rxjs"; import { Jsonify } from "type-fest"; import { perUserCache$ } from "@bitwarden/common/vault/utils/observable-utilities"; @@ -81,6 +81,12 @@ export class SearchService implements SearchServiceAbstraction { private readonly defaultSearchableMinLength: number = 2; private searchableMinLength: number = this.defaultSearchableMinLength; + private _isCipherSearching$ = new BehaviorSubject(false); + isCipherSearching$: Observable = this._isCipherSearching$.asObservable(); + + private _isSendSearching$ = new BehaviorSubject(false); + isSendSearching$: Observable = this._isSendSearching$.asObservable(); + constructor( private logService: LogService, private i18nService: I18nService, @@ -223,6 +229,7 @@ export class SearchService implements SearchServiceAbstraction { filter: ((cipher: C) => boolean) | ((cipher: C) => boolean)[] = null, ciphers: C[], ): Promise { + this._isCipherSearching$.next(true); const results: C[] = []; const searchStartTime = performance.now(); if (query != null) { @@ -243,6 +250,7 @@ export class SearchService implements SearchServiceAbstraction { } if (!(await this.isSearchable(userId, query))) { + this._isCipherSearching$.next(false); return ciphers; } @@ -258,6 +266,7 @@ export class SearchService implements SearchServiceAbstraction { // Fall back to basic search if index is not available const basicResults = this.searchCiphersBasic(ciphers, query); this.logService.measure(searchStartTime, "Vault", "SearchService", "basic search complete"); + this._isCipherSearching$.next(false); return basicResults; } @@ -293,6 +302,7 @@ export class SearchService implements SearchServiceAbstraction { }); } this.logService.measure(searchStartTime, "Vault", "SearchService", "search complete"); + this._isCipherSearching$.next(false); return results; } @@ -335,8 +345,10 @@ export class SearchService implements SearchServiceAbstraction { } searchSends(sends: SendView[], query: string) { + this._isSendSearching$.next(true); query = SearchService.normalizeSearchQuery(query.trim().toLocaleLowerCase()); if (query === null) { + this._isSendSearching$.next(false); return sends; } const sendsMatched: SendView[] = []; @@ -359,6 +371,7 @@ export class SearchService implements SearchServiceAbstraction { lowPriorityMatched.push(s); } }); + this._isSendSearching$.next(false); return sendsMatched.concat(lowPriorityMatched); } diff --git a/libs/common/src/vault/utils/skeleton-loading.operator.spec.ts b/libs/common/src/vault/utils/skeleton-loading.operator.spec.ts new file mode 100644 index 00000000000..3ba790f64cb --- /dev/null +++ b/libs/common/src/vault/utils/skeleton-loading.operator.spec.ts @@ -0,0 +1,109 @@ +import { BehaviorSubject } from "rxjs"; + +import { skeletonLoadingDelay } from "./skeleton-loading.operator"; + +describe("skeletonLoadingDelay", () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + }); + + it("returns false immediately when starting with false", () => { + const source$ = new BehaviorSubject(false); + const results: boolean[] = []; + + source$.pipe(skeletonLoadingDelay()).subscribe((value) => results.push(value)); + + expect(results).toEqual([false]); + }); + + it("waits 1 second before returning true when starting with true", () => { + const source$ = new BehaviorSubject(true); + const results: boolean[] = []; + + source$.pipe(skeletonLoadingDelay()).subscribe((value) => results.push(value)); + + expect(results).toEqual([]); + + jest.advanceTimersByTime(999); + expect(results).toEqual([]); + + jest.advanceTimersByTime(1); + expect(results).toEqual([true]); + }); + + it("cancels if source becomes false before show delay completes", () => { + const source$ = new BehaviorSubject(true); + const results: boolean[] = []; + + source$.pipe(skeletonLoadingDelay()).subscribe((value) => results.push(value)); + + jest.advanceTimersByTime(500); + source$.next(false); + + expect(results).toEqual([false]); + + jest.advanceTimersByTime(1000); + expect(results).toEqual([false]); + }); + + it("delays hiding if minimum display time has not elapsed", () => { + const source$ = new BehaviorSubject(true); + const results: boolean[] = []; + + source$.pipe(skeletonLoadingDelay()).subscribe((value) => results.push(value)); + + jest.advanceTimersByTime(1000); + expect(results).toEqual([true]); + + source$.next(false); + + expect(results).toEqual([true]); + + jest.advanceTimersByTime(1000); + expect(results).toEqual([true, false]); + }); + + it("handles rapid true->false->true transitions", () => { + const source$ = new BehaviorSubject(true); + const results: boolean[] = []; + + source$.pipe(skeletonLoadingDelay()).subscribe((value) => results.push(value)); + + jest.advanceTimersByTime(500); + expect(results).toEqual([]); + + source$.next(false); + expect(results).toEqual([false]); + + source$.next(true); + + jest.advanceTimersByTime(999); + expect(results).toEqual([false]); + + jest.advanceTimersByTime(1); + expect(results).toEqual([false, true]); + }); + + it("allows for custom timings", () => { + const source$ = new BehaviorSubject(true); + const results: boolean[] = []; + + source$.pipe(skeletonLoadingDelay(1000, 2000)).subscribe((value) => results.push(value)); + + jest.advanceTimersByTime(1000); + expect(results).toEqual([true]); + + source$.next(false); + + jest.advanceTimersByTime(1999); + expect(results).toEqual([true]); + + jest.advanceTimersByTime(1); + expect(results).toEqual([true, false]); + }); +}); diff --git a/libs/common/src/vault/utils/skeleton-loading.operator.ts b/libs/common/src/vault/utils/skeleton-loading.operator.ts new file mode 100644 index 00000000000..b9ff28f64b5 --- /dev/null +++ b/libs/common/src/vault/utils/skeleton-loading.operator.ts @@ -0,0 +1,59 @@ +import { defer, Observable, of, timer } from "rxjs"; +import { map, switchMap, tap } from "rxjs/operators"; + +/** + * RxJS operator that adds skeleton loading delay behavior. + * + * - Waits 1 second before showing (prevents flashing for quick loads) + * - Ensures skeleton stays visible for at least 1 second once shown regardless of the source observable emissions + * - After the minimum display time, if the source is still true, continues to emit true until the source becomes false + * - False can only be emitted either: + * - Immediately when the source emits false before the skeleton is shown + * - After the minimum display time has passed once the skeleton is shown + */ +export function skeletonLoadingDelay( + showDelay = 1000, + minDisplayTime = 1000, +): (source: Observable) => Observable { + return (source: Observable) => { + return defer(() => { + let skeletonShownAt: number | null = null; + + return source.pipe( + switchMap((shouldShow): Observable => { + if (shouldShow) { + if (skeletonShownAt !== null) { + return of(true); // Already shown, continue showing + } + + // Wait for delay, then mark the skeleton as shown and emit true + return timer(showDelay).pipe( + tap(() => { + skeletonShownAt = Date.now(); + }), + map(() => true), + ); + } else { + if (skeletonShownAt === null) { + // Skeleton not shown yet, can emit false immediately + return of(false); + } + + // Skeleton shown, ensure minimum display time has passed + const elapsedTime = Date.now() - skeletonShownAt; + const remainingTime = Math.max(0, minDisplayTime - elapsedTime); + + // Wait for remaining time to ensure minimum display time + return timer(remainingTime).pipe( + tap(() => { + // Reset the shown timestamp + skeletonShownAt = null; + }), + map(() => false), + ); + } + }), + ); + }); + }; +}