diff --git a/apps/desktop/src/app/layout/desktop-layout.component.html b/apps/desktop/src/app/layout/desktop-layout.component.html index 7f8bd265102..1717b29acd1 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.html +++ b/apps/desktop/src/app/layout/desktop-layout.component.html @@ -3,7 +3,7 @@ - + diff --git a/apps/desktop/src/app/layout/desktop-layout.component.spec.ts b/apps/desktop/src/app/layout/desktop-layout.component.spec.ts index cc2f7e58dfb..74cddd02495 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.spec.ts +++ b/apps/desktop/src/app/layout/desktop-layout.component.spec.ts @@ -1,3 +1,4 @@ +import { ChangeDetectionStrategy, Component } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { RouterModule } from "@angular/router"; import { mock } from "jest-mock-extended"; @@ -5,8 +6,18 @@ import { mock } from "jest-mock-extended"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { NavigationModule } from "@bitwarden/components"; +import { SendFiltersNavComponent } from "../tools/send-v2/send-filters-nav.component"; + import { DesktopLayoutComponent } from "./desktop-layout.component"; +// Mock the child component to isolate DesktopLayoutComponent testing +@Component({ + selector: "app-send-filters-nav", + template: "", + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockSendFiltersNavComponent {} + Object.defineProperty(window, "matchMedia", { writable: true, value: jest.fn().mockImplementation((query) => ({ @@ -34,7 +45,12 @@ describe("DesktopLayoutComponent", () => { useValue: mock(), }, ], - }).compileComponents(); + }) + .overrideComponent(DesktopLayoutComponent, { + remove: { imports: [SendFiltersNavComponent] }, + add: { imports: [MockSendFiltersNavComponent] }, + }) + .compileComponents(); fixture = TestBed.createComponent(DesktopLayoutComponent); component = fixture.componentInstance; @@ -58,4 +74,11 @@ describe("DesktopLayoutComponent", () => { expect(ngContent).toBeTruthy(); }); + + it("renders send filters navigation component", () => { + const compiled = fixture.nativeElement; + const sendFiltersNav = compiled.querySelector("app-send-filters-nav"); + + expect(sendFiltersNav).toBeTruthy(); + }); }); diff --git a/apps/desktop/src/app/layout/desktop-layout.component.ts b/apps/desktop/src/app/layout/desktop-layout.component.ts index 006055f475f..0ee7065fba8 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.ts +++ b/apps/desktop/src/app/layout/desktop-layout.component.ts @@ -5,13 +5,22 @@ import { PasswordManagerLogo } from "@bitwarden/assets/svg"; import { LayoutComponent, NavigationModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; +import { SendFiltersNavComponent } from "../tools/send-v2/send-filters-nav.component"; + import { DesktopSideNavComponent } from "./desktop-side-nav.component"; // FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-layout", - imports: [RouterModule, I18nPipe, LayoutComponent, NavigationModule, DesktopSideNavComponent], + imports: [ + RouterModule, + I18nPipe, + LayoutComponent, + NavigationModule, + DesktopSideNavComponent, + SendFiltersNavComponent, + ], templateUrl: "./desktop-layout.component.html", }) export class DesktopLayoutComponent { diff --git a/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.html b/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.html new file mode 100644 index 00000000000..64c52b50a49 --- /dev/null +++ b/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.html @@ -0,0 +1,25 @@ + + + + + diff --git a/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.spec.ts b/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.spec.ts new file mode 100644 index 00000000000..95ba5c53e36 --- /dev/null +++ b/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.spec.ts @@ -0,0 +1,204 @@ +import { ChangeDetectionStrategy, Component } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { Router, provideRouter } from "@angular/router"; +import { RouterTestingHarness } from "@angular/router/testing"; +import { BehaviorSubject } from "rxjs"; + +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; +import { NavigationModule } from "@bitwarden/components"; +import { SendListFiltersService } from "@bitwarden/send-ui"; + +import { SendFiltersNavComponent } from "./send-filters-nav.component"; + +@Component({ template: "", changeDetection: ChangeDetectionStrategy.OnPush }) +class DummyComponent {} + +Object.defineProperty(window, "matchMedia", { + writable: true, + value: jest.fn().mockImplementation((query) => ({ + matches: true, + media: query, + onchange: null, + addListener: jest.fn(), + removeListener: jest.fn(), + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), +}); + +describe("SendFiltersNavComponent", () => { + let component: SendFiltersNavComponent; + let fixture: ComponentFixture; + let harness: RouterTestingHarness; + let filterFormValueSubject: BehaviorSubject<{ sendType: SendType | null }>; + let mockSendListFiltersService: Partial; + + beforeEach(async () => { + filterFormValueSubject = new BehaviorSubject<{ sendType: SendType | null }>({ + sendType: null, + }); + + mockSendListFiltersService = { + filterForm: { + value: { sendType: null }, + valueChanges: filterFormValueSubject.asObservable(), + patchValue: jest.fn((value) => { + mockSendListFiltersService.filterForm.value = { + ...mockSendListFiltersService.filterForm.value, + ...value, + }; + filterFormValueSubject.next(mockSendListFiltersService.filterForm.value); + }), + } as any, + filters$: filterFormValueSubject.asObservable(), + }; + + await TestBed.configureTestingModule({ + imports: [SendFiltersNavComponent, NavigationModule], + providers: [ + provideRouter([ + { path: "vault", component: DummyComponent }, + { path: "new-sends", component: DummyComponent }, + ]), + { + provide: SendListFiltersService, + useValue: mockSendListFiltersService, + }, + { + provide: I18nService, + useValue: { + t: jest.fn((key) => key), + }, + }, + ], + }).compileComponents(); + + // Create harness and navigate to initial route + harness = await RouterTestingHarness.create("/vault"); + + // Create the component fixture separately (not a routed component) + fixture = TestBed.createComponent(SendFiltersNavComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("creates component", () => { + expect(component).toBeTruthy(); + }); + + it("renders bit-nav-group with Send icon and text", () => { + const compiled = fixture.nativeElement; + const navGroup = compiled.querySelector("bit-nav-group"); + + expect(navGroup).toBeTruthy(); + expect(navGroup.getAttribute("icon")).toBe("bwi-send"); + }); + + it("component exposes SendType enum for template", () => { + expect(component["SendType"]).toBe(SendType); + }); + + describe("isSendRouteActive", () => { + it("returns true when on /new-sends route", async () => { + await harness.navigateByUrl("/new-sends"); + fixture.detectChanges(); + + expect(component["isSendRouteActive"]()).toBe(true); + }); + + it("returns false when not on /new-sends route", () => { + expect(component["isSendRouteActive"]()).toBe(false); + }); + }); + + describe("activeSendType", () => { + it("returns the active send type when on send route and filter type is set", async () => { + await harness.navigateByUrl("/new-sends"); + mockSendListFiltersService.filterForm.value = { sendType: SendType.Text }; + filterFormValueSubject.next({ sendType: SendType.Text }); + fixture.detectChanges(); + + expect(component["activeSendType"]()).toBe(SendType.Text); + }); + + it("returns undefined when not on send route", () => { + mockSendListFiltersService.filterForm.value = { sendType: SendType.Text }; + filterFormValueSubject.next({ sendType: SendType.Text }); + fixture.detectChanges(); + + expect(component["activeSendType"]()).toBeUndefined(); + }); + + it("returns null when on send route but no type is selected", async () => { + await harness.navigateByUrl("/new-sends"); + mockSendListFiltersService.filterForm.value = { sendType: null }; + filterFormValueSubject.next({ sendType: null }); + fixture.detectChanges(); + + expect(component["activeSendType"]()).toBeNull(); + }); + }); + + describe("selectTypeAndNavigate", () => { + it("clears the sendType filter when called with no parameter", async () => { + await component["selectTypeAndNavigate"](); + + expect(mockSendListFiltersService.filterForm.patchValue).toHaveBeenCalledWith({ + sendType: null, + }); + }); + + it("updates filter form with Text type", async () => { + await component["selectTypeAndNavigate"](SendType.Text); + + expect(mockSendListFiltersService.filterForm.patchValue).toHaveBeenCalledWith({ + sendType: SendType.Text, + }); + }); + + it("updates filter form with File type", async () => { + await component["selectTypeAndNavigate"](SendType.File); + + expect(mockSendListFiltersService.filterForm.patchValue).toHaveBeenCalledWith({ + sendType: SendType.File, + }); + }); + + it("navigates to /new-sends when not on send route", async () => { + expect(harness.routeNativeElement?.textContent).toBeDefined(); + + await component["selectTypeAndNavigate"](SendType.Text); + + const currentUrl = TestBed.inject(Router).url; + expect(currentUrl).toBe("/new-sends"); + expect(mockSendListFiltersService.filterForm.patchValue).toHaveBeenCalledWith({ + sendType: SendType.Text, + }); + }); + + it("does not navigate when already on send route (component is reactive)", async () => { + await harness.navigateByUrl("/new-sends"); + const router = TestBed.inject(Router); + const navigateSpy = jest.spyOn(router, "navigate"); + + await component["selectTypeAndNavigate"](SendType.Text); + + expect(navigateSpy).not.toHaveBeenCalled(); + expect(mockSendListFiltersService.filterForm.patchValue).toHaveBeenCalledWith({ + sendType: SendType.Text, + }); + }); + + it("navigates when clearing filter from different route", async () => { + await component["selectTypeAndNavigate"](); // No parameter = clear filter + + const currentUrl = TestBed.inject(Router).url; + expect(currentUrl).toBe("/new-sends"); + expect(mockSendListFiltersService.filterForm.patchValue).toHaveBeenCalledWith({ + sendType: null, + }); + }); + }); +}); diff --git a/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.ts b/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.ts new file mode 100644 index 00000000000..28004f475e5 --- /dev/null +++ b/apps/desktop/src/app/tools/send-v2/send-filters-nav.component.ts @@ -0,0 +1,54 @@ +import { CommonModule } from "@angular/common"; +import { ChangeDetectionStrategy, Component, computed, inject } from "@angular/core"; +import { toSignal } from "@angular/core/rxjs-interop"; +import { NavigationEnd, Router } from "@angular/router"; +import { filter, map, startWith } from "rxjs"; + +import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; +import { NavigationModule } from "@bitwarden/components"; +import { SendListFiltersService } from "@bitwarden/send-ui"; +import { I18nPipe } from "@bitwarden/ui-common"; + +/** + * Navigation component that renders Send filter options in the sidebar. + * Fully reactive using signals - no manual subscriptions or method-based computed values. + * - Parent "Send" nav-group clears filter (shows all sends) + * - Child "Text"/"File" items set filter to specific type + * - Active states computed reactively from filter signal + route signal + */ +@Component({ + selector: "app-send-filters-nav", + templateUrl: "./send-filters-nav.component.html", + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [CommonModule, NavigationModule, I18nPipe], +}) +export class SendFiltersNavComponent { + protected readonly SendType = SendType; + private readonly filtersService = inject(SendListFiltersService); + private readonly router = inject(Router); + private readonly currentFilter = toSignal(this.filtersService.filters$); + + // Track whether current route is the send route + private readonly isSendRouteActive = toSignal( + this.router.events.pipe( + filter((event) => event instanceof NavigationEnd), + map((event) => (event as NavigationEnd).urlAfterRedirects.includes("/new-sends")), + startWith(this.router.url.includes("/new-sends")), + ), + { initialValue: this.router.url.includes("/new-sends") }, + ); + + // Computed: Active send type (null when on send route with no filter, undefined when not on send route) + protected readonly activeSendType = computed(() => { + return this.isSendRouteActive() ? this.currentFilter()?.sendType : undefined; + }); + + // Update send filter and navigate to /new-sends (only if not already there - send-v2 component reacts to filter changes) + protected async selectTypeAndNavigate(type?: SendType): Promise { + this.filtersService.filterForm.patchValue({ sendType: type !== undefined ? type : null }); + + if (!this.router.url.includes("/new-sends")) { + await this.router.navigate(["/new-sends"]); + } + } +} diff --git a/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts b/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts index 5798df0989d..8657f3e375e 100644 --- a/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts +++ b/apps/desktop/src/app/tools/send-v2/send-v2.component.spec.ts @@ -1,4 +1,8 @@ +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore +import { ChangeDetectorRef } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormBuilder } from "@angular/forms"; import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, of } from "rxjs"; @@ -15,6 +19,7 @@ import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.s import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { DialogService, ToastService } from "@bitwarden/components"; +import { SendListFiltersService } from "@bitwarden/send-ui"; import * as utils from "../../../utils"; import { SearchBarService } from "../../layout/search/search-bar.service"; @@ -35,6 +40,8 @@ describe("SendV2Component", () => { let broadcasterService: MockProxy; let accountService: MockProxy; let policyService: MockProxy; + let sendListFiltersService: SendListFiltersService; + let changeDetectorRef: MockProxy; beforeEach(async () => { sendService = mock(); @@ -42,6 +49,13 @@ describe("SendV2Component", () => { broadcasterService = mock(); accountService = mock(); policyService = mock(); + changeDetectorRef = mock(); + + // Create real SendListFiltersService with mocked dependencies + const formBuilder = new FormBuilder(); + const i18nService = mock(); + i18nService.t.mockImplementation((key: string) => key); + sendListFiltersService = new SendListFiltersService(i18nService, formBuilder); // Mock sendViews$ observable sendService.sendViews$ = of([]); @@ -51,6 +65,10 @@ describe("SendV2Component", () => { accountService.activeAccount$ = of({ id: "test-user-id" } as any); policyService.policyAppliesToUser$ = jest.fn().mockReturnValue(of(false)); + // Mock SearchService methods needed by base component + const mockSearchService = mock(); + mockSearchService.isSearchable.mockResolvedValue(false); + await TestBed.configureTestingModule({ imports: [SendV2Component], providers: [ @@ -59,7 +77,7 @@ describe("SendV2Component", () => { { provide: PlatformUtilsService, useValue: mock() }, { provide: EnvironmentService, useValue: mock() }, { provide: BroadcasterService, useValue: broadcasterService }, - { provide: SearchService, useValue: mock() }, + { provide: SearchService, useValue: mockSearchService }, { provide: PolicyService, useValue: policyService }, { provide: SearchBarService, useValue: searchBarService }, { provide: LogService, useValue: mock() }, @@ -67,6 +85,8 @@ describe("SendV2Component", () => { { provide: DialogService, useValue: mock() }, { provide: ToastService, useValue: mock() }, { provide: AccountService, useValue: accountService }, + { provide: SendListFiltersService, useValue: sendListFiltersService }, + { provide: ChangeDetectorRef, useValue: changeDetectorRef }, ], }).compileComponents(); @@ -331,7 +351,6 @@ describe("SendV2Component", () => { describe("load", () => { it("sets loading states correctly", async () => { jest.spyOn(component, "search").mockResolvedValue(); - jest.spyOn(component, "selectAll"); expect(component.loaded).toBeFalsy(); @@ -341,14 +360,17 @@ describe("SendV2Component", () => { expect(component.loaded).toBe(true); }); - it("calls selectAll when onSuccessfulLoad is not set", async () => { + it("sets up sendViews$ subscription", async () => { + const mockSends = [new SendView(), new SendView()]; + sendService.sendViews$ = of(mockSends); jest.spyOn(component, "search").mockResolvedValue(); - jest.spyOn(component, "selectAll"); - component.onSuccessfulLoad = null; await component.load(); - expect(component.selectAll).toHaveBeenCalled(); + // Give observable time to emit + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(component.sends).toEqual(mockSends); }); it("calls onSuccessfulLoad when it is set", async () => { diff --git a/apps/desktop/src/app/tools/send-v2/send-v2.component.ts b/apps/desktop/src/app/tools/send-v2/send-v2.component.ts index 4afe02d9f98..eb0856b76af 100644 --- a/apps/desktop/src/app/tools/send-v2/send-v2.component.ts +++ b/apps/desktop/src/app/tools/send-v2/send-v2.component.ts @@ -2,8 +2,9 @@ // @ts-strict-ignore import { CommonModule } from "@angular/common"; import { Component, OnInit, OnDestroy, ViewChild, NgZone, ChangeDetectorRef } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormsModule } from "@angular/forms"; -import { mergeMap } from "rxjs"; +import { mergeMap, Subscription } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { SendComponent as BaseSendComponent } from "@bitwarden/angular/tools/send/send.component"; @@ -14,11 +15,13 @@ import { EnvironmentService } from "@bitwarden/common/platform/abstractions/envi import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction"; import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { DialogService, ToastService } from "@bitwarden/components"; +import { SendListFiltersService } from "@bitwarden/send-ui"; import { invokeMenu, RendererMenuItem } from "../../../utils"; import { SearchBarService } from "../../layout/search/search-bar.service"; @@ -55,6 +58,9 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest // Tracks the current UI state: viewing list (None), adding new Send (Add), or editing existing Send (Edit) action: Action = Action.None; + // Subscription for sendViews$ cleanup + private sendViewsSubscription: Subscription; + constructor( sendService: SendService, i18nService: I18nService, @@ -71,6 +77,7 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest toastService: ToastService, accountService: AccountService, private cdr: ChangeDetectorRef, + private sendListFiltersService: SendListFiltersService, ) { super( sendService, @@ -88,12 +95,17 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest ); // Listen to search bar changes and update the Send list filter - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - this.searchBarService.searchText$.subscribe((searchText) => { + this.searchBarService.searchText$.pipe(takeUntilDestroyed()).subscribe((searchText) => { this.searchText = searchText; this.searchTextChanged(); - setTimeout(() => this.cdr.detectChanges(), 250); }); + + // Listen to filter changes from sidebar navigation + this.sendListFiltersService.filterForm.valueChanges + .pipe(takeUntilDestroyed()) + .subscribe((filters) => { + this.applySendTypeFilter(filters); + }); } // Initialize the component: enable search bar, subscribe to sync events, and load Send items @@ -103,6 +115,10 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest await super.ngOnInit(); + // Read current filter synchronously to avoid race condition on navigation + const currentFilter = this.sendListFiltersService.filterForm.value; + this.applySendTypeFilter(currentFilter); + // Listen for sync completion events to refresh the Send list this.broadcasterService.subscribe(BroadcasterSubscriptionId, (message: any) => { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. @@ -118,8 +134,18 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest await this.load(); } + // Apply send type filter to display: centralized logic for initial load and filter changes + private applySendTypeFilter(filters: Partial<{ sendType: SendType | null }>): void { + if (filters.sendType === null || filters.sendType === undefined) { + this.selectAll(); + } else { + this.selectType(filters.sendType); + } + } + // Clean up subscriptions and disable search bar when component is destroyed ngOnDestroy() { + this.sendViewsSubscription?.unsubscribe(); this.broadcasterService.unsubscribe(BroadcasterSubscriptionId); this.searchBarService.setEnabled(false); } @@ -130,7 +156,12 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest // Note: The filter parameter is ignored in this implementation for desktop-specific behavior. async load(filter: (send: SendView) => boolean = null) { this.loading = true; - this.sendService.sendViews$ + + // Recreate subscription on each load (required for sync refresh) + // Manual cleanup in ngOnDestroy is intentional - load() is called multiple times + this.sendViewsSubscription?.unsubscribe(); + + this.sendViewsSubscription = this.sendService.sendViews$ .pipe( mergeMap(async (sends) => { this.sends = sends; @@ -143,9 +174,6 @@ export class SendV2Component extends BaseSendComponent implements OnInit, OnDest .subscribe(); if (this.onSuccessfulLoad != null) { await this.onSuccessfulLoad(); - } else { - // Default action - this.selectAll(); } this.loading = false; this.loaded = true;