1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 07:43:35 +00:00

[PM-13896] Avoid sorting when a search term is applied (#11661)

This commit is contained in:
Shane Melton
2024-10-24 14:11:48 -07:00
committed by GitHub
parent bacb7cd7a0
commit 81d7f319f6
2 changed files with 31 additions and 13 deletions

View File

@@ -277,6 +277,10 @@ describe("VaultPopupItemsService", () => {
}); });
describe("remainingCiphers$", () => { describe("remainingCiphers$", () => {
beforeEach(() => {
searchService.isSearchable.mockImplementation(async (text) => text.length > 2);
});
it("should exclude autofill and favorite ciphers", (done) => { it("should exclude autofill and favorite ciphers", (done) => {
service.remainingCiphers$.subscribe((ciphers) => { service.remainingCiphers$.subscribe((ciphers) => {
// 2 autofill ciphers, 2 favorite ciphers = 6 remaining ciphers to show // 2 autofill ciphers, 2 favorite ciphers = 6 remaining ciphers to show
@@ -285,13 +289,21 @@ describe("VaultPopupItemsService", () => {
}); });
}); });
it("should sort by last used then by name", (done) => { it("should sort by last used then by name by default", (done) => {
service.remainingCiphers$.subscribe((ciphers) => { service.remainingCiphers$.subscribe(() => {
expect(cipherServiceMock.getLocaleSortingFunction).toHaveBeenCalled(); expect(cipherServiceMock.getLocaleSortingFunction).toHaveBeenCalled();
done(); done();
}); });
}); });
it("should NOT sort by last used then by name when search text is applied", (done) => {
service.applyFilter("Login");
service.remainingCiphers$.subscribe(() => {
expect(cipherServiceMock.getLocaleSortingFunction).not.toHaveBeenCalled();
done();
});
});
it("should filter remainingCiphers$ down to search term", (done) => { it("should filter remainingCiphers$ down to search term", (done) => {
const cipherList = Object.values(allCiphers); const cipherList = Object.values(allCiphers);
const searchText = "Login"; const searchText = "Login";

View File

@@ -6,7 +6,6 @@ import {
distinctUntilChanged, distinctUntilChanged,
distinctUntilKeyChanged, distinctUntilKeyChanged,
filter, filter,
from,
map, map,
merge, merge,
MonoTypeOperatorFunction, MonoTypeOperatorFunction,
@@ -111,6 +110,14 @@ export class VaultPopupItemsService {
), ),
); );
/**
* Observable that indicates whether there is search text present that is searchable.
* @private
*/
private _hasSearchText$ = this._searchText$.pipe(
switchMap((searchText) => this.searchService.isSearchable(searchText)),
);
private _filteredCipherList$: Observable<PopupCipherView[]> = combineLatest([ private _filteredCipherList$: Observable<PopupCipherView[]> = combineLatest([
this._activeCipherList$, this._activeCipherList$,
this._searchText$, this._searchText$,
@@ -179,7 +186,11 @@ export class VaultPopupItemsService {
(cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher), (cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher),
), ),
), ),
map((ciphers) => ciphers.sort(this.cipherService.getLocaleSortingFunction())), withLatestFrom(this._hasSearchText$),
map(([ciphers, hasSearchText]) =>
// Do not sort alphabetically when there is search text, default to the search service scoring
hasSearchText ? ciphers : ciphers.sort(this.cipherService.getLocaleSortingFunction()),
),
shareReplay({ refCount: false, bufferSize: 1 }), shareReplay({ refCount: false, bufferSize: 1 }),
); );
@@ -192,19 +203,14 @@ export class VaultPopupItemsService {
).pipe(startWith(true), distinctUntilChanged(), shareReplay({ refCount: false, bufferSize: 1 })); ).pipe(startWith(true), distinctUntilChanged(), shareReplay({ refCount: false, bufferSize: 1 }));
/** /**
* Observable that indicates whether a filter is currently applied to the ciphers. * Observable that indicates whether a filter or search text is currently applied to the ciphers.
*/ */
hasFilterApplied$ = combineLatest([ hasFilterApplied$ = combineLatest([
this._searchText$, this._hasSearchText$,
this.vaultPopupListFiltersService.filters$, this.vaultPopupListFiltersService.filters$,
]).pipe( ]).pipe(
switchMap(([searchText, filters]) => { map(([hasSearchText, filters]) => {
return from(this.searchService.isSearchable(searchText)).pipe( return hasSearchText || Object.values(filters).some((filter) => filter !== null);
map(
(isSearchable) =>
isSearchable || Object.values(filters).some((filter) => filter !== null),
),
);
}), }),
); );