Skip to content

Commit

Permalink
[Global Search] Instantly set isLoading=true when search value chan…
Browse files Browse the repository at this point in the history
…ges (#197750)

## Summary

Close #77059

This PR solves the bug by setting the `isLoading` flag outside of the
block of debounced code whenever the search term changes.

This also makes a few slight cleanups to `search_bar.tsx`, which is
quite large. I avoided doing any serious cleanups that would make the
diff hard to read or detract from the fix.
  • Loading branch information
tsullivan authored Oct 28, 2024
1 parent 8146226 commit c378cd9
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 30 deletions.
46 changes: 19 additions & 27 deletions x-pack/plugins/global_search_bar/public/components/search_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ import { PopoverPlaceholder } from './popover_placeholder';
import './search_bar.scss';
import { SearchBarProps } from './types';

const NoMatchesMessage = (props: { basePathUrl: string }) => {
return <PopoverPlaceholder basePath={props.basePathUrl} />;
};

const SearchCharLimitExceededMessage = (props: { basePathUrl: string }) => {
const charLimitMessage = (
<>
Expand Down Expand Up @@ -90,17 +86,17 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
// General hooks
const [initialLoad, setInitialLoad] = useState(false);
const [searchValue, setSearchValue] = useState<string>('');
const [searchTerm, setSearchTerm] = useState<string>('');
const [searchRef, setSearchRef] = useState<HTMLInputElement | null>(null);
const [buttonRef, setButtonRef] = useState<HTMLDivElement | null>(null);
const searchSubscription = useRef<Subscription | null>(null);
const [options, _setOptions] = useState<EuiSelectableTemplateSitewideOption[]>([]);
const [options, setOptions] = useState<EuiSelectableTemplateSitewideOption[]>([]);
const [searchableTypes, setSearchableTypes] = useState<string[]>([]);
const [showAppend, setShowAppend] = useState<boolean>(true);
const UNKNOWN_TAG_ID = '__unknown__';
const [isLoading, setIsLoading] = useState<boolean>(false);
const [searchCharLimitExceeded, setSearchCharLimitExceeded] = useState(false);

// Initialize searchableTypes data
useEffect(() => {
if (initialLoad) {
const fetch = async () => {
Expand All @@ -111,6 +107,11 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
}
}, [globalSearch, initialLoad]);

// Whenever searchValue changes, isLoading = true
useEffect(() => {
setIsLoading(true);
}, [searchValue]);

const loadSuggestions = useCallback(
(term: string) => {
return getSuggestions({
Expand All @@ -122,17 +123,13 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
[taggingApi, searchableTypes]
);

const setOptions = useCallback(
const setDecoratedOptions = useCallback(
(
_options: GlobalSearchResult[],
suggestions: SearchSuggestion[],
searchTagIds: string[] = []
) => {
if (!isMounted()) {
return;
}

_setOptions([
setOptions([
...suggestions.map(suggestionToOption),
..._options.map((option) =>
resultToOption(
Expand All @@ -143,7 +140,7 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
),
]);
},
[isMounted, _setOptions, taggingApi]
[setOptions, taggingApi]
);

useDebounce(
Expand All @@ -163,9 +160,7 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
setSearchCharLimitExceeded(false);
}

setIsLoading(true);
const suggestions = loadSuggestions(searchValue.toLowerCase());
setIsLoading(false);

let aggregatedResults: GlobalSearchResult[] = [];

Expand All @@ -187,26 +182,23 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
types: rawParams.filters.types,
tags: tagIds,
};
// TODO technically a subtle bug here
// this term won't be set until the next time the debounce is fired
// so the SearchOption won't highlight anything if only one call is fired
// in practice, this is hard to spot, unlikely to happen, and is a negligible issue
setSearchTerm(rawParams.term ?? '');
setIsLoading(true);

searchSubscription.current = globalSearch.find(searchParams, {}).subscribe({
next: ({ results }) => {
if (!isMounted()) {
return;
}

if (searchValue.length > 0) {
aggregatedResults = [...results, ...aggregatedResults].sort(sort.byScore);
setOptions(aggregatedResults, suggestions, searchParams.tags);
setDecoratedOptions(aggregatedResults, suggestions, searchParams.tags);
return;
}

// if searchbar is empty, filter to only applications and sort alphabetically
results = results.filter(({ type }: GlobalSearchResult) => type === 'application');

aggregatedResults = [...results, ...aggregatedResults].sort(sort.byTitle);

setOptions(aggregatedResults, suggestions, searchParams.tags);
setDecoratedOptions(aggregatedResults, suggestions, searchParams.tags);
},
error: (err) => {
setIsLoading(false);
Expand Down Expand Up @@ -371,7 +363,7 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
className="kbnSearchBar"
popoverButtonBreakpoints={['xs', 's']}
singleSelection={true}
renderOption={(option) => euiSelectableTemplateSitewideRenderOptions(option, searchTerm)}
renderOption={(option) => euiSelectableTemplateSitewideRenderOptions(option, searchValue)}
listProps={{
className: 'eui-yScroll',
css: css`
Expand Down Expand Up @@ -401,7 +393,7 @@ export const SearchBar: FC<SearchBarProps> = (opts) => {
}}
errorMessage={searchCharLimitExceeded ? <SearchCharLimitExceededMessage {...props} /> : null}
emptyMessage={<EmptyMessage />}
noMatchesMessage={<NoMatchesMessage {...props} />}
noMatchesMessage={<PopoverPlaceholder basePath={props.basePathUrl} />}
popoverProps={{
'data-test-subj': 'nav-search-popover',
panelClassName: 'navSearch__panel',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,7 @@ describe('SearchBar', () => {
});

it(`tracks the user's search term`, async () => {
searchService.find.mockReturnValueOnce(
of(createBatch('Discover', { id: 'My Dashboard', type: 'test' }))
);
searchService.find.mockReturnValue(of(createBatch('Discover')));

render(
<IntlProvider locale="en">
Expand Down

0 comments on commit c378cd9

Please sign in to comment.