[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
This commit is contained in:
SmithThe4th 2024-08-08 11:29:33 -04:00 committed by GitHub
parent 48cb6fbec4
commit c1bf1a797f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 98 additions and 24 deletions

View File

@ -8,6 +8,7 @@ import {
ObservableStorageService, ObservableStorageService,
StorageUpdate, StorageUpdate,
} from "@bitwarden/common/platform/abstractions/storage.service"; } 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 { Lazy } from "@bitwarden/common/platform/misc/lazy";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options";
@ -190,23 +191,7 @@ export class LocalBackedSessionStorageService
private compareValues<T>(value1: T, value2: T): boolean { private compareValues<T>(value1: T, value2: T): boolean {
try { try {
if (value1 == null && value2 == null) { return compareValues(value1, value2);
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);
} catch (e) { } catch (e) {
this.logService.error( this.logService.error(
`error comparing values\n${JSON.stringify(value1)}\n${JSON.stringify(value2)}`, `error comparing values\n${JSON.stringify(value1)}\n${JSON.stringify(value2)}`,

View File

@ -64,6 +64,7 @@ import { ImportBrowserV2Component } from "../tools/popup/settings/import/import-
import { ImportBrowserComponent } from "../tools/popup/settings/import/import-browser.component"; import { ImportBrowserComponent } from "../tools/popup/settings/import/import-browser.component";
import { SettingsV2Component } from "../tools/popup/settings/settings-v2.component"; import { SettingsV2Component } from "../tools/popup/settings/settings-v2.component";
import { SettingsComponent } from "../tools/popup/settings/settings.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 { AddEditComponent } from "../vault/popup/components/vault/add-edit.component";
import { AttachmentsComponent } from "../vault/popup/components/vault/attachments.component"; import { AttachmentsComponent } from "../vault/popup/components/vault/attachments.component";
import { CollectionsComponent } from "../vault/popup/components/vault/collections.component"; import { CollectionsComponent } from "../vault/popup/components/vault/collections.component";
@ -457,6 +458,7 @@ const routes: Routes = [
...extensionRefreshSwap(VaultFilterComponent, VaultV2Component, { ...extensionRefreshSwap(VaultFilterComponent, VaultV2Component, {
path: "vault", path: "vault",
canActivate: [authGuard], canActivate: [authGuard],
canDeactivate: [clearVaultStateGuard],
data: { state: "tabs_vault" }, data: { state: "tabs_vault" },
}), }),
{ {

View File

@ -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<VaultV2Component> = (
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");

View File

@ -1,5 +1,5 @@
import { CommonModule } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, OnDestroy } from "@angular/core"; import { Component } from "@angular/core";
import { ReactiveFormsModule } from "@angular/forms"; import { ReactiveFormsModule } from "@angular/forms";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
@ -13,7 +13,7 @@ import { VaultPopupListFiltersService } from "../../../services/vault-popup-list
templateUrl: "./vault-list-filters.component.html", templateUrl: "./vault-list-filters.component.html",
imports: [CommonModule, JslibModule, ChipSelectComponent, ReactiveFormsModule], imports: [CommonModule, JslibModule, ChipSelectComponent, ReactiveFormsModule],
}) })
export class VaultListFiltersComponent implements OnDestroy { export class VaultListFiltersComponent {
protected filterForm = this.vaultPopupListFiltersService.filterForm; protected filterForm = this.vaultPopupListFiltersService.filterForm;
protected organizations$ = this.vaultPopupListFiltersService.organizations$; protected organizations$ = this.vaultPopupListFiltersService.organizations$;
protected collections$ = this.vaultPopupListFiltersService.collections$; protected collections$ = this.vaultPopupListFiltersService.collections$;
@ -21,8 +21,4 @@ export class VaultListFiltersComponent implements OnDestroy {
protected cipherTypes = this.vaultPopupListFiltersService.cipherTypes; protected cipherTypes = this.vaultPopupListFiltersService.cipherTypes;
constructor(private vaultPopupListFiltersService: VaultPopupListFiltersService) {} constructor(private vaultPopupListFiltersService: VaultPopupListFiltersService) {}
ngOnDestroy(): void {
this.vaultPopupListFiltersService.resetFilterForm();
}
} }

View File

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

View File

@ -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<T>(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);
}

View File

@ -1,6 +1,7 @@
import { Component, HostListener, Input, booleanAttribute, signal } from "@angular/core"; import { Component, HostListener, Input, booleanAttribute, signal } from "@angular/core";
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms"; import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms";
import { compareValues } from "../../../common/src/platform/misc/compare-values";
import { ButtonModule } from "../button"; import { ButtonModule } from "../button";
import { IconButtonModule } from "../icon-button"; import { IconButtonModule } from "../icon-button";
import { MenuModule } from "../menu"; import { MenuModule } from "../menu";
@ -108,7 +109,7 @@ export class ChipSelectComponent<T = unknown> implements ControlValueAccessor {
*/ */
private findOption(tree: ChipSelectOption<T>, value: T): ChipSelectOption<T> | null { private findOption(tree: ChipSelectOption<T>, value: T): ChipSelectOption<T> | null {
let result = null; let result = null;
if (tree.value !== null && tree.value === value) { if (tree.value !== null && compareValues(tree.value, value)) {
return tree; return tree;
} }