From b61ec891071dbb9d2e037cfb7752f7d3ca31d986 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 25 Oct 2024 12:31:28 -0700 Subject: [PATCH 1/6] Call setIsLoading(true) whenever the searchValue changes --- .../global_search_bar/public/components/search_bar.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx index de9bb85f7a8a3..ca84de1f2d35e 100644 --- a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx +++ b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx @@ -111,6 +111,11 @@ export const SearchBar: FC = (opts) => { } }, [globalSearch, initialLoad]); + // Whenever searchValue changes, isLoading = true + useEffect(() => { + setIsLoading(true); + }, [searchValue]); + const loadSuggestions = useCallback( (term: string) => { return getSuggestions({ @@ -163,9 +168,7 @@ export const SearchBar: FC = (opts) => { setSearchCharLimitExceeded(false); } - setIsLoading(true); const suggestions = loadSuggestions(searchValue.toLowerCase()); - setIsLoading(false); let aggregatedResults: GlobalSearchResult[] = []; @@ -192,7 +195,7 @@ export const SearchBar: FC = (opts) => { // 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 (searchValue.length > 0) { From 881ae9b7baa9e951ed4dd7f190ffeb154f5006e1 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 25 Oct 2024 14:30:52 -0700 Subject: [PATCH 2/6] Remove `searchTerm`, use `searchValue` for renderOption --- .../global_search_bar/public/components/search_bar.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx index ca84de1f2d35e..42da2f6b19445 100644 --- a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx +++ b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx @@ -90,7 +90,6 @@ export const SearchBar: FC = (opts) => { // General hooks const [initialLoad, setInitialLoad] = useState(false); const [searchValue, setSearchValue] = useState(''); - const [searchTerm, setSearchTerm] = useState(''); const [searchRef, setSearchRef] = useState(null); const [buttonRef, setButtonRef] = useState(null); const searchSubscription = useRef(null); @@ -190,11 +189,6 @@ export const SearchBar: FC = (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 ?? ''); searchSubscription.current = globalSearch.find(searchParams, {}).subscribe({ next: ({ results }) => { @@ -373,7 +367,7 @@ export const SearchBar: FC = (opts) => { className="kbnSearchBar" popoverButtonBreakpoints={['xs', 's']} singleSelection={true} - renderOption={(option) => euiSelectableTemplateSitewideRenderOptions(option, searchTerm)} + renderOption={(option) => euiSelectableTemplateSitewideRenderOptions(option, searchValue)} listProps={{ className: 'eui-yScroll', css: css` From 00fe6b16591c3e164674387f2bf8b5eaae15c194 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 25 Oct 2024 14:34:13 -0700 Subject: [PATCH 3/6] Move unnecessary helper component --- .../global_search_bar/public/components/search_bar.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx index 42da2f6b19445..0e41a783c18c4 100644 --- a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx +++ b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx @@ -39,10 +39,6 @@ import { PopoverPlaceholder } from './popover_placeholder'; import './search_bar.scss'; import { SearchBarProps } from './types'; -const NoMatchesMessage = (props: { basePathUrl: string }) => { - return ; -}; - const SearchCharLimitExceededMessage = (props: { basePathUrl: string }) => { const charLimitMessage = ( <> @@ -397,7 +393,7 @@ export const SearchBar: FC = (opts) => { }} errorMessage={searchCharLimitExceeded ? : null} emptyMessage={} - noMatchesMessage={} + noMatchesMessage={} popoverProps={{ 'data-test-subj': 'nav-search-popover', panelClassName: 'navSearch__panel', From f514f83ec7faaef72aac946fa69786b7a1373527 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 25 Oct 2024 14:34:23 -0700 Subject: [PATCH 4/6] Minor cleanup --- .../public/components/search_bar.tsx | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx index 0e41a783c18c4..d5e0222eb7d1b 100644 --- a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx +++ b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx @@ -89,7 +89,7 @@ export const SearchBar: FC = (opts) => { const [searchRef, setSearchRef] = useState(null); const [buttonRef, setButtonRef] = useState(null); const searchSubscription = useRef(null); - const [options, _setOptions] = useState([]); + const [options, setOptions] = useState([]); const [searchableTypes, setSearchableTypes] = useState([]); const [showAppend, setShowAppend] = useState(true); const UNKNOWN_TAG_ID = '__unknown__'; @@ -122,17 +122,13 @@ export const SearchBar: FC = (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( @@ -143,7 +139,7 @@ export const SearchBar: FC = (opts) => { ), ]); }, - [isMounted, _setOptions, taggingApi] + [setOptions, taggingApi] ); useDebounce( @@ -188,18 +184,20 @@ export const SearchBar: FC = (opts) => { 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); From 6cc9f4949e2e4cc0822a5a3e96e25a6fc5e421aa Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 25 Oct 2024 14:37:06 -0700 Subject: [PATCH 5/6] small comment --- .../plugins/global_search_bar/public/components/search_bar.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx index d5e0222eb7d1b..318b5cf2e4526 100644 --- a/x-pack/plugins/global_search_bar/public/components/search_bar.tsx +++ b/x-pack/plugins/global_search_bar/public/components/search_bar.tsx @@ -96,6 +96,7 @@ export const SearchBar: FC = (opts) => { const [isLoading, setIsLoading] = useState(false); const [searchCharLimitExceeded, setSearchCharLimitExceeded] = useState(false); + // Initialize searchableTypes data useEffect(() => { if (initialLoad) { const fetch = async () => { From cd62e54a7301f2e8ffecaebc440d56312e7faf41 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Mon, 28 Oct 2024 11:48:35 -0700 Subject: [PATCH 6/6] fix unit test --- .../global_search_bar/public/telemetry/telemetry.test.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugins/global_search_bar/public/telemetry/telemetry.test.tsx b/x-pack/plugins/global_search_bar/public/telemetry/telemetry.test.tsx index 2137679fcf5c3..e4302c1e64aec 100644 --- a/x-pack/plugins/global_search_bar/public/telemetry/telemetry.test.tsx +++ b/x-pack/plugins/global_search_bar/public/telemetry/telemetry.test.tsx @@ -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(