From cf7c36a354cc1007e0ca5a8c2c71bbbec6d027f8 Mon Sep 17 00:00:00 2001 From: Anish Sharma Date: Thu, 4 Jan 2024 16:48:18 +0530 Subject: [PATCH] Fix: Enable search history by default for Search Box (#2566) * make enable search history true by default * fix: debounceTime being shadowed (linting) * check for normal debounce time input * remove redundant comment * minimum debounce value for search history * remove search history debounce input * incorporate debounce time in test * added test for higher debounce time * search history default enabled test --- .../search-box/search-box.component.test.ts | 64 ++++++++++++++++++- .../src/search-box/search-box.component.ts | 35 +++++++--- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/projects/components/src/search-box/search-box.component.test.ts b/projects/components/src/search-box/search-box.component.test.ts index 59c17cd3d..60843d5f2 100644 --- a/projects/components/src/search-box/search-box.component.test.ts +++ b/projects/components/src/search-box/search-box.component.test.ts @@ -119,6 +119,21 @@ describe('Search box Component', () => { }); })); + test('search history should be enabled by default', fakeAsync(() => { + spectator = createHost( + ``, + { + hostProps: { + placeholder: 'Test Placeholder', + debounceTime: 200, + searchMode: SearchBoxEmitMode.Incremental, + }, + }, + ); + + expect(spectator.component.enableSearchHistory).toBe(true); + })); + test('enabled search history should work as expected', fakeAsync(() => { spectator = createHost( ``, @@ -144,16 +159,61 @@ describe('Search box Component', () => { spectator.setInput({ value: 'test-value' }); spectator.triggerEventHandler('input', 'input', 'test-value'); expect(searchBoxELement).toHaveClass('has-value'); - spectator.tick(500); + + spectator.tick(1500); + + expect(spectator.inject(PopoverService).drawPopover).not.toHaveBeenCalled(); inputElement.blur(); spectator.click('.icon.close'); - spectator.tick(500); + spectator.tick(1500); expect(spectator.inject(PopoverService).drawPopover).toHaveBeenCalled(); flush(); })); + test('enabled search history should work as expected for high debounce time', fakeAsync(() => { + const debounceTime = 3000; + spectator = createHost( + ``, + { + hostProps: { + placeholder: 'Test Placeholder', + debounceTime: debounceTime, + searchMode: SearchBoxEmitMode.Incremental, + enableSearchHistory: true, + }, + }, + ); + + const searchBoxELement = spectator.query('.ht-search-box')!; + expect(searchBoxELement).not.toHaveClass('has-value'); + expect(searchBoxELement).not.toHaveClass('focused'); + + spectator.click('.icon.search'); + expect(searchBoxELement).toHaveClass('focused'); + expect(spectator.inject(PopoverService).drawPopover).toHaveBeenCalledTimes(1); + + const inputElement = spectator.query('input') as HTMLInputElement; + spectator.setInput({ value: 'test-value' }); + spectator.triggerEventHandler('input', 'input', 'test-value'); + expect(searchBoxELement).toHaveClass('has-value'); + + // popover not called before debounce is over + expect(spectator.inject(PopoverService).drawPopover).toHaveBeenCalledTimes(1); + + spectator.tick(debounceTime + 100); + + expect(spectator.inject(PopoverService).drawPopover).toHaveBeenCalledTimes(1); + + inputElement.blur(); + spectator.click('.icon.close'); + spectator.tick(debounceTime + 100); + expect(spectator.inject(PopoverService).drawPopover).toHaveBeenCalledTimes(2); + + flush(); + })); + test('collapsable enabled should add collapsable class', () => { spectator = createHost( ``, diff --git a/projects/components/src/search-box/search-box.component.ts b/projects/components/src/search-box/search-box.component.ts index 749eb98f2..060d6bb1c 100644 --- a/projects/components/src/search-box/search-box.component.ts +++ b/projects/components/src/search-box/search-box.component.ts @@ -21,7 +21,7 @@ import { TypedSimpleChanges, } from '@hypertrace/common'; import { combineLatest, Observable, Subject, timer } from 'rxjs'; -import { debounce, map } from 'rxjs/operators'; +import { debounce, debounceTime, map } from 'rxjs/operators'; import { IconSize } from '../icon/icon-size'; import { PopoverService } from '../popover/popover.service'; import { PopoverRef } from '../popover/popover-ref'; @@ -111,7 +111,7 @@ export class SearchBoxComponent implements OnInit, OnChanges { public searchMode: SearchBoxEmitMode = SearchBoxEmitMode.Incremental; @Input() - public enableSearchHistory: boolean = false; // Experimental + public enableSearchHistory: boolean = true; @Input() public collapsable: boolean = false; @@ -137,6 +137,8 @@ export class SearchBoxComponent implements OnInit, OnChanges { public popover?: PopoverRef; + public defaultSearchHistoryDebounceTime = 1000; + public constructor( private readonly cdr: ChangeDetectorRef, private readonly host: ElementRef, @@ -222,7 +224,7 @@ export class SearchBoxComponent implements OnInit, OnChanges { this.subscriptionLifecycle.unsubscribe(); this.subscriptionLifecycle.add( combineLatest([this.debouncedValueSubject, this.getDebounceTime()]) - .pipe(debounce(([_, debounceTime]) => timer(debounceTime))) + .pipe(debounce(([_, valueDebounceTime]) => timer(valueDebounceTime))) .subscribe(([value, _]) => this.valueChange.emit(value)), ); } @@ -260,12 +262,27 @@ export class SearchBoxComponent implements OnInit, OnChanges { }), ); + // Use the default search history debounce time if the value debounce time is less than the default + const searchHistoryDebounce = + this.debounceTime && this.debounceTime > this.defaultSearchHistoryDebounceTime + ? 0 + : this.defaultSearchHistoryDebounceTime; + this.subscriptionLifecycle.add( - this.valueChange.asObservable().subscribe(emittedValue => { - if (!isEmpty(emittedValue)) { - this.lastEmittedValues = [emittedValue, ...this.lastEmittedValues]; - } - }), + searchHistoryDebounce > 0 + ? this.valueChange + .asObservable() + .pipe(debounceTime(searchHistoryDebounce)) + .subscribe(emittedValue => { + if (!isEmpty(emittedValue)) { + this.lastEmittedValues = [emittedValue, ...this.lastEmittedValues]; + } + }) + : this.valueChange.asObservable().subscribe(emittedValue => { + if (!isEmpty(emittedValue)) { + this.lastEmittedValues = [emittedValue, ...this.lastEmittedValues]; + } + }), ); } @@ -296,7 +313,7 @@ export class SearchBoxComponent implements OnInit, OnChanges { } private handleSearchHistoryOnInputBlur(): void { - this.searchHistory = [...uniq(this.lastEmittedValues), ...this.searchHistory]; + this.searchHistory = uniq([...this.lastEmittedValues, ...this.searchHistory]); this.filteredSearchHistory = [...this.searchHistory]; this.lastEmittedValues = []; }