1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-19 09:43:23 +00:00

Feature/pm 27795 migrate send filters desktop migration (#17802)

Created a new navigation component that renders Send type filters as sidebar navigation items.
This commit is contained in:
Isaac Ivins
2025-12-10 04:02:30 -05:00
committed by GitHub
parent 3e9db6b472
commit 663ef60ae5
8 changed files with 382 additions and 17 deletions

View File

@@ -3,7 +3,7 @@
<bit-nav-logo [openIcon]="logo" route="." [label]="'passwordManager' | i18n"></bit-nav-logo>
<bit-nav-item icon="bwi-vault" [text]="'vault' | i18n" route="new-vault"></bit-nav-item>
<bit-nav-item icon="bwi-send" [text]="'send' | i18n" route="new-sends"></bit-nav-item>
<app-send-filters-nav></app-send-filters-nav>
</app-side-nav>
<router-outlet></router-outlet>

View File

@@ -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<I18nService>(),
},
],
}).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();
});
});

View File

@@ -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 {

View File

@@ -0,0 +1,25 @@
<bit-nav-group
icon="bwi-send"
[text]="'send' | i18n"
route="new-sends"
(click)="selectTypeAndNavigate()"
>
<bit-nav-item
icon="bwi-send"
[text]="'allSends' | i18n"
(click)="selectTypeAndNavigate(null); $event.stopPropagation()"
[forceActiveStyles]="activeSendType() === null"
></bit-nav-item>
<bit-nav-item
icon="bwi-file-text"
[text]="'sendTypeText' | i18n"
(click)="selectTypeAndNavigate(SendType.Text); $event.stopPropagation()"
[forceActiveStyles]="activeSendType() === SendType.Text"
></bit-nav-item>
<bit-nav-item
icon="bwi-file"
[text]="'sendTypeFile' | i18n"
(click)="selectTypeAndNavigate(SendType.File); $event.stopPropagation()"
[forceActiveStyles]="activeSendType() === SendType.File"
></bit-nav-item>
</bit-nav-group>

View File

@@ -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<SendFiltersNavComponent>;
let harness: RouterTestingHarness;
let filterFormValueSubject: BehaviorSubject<{ sendType: SendType | null }>;
let mockSendListFiltersService: Partial<SendListFiltersService>;
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,
});
});
});
});

View File

@@ -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<void> {
this.filtersService.filterForm.patchValue({ sendType: type !== undefined ? type : null });
if (!this.router.url.includes("/new-sends")) {
await this.router.navigate(["/new-sends"]);
}
}
}

View File

@@ -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<BroadcasterService>;
let accountService: MockProxy<AccountService>;
let policyService: MockProxy<PolicyService>;
let sendListFiltersService: SendListFiltersService;
let changeDetectorRef: MockProxy<ChangeDetectorRef>;
beforeEach(async () => {
sendService = mock<SendService>();
@@ -42,6 +49,13 @@ describe("SendV2Component", () => {
broadcasterService = mock<BroadcasterService>();
accountService = mock<AccountService>();
policyService = mock<PolicyService>();
changeDetectorRef = mock<ChangeDetectorRef>();
// Create real SendListFiltersService with mocked dependencies
const formBuilder = new FormBuilder();
const i18nService = mock<I18nService>();
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<SearchService>();
mockSearchService.isSearchable.mockResolvedValue(false);
await TestBed.configureTestingModule({
imports: [SendV2Component],
providers: [
@@ -59,7 +77,7 @@ describe("SendV2Component", () => {
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: EnvironmentService, useValue: mock<EnvironmentService>() },
{ provide: BroadcasterService, useValue: broadcasterService },
{ provide: SearchService, useValue: mock<SearchService>() },
{ provide: SearchService, useValue: mockSearchService },
{ provide: PolicyService, useValue: policyService },
{ provide: SearchBarService, useValue: searchBarService },
{ provide: LogService, useValue: mock<LogService>() },
@@ -67,6 +85,8 @@ describe("SendV2Component", () => {
{ provide: DialogService, useValue: mock<DialogService>() },
{ provide: ToastService, useValue: mock<ToastService>() },
{ 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 () => {

View File

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