1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-26688][PM-27710] Delay skeletons from showing + search (#17394)

* add custom operator for loading skeleton delays

* add `isCipherSearching$` observable to search service

* prevent vault skeleton from showing immediately

* add skeleton for search + delay to sends

* update fade-in-out component selector

* add fade-in-out component for generic use

* address memory leak by using defer to encapsulate `skeletonShownAt`

* add missing provider
This commit is contained in:
Nick Krantz
2025-11-20 08:26:47 -06:00
committed by GitHub
parent 9e6d0cce35
commit b00987180d
11 changed files with 295 additions and 38 deletions

View File

@@ -47,8 +47,8 @@
<app-send-list-items-container [headerText]="title | i18n" [sends]="sends$ | async" />
</ng-container>
@if (showSkeletonsLoaders$ | async) {
<vault-fade-in-skeleton>
<vault-fade-in-out-skeleton>
<vault-loading-skeleton></vault-loading-skeleton>
</vault-fade-in-skeleton>
</vault-fade-in-out-skeleton>
}
</popup-page>

View File

@@ -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$,

View File

@@ -0,0 +1 @@
<ng-content></ng-content>

View File

@@ -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;
}

View File

@@ -8,20 +8,32 @@
</ng-container>
</popup-header>
<div
*ngIf="vaultState === VaultStateEnum.Empty"
class="tw-flex tw-flex-col tw-h-full tw-justify-center"
>
<bit-no-items [icon]="vaultIcon">
<ng-container slot="title">{{ "yourVaultIsEmpty" | i18n }}</ng-container>
<ng-container slot="description">
<p bitTypography="body2" class="tw-mx-6 tw-mt-2">{{ "emptyVaultDescription" | i18n }}</p>
</ng-container>
<a slot="button" bitButton buttonType="secondary" [routerLink]="['/add-cipher']">
{{ "newLogin" | i18n }}
</a>
</bit-no-items>
</div>
<ng-template #emptyVaultTemplate>
<div
*ngIf="vaultState === VaultStateEnum.Empty"
class="tw-flex tw-flex-col tw-h-full tw-justify-center"
>
<bit-no-items [icon]="vaultIcon">
<ng-container slot="title">{{ "yourVaultIsEmpty" | i18n }}</ng-container>
<ng-container slot="description">
<p bitTypography="body2" class="tw-mx-6 tw-mt-2">
{{ "emptyVaultDescription" | i18n }}
</p>
</ng-container>
<a slot="button" bitButton buttonType="secondary" [routerLink]="['/add-cipher']">
{{ "newLogin" | i18n }}
</a>
</bit-no-items>
</div>
</ng-template>
@if (skeletonFeatureFlag$ | async) {
<vault-fade-in-out *ngIf="vaultState === VaultStateEnum.Empty">
<ng-container *ngTemplateOutlet="emptyVaultTemplate"></ng-container>
</vault-fade-in-out>
} @else {
<ng-container *ngTemplateOutlet="emptyVaultTemplate"></ng-container>
}
<blocked-injection-banner
*ngIf="vaultState !== VaultStateEnum.Empty"
@@ -95,22 +107,32 @@
</div>
</div>
<ng-container *ngIf="vaultState === null">
<app-autofill-vault-list-items></app-autofill-vault-list-items>
<app-vault-list-items-container
[title]="'favorites' | i18n"
[ciphers]="(favoriteCiphers$ | async) || []"
id="favorites"
collapsibleKey="favorites"
></app-vault-list-items-container>
<app-vault-list-items-container
[title]="'allItems' | i18n"
[ciphers]="(remainingCiphers$ | async) || []"
id="allItems"
disableSectionMargin
collapsibleKey="allItems"
></app-vault-list-items-container>
</ng-container>
<ng-template #vaultContentTemplate>
<ng-container *ngIf="vaultState === null">
<app-autofill-vault-list-items></app-autofill-vault-list-items>
<app-vault-list-items-container
[title]="'favorites' | i18n"
[ciphers]="(favoriteCiphers$ | async) || []"
id="favorites"
collapsibleKey="favorites"
></app-vault-list-items-container>
<app-vault-list-items-container
[title]="'allItems' | i18n"
[ciphers]="(remainingCiphers$ | async) || []"
id="allItems"
disableSectionMargin
collapsibleKey="allItems"
></app-vault-list-items-container>
</ng-container>
</ng-template>
@if (skeletonFeatureFlag$ | async) {
<vault-fade-in-out *ngIf="vaultState === null">
<ng-container *ngTemplateOutlet="vaultContentTemplate"></ng-container>
</vault-fade-in-out>
} @else {
<ng-container *ngTemplateOutlet="vaultContentTemplate"></ng-container>
}
</ng-container>
@if (showSkeletonsLoaders$ | async) {

View File

@@ -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();

View File

@@ -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<typeof VaultState>;
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<NewItemInitialValues> =
@@ -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$,

View File

@@ -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<boolean>;
abstract isSendSearching$: Observable<boolean>;
abstract indexedEntityId$(userId: UserId): Observable<IndexedEntityId | null>;
abstract clearIndex(userId: UserId): Promise<void>;

View File

@@ -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<boolean>(false);
isCipherSearching$: Observable<boolean> = this._isCipherSearching$.asObservable();
private _isSendSearching$ = new BehaviorSubject<boolean>(false);
isSendSearching$: Observable<boolean> = 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<C[]> {
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);
}

View File

@@ -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<boolean>(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<boolean>(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<boolean>(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<boolean>(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<boolean>(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<boolean>(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]);
});
});

View File

@@ -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<boolean>) => Observable<boolean> {
return (source: Observable<boolean>) => {
return defer(() => {
let skeletonShownAt: number | null = null;
return source.pipe(
switchMap((shouldShow): Observable<boolean> => {
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),
);
}
}),
);
});
};
}