1
0
mirror of https://github.com/bitwarden/browser synced 2026-01-28 15:23:53 +00:00

remove skeleton ff

This commit is contained in:
Nick Krantz
2026-01-20 09:23:13 -06:00
parent 246765a1aa
commit d61fd99063
9 changed files with 61 additions and 159 deletions

View File

@@ -1,4 +1,4 @@
<popup-page [loading]="showSpinnerLoaders$ | async" [hideOverflow]="showSkeletonsLoaders$ | async">
<popup-page [hideOverflow]="showSkeletonsLoaders$ | async">
<popup-header slot="header" [pageTitle]="'send' | i18n">
<ng-container slot="end">
<tools-new-send-dropdown *ngIf="!sendsDisabled"></tools-new-send-dropdown>

View File

@@ -11,7 +11,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
@@ -110,7 +109,6 @@ describe("SendV2Component", () => {
provide: BillingAccountProfileStateService,
useValue: { hasPremiumFromAnySource$: of(false) },
},
{ provide: ConfigService, useValue: mock<ConfigService>() },
{ provide: EnvironmentService, useValue: mock<EnvironmentService>() },
{ provide: LogService, useValue: mock<LogService>() },
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },

View File

@@ -11,8 +11,6 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
import { PolicyType } from "@bitwarden/common/admin-console/enums";
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 { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { SendType } from "@bitwarden/common/tools/send/types/send-type";
import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service";
import { SearchService } from "@bitwarden/common/vault/abstractions/search.service";
@@ -84,30 +82,17 @@ export class SendV2Component implements OnDestroy {
protected listState: SendState | null = null;
protected sends$ = this.sendItemsService.filteredAndSortedSends$;
private skeletonFeatureFlag$ = this.configService.getFeatureFlag$(
FeatureFlag.VaultLoadingSkeletons,
);
protected sendsLoading$ = this.sendItemsService.loading$.pipe(
distinctUntilChanged(),
shareReplay({ bufferSize: 1, refCount: true }),
);
/** Spinner Loading State */
protected showSpinnerLoaders$ = combineLatest([
this.sendsLoading$,
this.skeletonFeatureFlag$,
]).pipe(map(([loading, skeletonsEnabled]) => loading && !skeletonsEnabled));
/** Skeleton Loading State */
protected showSkeletonsLoaders$ = combineLatest([
this.sendsLoading$,
this.searchService.isSendSearching$,
this.skeletonFeatureFlag$,
]).pipe(
map(
([loading, cipherSearching, skeletonsEnabled]) =>
(loading || cipherSearching) && skeletonsEnabled,
),
map(([loading, cipherSearching]) => loading || cipherSearching),
distinctUntilChanged(),
skeletonLoadingDelay(),
);
@@ -128,7 +113,6 @@ export class SendV2Component implements OnDestroy {
protected sendListFiltersService: SendListFiltersService,
private policyService: PolicyService,
private accountService: AccountService,
private configService: ConfigService,
private searchService: SearchService,
) {
combineLatest([

View File

@@ -4,7 +4,6 @@ import { FormsModule } from "@angular/forms";
import { BehaviorSubject } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { SearchTextDebounceInterval } from "@bitwarden/common/vault/services/search.service";
import { SearchModule } from "@bitwarden/components";
@@ -20,7 +19,6 @@ describe("VaultV2SearchComponent", () => {
const searchText$ = new BehaviorSubject("");
const loading$ = new BehaviorSubject(false);
const featureFlag$ = new BehaviorSubject(true);
const applyFilter = jest.fn();
const createComponent = () => {
@@ -31,7 +29,6 @@ describe("VaultV2SearchComponent", () => {
beforeEach(async () => {
applyFilter.mockClear();
featureFlag$.next(true);
await TestBed.configureTestingModule({
imports: [VaultV2SearchComponent, CommonModule, SearchModule, JslibModule, FormsModule],
@@ -49,12 +46,6 @@ describe("VaultV2SearchComponent", () => {
loading$,
},
},
{
provide: ConfigService,
useValue: {
getFeatureFlag$: jest.fn(() => featureFlag$),
},
},
{ provide: I18nService, useValue: { t: (key: string) => key } },
],
}).compileComponents();
@@ -70,91 +61,55 @@ describe("VaultV2SearchComponent", () => {
});
describe("debouncing behavior", () => {
describe("when feature flag is enabled", () => {
beforeEach(() => {
featureFlag$.next(true);
createComponent();
});
it("debounces search text changes when not loading", fakeAsync(() => {
loading$.next(false);
component.searchText = "test";
component.onSearchTextChanged();
expect(applyFilter).not.toHaveBeenCalled();
tick(SearchTextDebounceInterval);
expect(applyFilter).toHaveBeenCalledWith("test");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
it("should not debounce search text changes when loading", fakeAsync(() => {
loading$.next(true);
component.searchText = "test";
component.onSearchTextChanged();
tick(0);
expect(applyFilter).toHaveBeenCalledWith("test");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
it("cancels previous debounce when new text is entered", fakeAsync(() => {
loading$.next(false);
component.searchText = "test";
component.onSearchTextChanged();
tick(SearchTextDebounceInterval / 2);
component.searchText = "test2";
component.onSearchTextChanged();
tick(SearchTextDebounceInterval / 2);
expect(applyFilter).not.toHaveBeenCalled();
tick(SearchTextDebounceInterval / 2);
expect(applyFilter).toHaveBeenCalledWith("test2");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
beforeEach(() => {
createComponent();
});
describe("when feature flag is disabled", () => {
beforeEach(() => {
featureFlag$.next(false);
createComponent();
});
it("debounces search text changes when not loading", fakeAsync(() => {
loading$.next(false);
it("debounces search text changes", fakeAsync(() => {
component.searchText = "test";
component.onSearchTextChanged();
component.searchText = "test";
component.onSearchTextChanged();
expect(applyFilter).not.toHaveBeenCalled();
expect(applyFilter).not.toHaveBeenCalled();
tick(SearchTextDebounceInterval);
tick(SearchTextDebounceInterval);
expect(applyFilter).toHaveBeenCalledWith("test");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
expect(applyFilter).toHaveBeenCalledWith("test");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
it("ignores loading state and always debounces", fakeAsync(() => {
loading$.next(true);
it("should not debounce search text changes when loading", fakeAsync(() => {
loading$.next(true);
component.searchText = "test";
component.onSearchTextChanged();
component.searchText = "test";
component.onSearchTextChanged();
expect(applyFilter).not.toHaveBeenCalled();
tick(0);
tick(SearchTextDebounceInterval);
expect(applyFilter).toHaveBeenCalledWith("test");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
expect(applyFilter).toHaveBeenCalledWith("test");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
});
it("cancels previous debounce when new text is entered", fakeAsync(() => {
loading$.next(false);
component.searchText = "test";
component.onSearchTextChanged();
tick(SearchTextDebounceInterval / 2);
component.searchText = "test2";
component.onSearchTextChanged();
tick(SearchTextDebounceInterval / 2);
expect(applyFilter).not.toHaveBeenCalled();
tick(SearchTextDebounceInterval / 2);
expect(applyFilter).toHaveBeenCalledWith("test2");
expect(applyFilter).toHaveBeenCalledTimes(1);
}));
});
});

View File

@@ -7,17 +7,13 @@ import {
Subscription,
combineLatest,
debounce,
debounceTime,
distinctUntilChanged,
filter,
map,
switchMap,
timer,
} from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { SearchTextDebounceInterval } from "@bitwarden/common/vault/services/search.service";
import { SearchModule } from "@bitwarden/components";
@@ -40,7 +36,6 @@ export class VaultV2SearchComponent {
constructor(
private vaultPopupItemsService: VaultPopupItemsService,
private vaultPopupLoadingService: VaultPopupLoadingService,
private configService: ConfigService,
private ngZone: NgZone,
) {
this.subscribeToLatestSearchText();
@@ -63,31 +58,19 @@ export class VaultV2SearchComponent {
}
subscribeToApplyFilter(): void {
this.configService
.getFeatureFlag$(FeatureFlag.VaultLoadingSkeletons)
combineLatest([this.searchText$, this.loading$])
.pipe(
switchMap((enabled) => {
if (!enabled) {
return this.searchText$.pipe(
debounceTime(SearchTextDebounceInterval),
distinctUntilChanged(),
);
}
return combineLatest([this.searchText$, this.loading$]).pipe(
debounce(([_, isLoading]) => {
// If loading apply immediately to avoid stale searches.
// After loading completes, debounce to avoid excessive searches.
const delayTime = isLoading ? 0 : SearchTextDebounceInterval;
return timer(delayTime);
}),
distinctUntilChanged(
([prevText, prevLoading], [newText, newLoading]) =>
prevText === newText && prevLoading === newLoading,
),
map(([text, _]) => text),
);
debounce(([_, isLoading]) => {
// If loading apply immediately to avoid stale searches.
// After loading completes, debounce to avoid excessive searches.
const delayTime = isLoading ? 0 : SearchTextDebounceInterval;
return timer(delayTime);
}),
distinctUntilChanged(
([prevText, prevLoading], [newText, newLoading]) =>
prevText === newText && prevLoading === newLoading,
),
map(([text, _]) => text),
takeUntilDestroyed(),
)
.subscribe((text) => {

View File

@@ -1,4 +1,4 @@
<popup-page [loading]="showSpinnerLoaders$ | async" [hideOverflow]="showSkeletonsLoaders$ | async">
<popup-page [hideOverflow]="showSkeletonsLoaders$ | async">
<popup-header slot="header" [pageTitle]="'vault' | i18n">
<ng-container slot="end">
<app-new-item-dropdown [initialValues]="newItemItemValues$ | async"></app-new-item-dropdown>
@@ -27,13 +27,9 @@
</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 {
<vault-fade-in-out *ngIf="vaultState === VaultStateEnum.Empty">
<ng-container *ngTemplateOutlet="emptyVaultTemplate"></ng-container>
}
</vault-fade-in-out>
<blocked-injection-banner
*ngIf="vaultState !== VaultStateEnum.Empty"
@@ -126,13 +122,9 @@
</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 {
<vault-fade-in-out *ngIf="vaultState === null">
<ng-container *ngTemplateOutlet="vaultContentTemplate"></ng-container>
}
</vault-fade-in-out>
</ng-container>
@if (showSkeletonsLoaders$ | async) {

View File

@@ -2,6 +2,7 @@ import { CdkVirtualScrollableElement } from "@angular/cdk/scrolling";
import { ChangeDetectionStrategy, Component, input, NO_ERRORS_SCHEMA } from "@angular/core";
import { TestBed, fakeAsync, flush, tick } from "@angular/core/testing";
import { By } from "@angular/platform-browser";
import { provideNoopAnimations } from "@angular/platform-browser/animations";
import { ActivatedRoute, Router } from "@angular/router";
import { RouterTestingModule } from "@angular/router/testing";
import { mock } from "jest-mock-extended";
@@ -244,6 +245,7 @@ describe("VaultV2Component", () => {
await TestBed.configureTestingModule({
imports: [VaultV2Component, RouterTestingModule],
providers: [
provideNoopAnimations(),
{ provide: VaultPopupItemsService, useValue: itemsSvc },
{ provide: VaultPopupListFiltersService, useValue: filtersSvc },
{ provide: VaultPopupScrollPositionService, useValue: scrollSvc },

View File

@@ -161,10 +161,6 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy {
}),
);
protected skeletonFeatureFlag$ = this.configService.getFeatureFlag$(
FeatureFlag.VaultLoadingSkeletons,
);
protected premiumSpotlightFeatureFlag$ = this.configService.getFeatureFlag$(
FeatureFlag.BrowserPremiumSpotlight,
);
@@ -219,20 +215,14 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy {
PremiumUpgradeDialogComponent.open(this.dialogService);
}
/** When true, show spinner loading state */
protected showSpinnerLoaders$ = 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.vaultItemsTransferService.transferInProgress$,
this.skeletonFeatureFlag$,
]).pipe(
map(([loading, cipherSearching, transferInProgress, skeletonsEnabled]) => {
return (loading || cipherSearching || transferInProgress) && skeletonsEnabled;
map(([loading, cipherSearching, transferInProgress]) => {
return loading || cipherSearching || transferInProgress;
}),
distinctUntilChanged(),
skeletonLoadingDelay(),

View File

@@ -62,7 +62,6 @@ export enum FeatureFlag {
PM22136_SdkCipherEncryption = "pm-22136-sdk-cipher-encryption",
CipherKeyEncryption = "cipher-key-encryption",
RiskInsightsForPremium = "pm-23904-risk-insights-for-premium",
VaultLoadingSkeletons = "pm-25081-vault-skeleton-loaders",
BrowserPremiumSpotlight = "pm-23384-browser-premium-spotlight",
MigrateMyVaultToMyItems = "pm-20558-migrate-myvault-to-myitems",
@@ -122,7 +121,6 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.PM22134SdkCipherListView]: FALSE,
[FeatureFlag.PM22136_SdkCipherEncryption]: FALSE,
[FeatureFlag.RiskInsightsForPremium]: FALSE,
[FeatureFlag.VaultLoadingSkeletons]: FALSE,
[FeatureFlag.BrowserPremiumSpotlight]: FALSE,
[FeatureFlag.MigrateMyVaultToMyItems]: FALSE,