diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts index 7ee15aa833b..13a312246fd 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.spec.ts @@ -21,12 +21,15 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; +import { DialogService, ToastService } from "@bitwarden/components"; import { CopyCipherFieldService } from "@bitwarden/vault"; import { BrowserApi } from "../../../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils"; import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service"; +import { VaultPopupScrollPositionService } from "../../../services/vault-popup-scroll-position.service"; import { VaultPopupAutofillService } from "./../../../services/vault-popup-autofill.service"; import { ViewV2Component } from "./view-v2.component"; @@ -44,6 +47,10 @@ describe("ViewV2Component", () => { const collect = jest.fn().mockResolvedValue(null); const doAutofill = jest.fn().mockResolvedValue(true); const copy = jest.fn().mockResolvedValue(true); + const back = jest.fn().mockResolvedValue(null); + const openSimpleDialog = jest.fn().mockResolvedValue(true); + const stop = jest.fn(); + const showToast = jest.fn(); const mockCipher = { id: "122-333-444", @@ -54,7 +61,7 @@ describe("ViewV2Component", () => { password: "test-password", totp: "123", }, - }; + } as unknown as CipherView; const mockVaultPopupAutofillService = { doAutofill, @@ -68,13 +75,21 @@ describe("ViewV2Component", () => { const mockCipherService = { get: jest.fn().mockResolvedValue({ decrypt: jest.fn().mockResolvedValue(mockCipher) }), getKeyForCipherKeyDecryption: jest.fn().mockResolvedValue({}), + deleteWithServer: jest.fn().mockResolvedValue(undefined), + softDeleteWithServer: jest.fn().mockResolvedValue(undefined), }; beforeEach(async () => { + mockCipherService.deleteWithServer.mockClear(); + mockCipherService.softDeleteWithServer.mockClear(); mockNavigate.mockClear(); collect.mockClear(); doAutofill.mockClear(); copy.mockClear(); + stop.mockClear(); + openSimpleDialog.mockClear(); + back.mockClear(); + showToast.mockClear(); await TestBed.configureTestingModule({ imports: [ViewV2Component], @@ -84,9 +99,12 @@ describe("ViewV2Component", () => { { provide: LogService, useValue: mock() }, { provide: PlatformUtilsService, useValue: mock() }, { provide: ConfigService, useValue: mock() }, - { provide: PopupRouterCacheService, useValue: mock() }, + { provide: PopupRouterCacheService, useValue: mock({ back }) }, { provide: ActivatedRoute, useValue: { queryParams: params$ } }, { provide: EventCollectionService, useValue: { collect } }, + { provide: VaultPopupScrollPositionService, useValue: { stop } }, + { provide: VaultPopupAutofillService, useValue: mockVaultPopupAutofillService }, + { provide: ToastService, useValue: { showToast } }, { provide: I18nService, useValue: { @@ -98,7 +116,6 @@ describe("ViewV2Component", () => { }, }, }, - { provide: VaultPopupAutofillService, useValue: mockVaultPopupAutofillService }, { provide: AccountService, useValue: accountService, @@ -114,7 +131,13 @@ describe("ViewV2Component", () => { useValue: mockCopyCipherFieldService, }, ], - }).compileComponents(); + }) + .overrideProvider(DialogService, { + useValue: { + openSimpleDialog, + }, + }) + .compileComponents(); fixture = TestBed.createComponent(ViewV2Component); component = fixture.componentInstance; @@ -223,4 +246,130 @@ describe("ViewV2Component", () => { expect(closeSpy).toHaveBeenCalledOnce(); })); }); + + describe("delete", () => { + beforeEach(() => { + component.cipher = mockCipher; + }); + + it("opens confirmation modal", async () => { + await component.delete(); + + expect(openSimpleDialog).toHaveBeenCalledOnce(); + }); + + it("navigates back", async () => { + await component.delete(); + + expect(back).toHaveBeenCalledOnce(); + }); + + it("stops scroll position service", async () => { + await component.delete(); + + expect(stop).toHaveBeenCalledOnce(); + expect(stop).toHaveBeenCalledWith(true); + }); + + describe("deny confirmation", () => { + beforeEach(() => { + openSimpleDialog.mockResolvedValue(false); + }); + + it("does not delete the cipher", async () => { + await component.delete(); + + expect(mockCipherService.deleteWithServer).not.toHaveBeenCalled(); + expect(mockCipherService.softDeleteWithServer).not.toHaveBeenCalled(); + }); + + it("does not interact with side effects", () => { + expect(back).not.toHaveBeenCalled(); + expect(stop).not.toHaveBeenCalled(); + expect(showToast).not.toHaveBeenCalled(); + }); + }); + + describe("accept confirmation", () => { + beforeEach(() => { + openSimpleDialog.mockResolvedValue(true); + }); + + describe("soft delete", () => { + beforeEach(() => { + (mockCipher as any).isDeleted = null; + }); + + it("opens confirmation dialog", async () => { + await component.delete(); + + expect(openSimpleDialog).toHaveBeenCalledOnce(); + expect(openSimpleDialog).toHaveBeenCalledWith({ + content: { + key: "deleteItemConfirmation", + }, + title: { + key: "deleteItem", + }, + type: "warning", + }); + }); + + it("calls soft delete", async () => { + await component.delete(); + + expect(mockCipherService.softDeleteWithServer).toHaveBeenCalled(); + expect(mockCipherService.deleteWithServer).not.toHaveBeenCalled(); + }); + + it("shows toast", async () => { + await component.delete(); + + expect(showToast).toHaveBeenCalledWith({ + variant: "success", + title: null, + message: "deletedItem", + }); + }); + }); + + describe("hard delete", () => { + beforeEach(() => { + (mockCipher as any).isDeleted = true; + }); + + it("opens confirmation dialog", async () => { + await component.delete(); + + expect(openSimpleDialog).toHaveBeenCalledOnce(); + expect(openSimpleDialog).toHaveBeenCalledWith({ + content: { + key: "permanentlyDeleteItemConfirmation", + }, + title: { + key: "deleteItem", + }, + type: "warning", + }); + }); + + it("calls soft delete", async () => { + await component.delete(); + + expect(mockCipherService.deleteWithServer).toHaveBeenCalled(); + expect(mockCipherService.softDeleteWithServer).not.toHaveBeenCalled(); + }); + + it("shows toast", async () => { + await component.delete(); + + expect(showToast).toHaveBeenCalledWith({ + variant: "success", + title: null, + message: "permanentlyDeletedItem", + }); + }); + }); + }); + }); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts index 55b59c087c5..434606548c4 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/view-v2/view-v2.component.ts @@ -45,6 +45,7 @@ import { PopOutComponent } from "../../../../../platform/popup/components/pop-ou import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service"; import { BrowserPremiumUpgradePromptService } from "../../../services/browser-premium-upgrade-prompt.service"; import { BrowserViewPasswordHistoryService } from "../../../services/browser-view-password-history.service"; +import { VaultPopupScrollPositionService } from "../../../services/vault-popup-scroll-position.service"; import { closeViewVaultItemPopout, VaultPopoutType } from "../../../utils/vault-popout-window"; import { PopupFooterComponent } from "./../../../../../platform/popup/layout/popup-footer.component"; @@ -109,6 +110,7 @@ export class ViewV2Component { private popupRouterCacheService: PopupRouterCacheService, protected cipherAuthorizationService: CipherAuthorizationService, private copyCipherFieldService: CopyCipherFieldService, + private popupScrollPositionService: VaultPopupScrollPositionService, ) { this.subscribeToParams(); } @@ -198,6 +200,7 @@ export class ViewV2Component { return false; } + this.popupScrollPositionService.stop(true); await this.popupRouterCacheService.back(); this.toastService.showToast({ diff --git a/apps/browser/src/vault/popup/components/vault/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault/vault-v2.component.ts index 68aa40cbf62..99cb137deff 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault/vault-v2.component.ts @@ -1,6 +1,6 @@ -import { ScrollingModule } from "@angular/cdk/scrolling"; +import { CdkVirtualScrollableElement, ScrollingModule } from "@angular/cdk/scrolling"; import { CommonModule } from "@angular/common"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { AfterViewInit, Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { RouterLink } from "@angular/router"; import { combineLatest, Observable, shareReplay, switchMap } from "rxjs"; @@ -17,6 +17,7 @@ import { PopupHeaderComponent } from "../../../../platform/popup/layout/popup-he import { PopupPageComponent } from "../../../../platform/popup/layout/popup-page.component"; 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"; import { VaultUiOnboardingService } from "../../services/vault-ui-onboarding.service"; import { AutofillVaultListItemsComponent, VaultListItemsContainerComponent } from "../vault-v2"; import { @@ -53,7 +54,9 @@ enum VaultState { ], providers: [VaultUiOnboardingService], }) -export class VaultV2Component implements OnInit, OnDestroy { +export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { + @ViewChild(CdkVirtualScrollableElement) virtualScrollElement?: CdkVirtualScrollableElement; + cipherType = CipherType; protected favoriteCiphers$ = this.vaultPopupItemsService.favoriteCiphers$; @@ -87,6 +90,7 @@ export class VaultV2Component implements OnInit, OnDestroy { private vaultPopupItemsService: VaultPopupItemsService, private vaultPopupListFiltersService: VaultPopupListFiltersService, private vaultUiOnboardingService: VaultUiOnboardingService, + private vaultScrollPositionService: VaultPopupScrollPositionService, ) { combineLatest([ this.vaultPopupItemsService.emptyVault$, @@ -112,9 +116,17 @@ export class VaultV2Component implements OnInit, OnDestroy { }); } + ngAfterViewInit(): void { + if (this.virtualScrollElement) { + this.vaultScrollPositionService.start(this.virtualScrollElement); + } + } + async ngOnInit() { await this.vaultUiOnboardingService.showOnboardingDialog(); } - ngOnDestroy(): void {} + ngOnDestroy(): void { + this.vaultScrollPositionService.stop(); + } } diff --git a/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts new file mode 100644 index 00000000000..6f3c72ce68c --- /dev/null +++ b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.spec.ts @@ -0,0 +1,134 @@ +import { CdkVirtualScrollableElement } from "@angular/cdk/scrolling"; +import { fakeAsync, TestBed, tick } from "@angular/core/testing"; +import { NavigationEnd, Router } from "@angular/router"; +import { Subject, Subscription } from "rxjs"; + +import { VaultPopupScrollPositionService } from "./vault-popup-scroll-position.service"; + +describe("VaultPopupScrollPositionService", () => { + let service: VaultPopupScrollPositionService; + const events$ = new Subject(); + const unsubscribe = jest.fn(); + + beforeEach(async () => { + unsubscribe.mockClear(); + + await TestBed.configureTestingModule({ + providers: [ + VaultPopupScrollPositionService, + { provide: Router, useValue: { events: events$ } }, + ], + }); + + service = TestBed.inject(VaultPopupScrollPositionService); + + // set up dummy values + service["scrollPosition"] = 234; + service["scrollSubscription"] = { unsubscribe } as unknown as Subscription; + }); + + describe("router events", () => { + it("does not reset service when navigating to `/tabs/vault`", fakeAsync(() => { + const event = new NavigationEnd(22, "/tabs/vault", ""); + events$.next(event); + + tick(); + + expect(service["scrollPosition"]).toBe(234); + expect(service["scrollSubscription"]).not.toBeNull(); + })); + + it("resets values when navigating to other tab pages", fakeAsync(() => { + const event = new NavigationEnd(23, "/tabs/generator", ""); + events$.next(event); + + tick(); + + expect(service["scrollPosition"]).toBeNull(); + expect(unsubscribe).toHaveBeenCalled(); + expect(service["scrollSubscription"]).toBeNull(); + })); + }); + + describe("stop", () => { + it("removes scroll listener", () => { + service.stop(); + + expect(unsubscribe).toHaveBeenCalledOnce(); + expect(service["scrollSubscription"]).toBeNull(); + }); + + it("resets stored values", () => { + service.stop(true); + + expect(service["scrollPosition"]).toBeNull(); + }); + }); + + describe("start", () => { + const elementScrolled$ = new Subject(); + const focus = jest.fn(); + const nativeElement = { + scrollTop: 0, + querySelector: jest.fn(() => ({ focus })), + addEventListener: jest.fn(), + }; + const virtualElement = { + elementScrolled: () => elementScrolled$, + getElementRef: () => ({ nativeElement }), + scrollTo: jest.fn(), + } as unknown as CdkVirtualScrollableElement; + + afterEach(() => { + // remove the actual subscription created by `.subscribe` + service["scrollSubscription"]?.unsubscribe(); + }); + + describe("initial scroll position", () => { + beforeEach(() => { + (virtualElement.scrollTo as jest.Mock).mockClear(); + nativeElement.querySelector.mockClear(); + }); + + it("does not scroll when `scrollPosition` is null", () => { + service["scrollPosition"] = null; + + service.start(virtualElement); + + expect(virtualElement.scrollTo).not.toHaveBeenCalled(); + }); + + it("scrolls the virtual element to `scrollPosition`", fakeAsync(() => { + service["scrollPosition"] = 500; + nativeElement.scrollTop = 500; + + service.start(virtualElement); + tick(); + + expect(virtualElement.scrollTo).toHaveBeenCalledWith({ behavior: "instant", top: 500 }); + })); + }); + + describe("scroll listener", () => { + it("unsubscribes from any existing subscription", () => { + service.start(virtualElement); + + expect(unsubscribe).toHaveBeenCalled(); + }); + + it("subscribes to `elementScrolled`", fakeAsync(() => { + virtualElement.measureScrollOffset = jest.fn(() => 455); + + service.start(virtualElement); + + elementScrolled$.next(null); // first subscription is skipped by `skip(1)` + elementScrolled$.next(null); + tick(); + + expect(virtualElement.measureScrollOffset).toHaveBeenCalledOnce(); + expect(virtualElement.measureScrollOffset).toHaveBeenCalledWith("top"); + expect(service["scrollPosition"]).toBe(455); + })); + }); + }); +}); diff --git a/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts new file mode 100644 index 00000000000..39ded764a22 --- /dev/null +++ b/apps/browser/src/vault/popup/services/vault-popup-scroll-position.service.ts @@ -0,0 +1,77 @@ +import { CdkVirtualScrollableElement } from "@angular/cdk/scrolling"; +import { inject, Injectable } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { NavigationEnd, Router } from "@angular/router"; +import { filter, skip, Subscription } from "rxjs"; + +@Injectable({ + providedIn: "root", +}) +export class VaultPopupScrollPositionService { + private router = inject(Router); + + /** Path of the vault screen */ + private readonly vaultPath = "/tabs/vault"; + + /** Current scroll position relative to the top of the viewport. */ + private scrollPosition: number | null = null; + + /** Subscription associated with the virtual scroll element. */ + private scrollSubscription: Subscription | null = null; + + constructor() { + this.router.events + .pipe( + takeUntilDestroyed(), + filter((event): event is NavigationEnd => event instanceof NavigationEnd), + ) + .subscribe((event) => { + this.resetListenerForNavigation(event); + }); + } + + /** Scrolls the user to the stored scroll position and starts tracking scroll of the page. */ + start(virtualScrollElement: CdkVirtualScrollableElement) { + if (this.scrollPosition) { + // Use `setTimeout` to scroll after rendering is complete + setTimeout(async () => { + // `?? 0` is only to make typescript happy. It shouldn't happen with the above truthy check for `this.scrollPosition`. + virtualScrollElement.scrollTo({ top: this.scrollPosition ?? 0, behavior: "instant" }); + }); + } + + this.scrollSubscription?.unsubscribe(); + + // Skip the first scroll event to avoid settings the scroll from the above `scrollTo` call + this.scrollSubscription = virtualScrollElement + ?.elementScrolled() + .pipe(skip(1)) + .subscribe(() => { + const offset = virtualScrollElement.measureScrollOffset("top"); + this.scrollPosition = offset; + }); + } + + /** Stops the scroll listener from updating the stored location. */ + stop(reset?: true) { + this.scrollSubscription?.unsubscribe(); + this.scrollSubscription = null; + + if (reset) { + this.scrollPosition = null; + } + } + + /** Conditionally resets the scroll listeners based on the ending path of the navigation */ + private resetListenerForNavigation(event: NavigationEnd): void { + // The vault page is the target of the scroll listener, return early + if (event.url === this.vaultPath) { + return; + } + + // For all other tab pages reset the scroll position + if (event.url.startsWith("/tabs/")) { + this.stop(true); + } + } +}