From c1bf1a797feff230c7499cec15df113ff448b806 Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Thu, 8 Aug 2024 11:29:33 -0400 Subject: [PATCH] [PM-9714] Search results should clear and results reset after navigating away from Vault tab but persist if navigating to an Item view (#10378) * created guard to clear search text when navigating between tabs * removed reset filter from from vault list filter component on destroy and move to guard renamed guard to clear vault state * Fixed bug on chip select when comparing complex objects moved compare values function to utils * Added comment for future reference * moved compare values to a seperate file * fixed lint issue --- .../local-backed-session-storage.service.ts | 19 ++-------- apps/browser/src/popup/app-routing.module.ts | 2 ++ .../vault/guards/clear-vault-state.guard.ts | 28 +++++++++++++++ .../vault-list-filters.component.ts | 8 ++--- .../src/platform/misc/compare-values.spec.ts | 35 +++++++++++++++++++ .../src/platform/misc/compare-values.ts | 27 ++++++++++++++ .../src/chip-select/chip-select.component.ts | 3 +- 7 files changed, 98 insertions(+), 24 deletions(-) create mode 100644 apps/browser/src/vault/guards/clear-vault-state.guard.ts create mode 100644 libs/common/src/platform/misc/compare-values.spec.ts create mode 100644 libs/common/src/platform/misc/compare-values.ts diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index 2c14ac2833..28abb78b19 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -8,6 +8,7 @@ import { ObservableStorageService, StorageUpdate, } from "@bitwarden/common/platform/abstractions/storage.service"; +import { compareValues } from "@bitwarden/common/platform/misc/compare-values"; import { Lazy } from "@bitwarden/common/platform/misc/lazy"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; @@ -190,23 +191,7 @@ export class LocalBackedSessionStorageService private compareValues(value1: T, value2: T): boolean { try { - if (value1 == null && value2 == null) { - return true; - } - - if (value1 && value2 == null) { - return false; - } - - if (value1 == null && value2) { - return false; - } - - if (typeof value1 !== "object" || typeof value2 !== "object") { - return value1 === value2; - } - - return JSON.stringify(value1) === JSON.stringify(value2); + return compareValues(value1, value2); } catch (e) { this.logService.error( `error comparing values\n${JSON.stringify(value1)}\n${JSON.stringify(value2)}`, diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index e556e45928..9f13ab57d9 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -64,6 +64,7 @@ import { ImportBrowserV2Component } from "../tools/popup/settings/import/import- import { ImportBrowserComponent } from "../tools/popup/settings/import/import-browser.component"; import { SettingsV2Component } from "../tools/popup/settings/settings-v2.component"; import { SettingsComponent } from "../tools/popup/settings/settings.component"; +import { clearVaultStateGuard } from "../vault/guards/clear-vault-state.guard"; import { AddEditComponent } from "../vault/popup/components/vault/add-edit.component"; import { AttachmentsComponent } from "../vault/popup/components/vault/attachments.component"; import { CollectionsComponent } from "../vault/popup/components/vault/collections.component"; @@ -457,6 +458,7 @@ const routes: Routes = [ ...extensionRefreshSwap(VaultFilterComponent, VaultV2Component, { path: "vault", canActivate: [authGuard], + canDeactivate: [clearVaultStateGuard], data: { state: "tabs_vault" }, }), { diff --git a/apps/browser/src/vault/guards/clear-vault-state.guard.ts b/apps/browser/src/vault/guards/clear-vault-state.guard.ts new file mode 100644 index 0000000000..2b43f1ecbd --- /dev/null +++ b/apps/browser/src/vault/guards/clear-vault-state.guard.ts @@ -0,0 +1,28 @@ +import { inject } from "@angular/core"; +import { CanDeactivateFn } from "@angular/router"; + +import { VaultV2Component } from "../popup/components/vault/vault-v2.component"; +import { VaultPopupItemsService } from "../popup/services/vault-popup-items.service"; +import { VaultPopupListFiltersService } from "../popup/services/vault-popup-list-filters.service"; + +/** + * Guard to clear the vault state (search and filter) when navigating away from the vault view. + * This ensures the search and filter state is reset when navigating between different tabs, except viewing a cipher. + */ +export const clearVaultStateGuard: CanDeactivateFn = ( + component: VaultV2Component, + currentRoute, + currentState, + nextState, +) => { + const vaultPopupItemsService = inject(VaultPopupItemsService); + const vaultPopupListFiltersService = inject(VaultPopupListFiltersService); + if (nextState && !isViewingCipher(nextState.url)) { + vaultPopupItemsService.applyFilter(""); + vaultPopupListFiltersService.resetFilterForm(); + } + + return true; +}; + +const isViewingCipher = (url: string): boolean => url.includes("view-cipher"); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-filters/vault-list-filters.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-list-filters/vault-list-filters.component.ts index 886e1a966a..0e57450fc7 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-filters/vault-list-filters.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-filters/vault-list-filters.component.ts @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { Component, OnDestroy } from "@angular/core"; +import { Component } from "@angular/core"; import { ReactiveFormsModule } from "@angular/forms"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -13,7 +13,7 @@ import { VaultPopupListFiltersService } from "../../../services/vault-popup-list templateUrl: "./vault-list-filters.component.html", imports: [CommonModule, JslibModule, ChipSelectComponent, ReactiveFormsModule], }) -export class VaultListFiltersComponent implements OnDestroy { +export class VaultListFiltersComponent { protected filterForm = this.vaultPopupListFiltersService.filterForm; protected organizations$ = this.vaultPopupListFiltersService.organizations$; protected collections$ = this.vaultPopupListFiltersService.collections$; @@ -21,8 +21,4 @@ export class VaultListFiltersComponent implements OnDestroy { protected cipherTypes = this.vaultPopupListFiltersService.cipherTypes; constructor(private vaultPopupListFiltersService: VaultPopupListFiltersService) {} - - ngOnDestroy(): void { - this.vaultPopupListFiltersService.resetFilterForm(); - } } diff --git a/libs/common/src/platform/misc/compare-values.spec.ts b/libs/common/src/platform/misc/compare-values.spec.ts new file mode 100644 index 0000000000..2734e11374 --- /dev/null +++ b/libs/common/src/platform/misc/compare-values.spec.ts @@ -0,0 +1,35 @@ +import { compareValues } from "./compare-values"; + +describe("compareValues", () => { + it("should return true for equal primitive values", () => { + expect(compareValues(1, 1)).toEqual(true); + expect(compareValues("bitwarden", "bitwarden")).toEqual(true); + expect(compareValues(true, true)).toEqual(true); + }); + + it("should return false for different primitive values", () => { + expect(compareValues(1, 2)).toEqual(false); + expect(compareValues("bitwarden", "bitwarden.com")).toEqual(false); + expect(compareValues(true, false)).toEqual(false); + }); + + it("should return true when both values are null", () => { + expect(compareValues(null, null)).toEqual(true); + }); + + it("should compare deeply nested objects correctly", () => { + // Deeply nested objects + const obj1 = { a: 1, b: { c: 2, d: { e: 3, f: [4, 5, 6] } }, g: [7, 8, { h: 9 }] }; + const obj2 = { a: 1, b: { c: 2, d: { e: 3, f: [4, 5, 6] } }, g: [7, 8, { h: 9 }] }; + + expect(compareValues(obj1, obj2)).toEqual(true); + }); + + it("should return false for deeply nested objects with different values", () => { + // Deeply nested objects + const obj1 = { a: 1, b: { c: 2, d: { e: 3, f: [4, 5, 6] } }, g: [7, 8, { h: 9 }] }; + const obj2 = { a: 1, b: { c: 2, d: { e: 3, f: [4, 5, 7] } }, g: [7, 8, { h: 9 }] }; + + expect(compareValues(obj1, obj2)).toEqual(false); + }); +}); diff --git a/libs/common/src/platform/misc/compare-values.ts b/libs/common/src/platform/misc/compare-values.ts new file mode 100644 index 0000000000..02501506a4 --- /dev/null +++ b/libs/common/src/platform/misc/compare-values.ts @@ -0,0 +1,27 @@ +/** + * Performs deep equality check between two values + * + * NOTE: This method uses JSON.stringify to compare objects, which may return false + * for objects with the same properties but in different order. If order-insensitive + * comparison becomes necessary in future, consider updating this method to use a comparison + * that checks for property existence and value equality without regard to order. + */ +export function compareValues(value1: T, value2: T): boolean { + if (value1 == null && value2 == null) { + return true; + } + + if (value1 && value2 == null) { + return false; + } + + if (value1 == null && value2) { + return false; + } + + if (typeof value1 !== "object" || typeof value2 !== "object") { + return value1 === value2; + } + + return JSON.stringify(value1) === JSON.stringify(value2); +} diff --git a/libs/components/src/chip-select/chip-select.component.ts b/libs/components/src/chip-select/chip-select.component.ts index ff2282b72a..66b4dddb29 100644 --- a/libs/components/src/chip-select/chip-select.component.ts +++ b/libs/components/src/chip-select/chip-select.component.ts @@ -1,6 +1,7 @@ import { Component, HostListener, Input, booleanAttribute, signal } from "@angular/core"; import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms"; +import { compareValues } from "../../../common/src/platform/misc/compare-values"; import { ButtonModule } from "../button"; import { IconButtonModule } from "../icon-button"; import { MenuModule } from "../menu"; @@ -108,7 +109,7 @@ export class ChipSelectComponent implements ControlValueAccessor { */ private findOption(tree: ChipSelectOption, value: T): ChipSelectOption | null { let result = null; - if (tree.value !== null && tree.value === value) { + if (tree.value !== null && compareValues(tree.value, value)) { return tree; }