From d61fd99063998dd0fb1de2bbd5165b1e8d04ff05 Mon Sep 17 00:00:00 2001 From: Nick Krantz Date: Tue, 20 Jan 2026 09:23:13 -0600 Subject: [PATCH] remove skeleton ff --- .../popup/send-v2/send-v2.component.html | 2 +- .../popup/send-v2/send-v2.component.spec.ts | 2 - .../tools/popup/send-v2/send-v2.component.ts | 18 +-- .../vault-v2-search.component.spec.ts | 123 ++++++------------ .../vault-search/vault-v2-search.component.ts | 39 ++---- .../vault-v2/vault-v2.component.html | 18 +-- .../vault-v2/vault-v2.component.spec.ts | 2 + .../components/vault-v2/vault-v2.component.ts | 14 +- libs/common/src/enums/feature-flag.enum.ts | 2 - 9 files changed, 61 insertions(+), 159 deletions(-) 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 47ecd7564dc..48295fda35d 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 @@ -1,4 +1,4 @@ - + diff --git a/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts b/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts index dfbfabf8d5e..dc4b935c6c8 100644 --- a/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts +++ b/apps/browser/src/tools/popup/send-v2/send-v2.component.spec.ts @@ -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() }, { provide: EnvironmentService, useValue: mock() }, { provide: LogService, useValue: mock() }, { provide: PlatformUtilsService, useValue: mock() }, 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 f36a475a805..8c1edee79dc 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 @@ -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([ diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts index 37c4804e600..ca73a7332ee 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.spec.ts @@ -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); + })); }); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts index 154cd49c5a3..3419bd30ea0 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts @@ -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) => { 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 6382b5fee0e..d6f52045a33 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 @@ -1,4 +1,4 @@ - + @@ -27,13 +27,9 @@ - @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 e6dffdaff08..2885b45c9b4 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 @@ -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 }, 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 761b366bcd2..6ff039a1c93 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 @@ -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(), diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 00de0d25ccd..ca207b08e9d 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -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,