Skip to content

Commit

Permalink
[8.x] [Global Search] Instantly set `isLoading=true` w…
Browse files Browse the repository at this point in the history
…hen search value changes (elastic#197750) (elastic#198090)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Global Search] Instantly set `isLoading=true` when
search value changes
(elastic#197750)](elastic#197750)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-28T20:35:17Z","message":"[Global
Search] Instantly set `isLoading=true` when search value changes
(elastic#197750)\n\n## Summary\r\n\r\nClose
https://github.com/elastic/kibana/issues/77059\r\n\r\nThis PR solves the
bug by setting the `isLoading` flag outside of the\r\nblock of debounced
code whenever the search term changes.\r\n\r\nThis also makes a few
slight cleanups to `search_bar.tsx`, which is\r\nquite large. I avoided
doing any serious cleanups that would make the\r\ndiff hard to read or
detract from the
fix.","sha":"c378cd9186278d47d97ca7d328b4c666eb7a9df4","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","backport:version","v8.17.0"],"title":"[Global
Search] Instantly set `isLoading=true` when search value
changes","number":197750,"url":"https://github.com/elastic/kibana/pull/197750","mergeCommit":{"message":"[Global
Search] Instantly set `isLoading=true` when search value changes
(elastic#197750)\n\n## Summary\r\n\r\nClose
https://github.com/elastic/kibana/issues/77059\r\n\r\nThis PR solves the
bug by setting the `isLoading` flag outside of the\r\nblock of debounced
code whenever the search term changes.\r\n\r\nThis also makes a few
slight cleanups to `search_bar.tsx`, which is\r\nquite large. I avoided
doing any serious cleanups that would make the\r\ndiff hard to read or
detract from the
fix.","sha":"c378cd9186278d47d97ca7d328b4c666eb7a9df4"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197750","number":197750,"mergeCommit":{"message":"[Global
Search] Instantly set `isLoading=true` when search value changes
(elastic#197750)\n\n## Summary\r\n\r\nClose
https://github.com/elastic/kibana/issues/77059\r\n\r\nThis PR solves the
bug by setting the `isLoading` flag outside of the\r\nblock of debounced
code whenever the search term changes.\r\n\r\nThis also makes a few
slight cleanups to `search_bar.tsx`, which is\r\nquite large. I avoided
doing any serious cleanups that would make the\r\ndiff hard to read or
detract from the
fix.","sha":"c378cd9186278d47d97ca7d328b4c666eb7a9df4"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Tim Sullivan <[email protected]>
  • Loading branch information
kibanamachine and tsullivan authored Oct 28, 2024
1 parent 68b0489 commit 361e715
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 @@ -370,7 +362,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 @@ -400,7 +392,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 361e715

Please sign in to comment.