Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Enable search history by default for Search Box #2566

Merged
merged 13 commits into from
Jan 4, 2024
Merged
64 changes: 62 additions & 2 deletions projects/components/src/search-box/search-box.component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,21 @@ describe('Search box Component', () => {
});
}));

test('search history should be enabled by default', fakeAsync(() => {
spectator = createHost(
`<ht-search-box [placeholder]="placeholder" [debounceTime]="debounceTime" [searchMode]="searchMode"></ht-search-box>`,
{
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(
`<ht-search-box [placeholder]="placeholder" [debounceTime]="debounceTime" [searchMode]="searchMode" [enableSearchHistory]="enableSearchHistory"></ht-search-box>`,
Expand All @@ -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(
`<ht-search-box [placeholder]="placeholder" [debounceTime]="debounceTime" [searchMode]="searchMode" [enableSearchHistory]="enableSearchHistory"></ht-search-box>`,
{
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(
`<ht-search-box [placeholder]="placeholder" [debounceTime]="debounceTime" [searchMode]="searchMode" [enableSearchHistory]="enableSearchHistory" [collapsable]="collapsable"></ht-search-box>`,
Expand Down
35 changes: 26 additions & 9 deletions projects/components/src/search-box/search-box.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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)),
);
}
Expand Down Expand Up @@ -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];
}
}),
);
}

Expand Down Expand Up @@ -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 = [];
}
Expand Down