From 7307a7f3eb8acf3b3ae7b4f1067d4edcd9a9029d Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Wed, 9 Dec 2020 16:25:21 -0500 Subject: [PATCH] Adding tags UI to search results Co-authored-by: Ryan Keairns --- .../public/api.mock.ts | 1 + .../saved_objects_tagging_oss/public/api.ts | 7 ++ .../public/components/search_bar.scss | 24 +++- .../public/components/search_bar.tsx | 116 +++++++++++++++--- .../map_object_to_result.test.ts | 18 ++- .../saved_objects/map_object_to_result.ts | 3 + .../providers/saved_objects/provider.test.ts | 2 + .../saved_objects_tagging/public/index.ts | 1 + .../public/ui_api/index.ts | 8 +- .../public/utils.test.ts | 10 ++ .../saved_objects_tagging/public/utils.ts | 4 + 11 files changed, 172 insertions(+), 22 deletions(-) diff --git a/src/plugins/saved_objects_tagging_oss/public/api.mock.ts b/src/plugins/saved_objects_tagging_oss/public/api.mock.ts index 1e66a9baa812e..efde832e7c97d 100644 --- a/src/plugins/saved_objects_tagging_oss/public/api.mock.ts +++ b/src/plugins/saved_objects_tagging_oss/public/api.mock.ts @@ -73,6 +73,7 @@ const createApiUiMock = () => { getTagIdsFromReferences: jest.fn(), getTagIdFromName: jest.fn(), updateTagsReferences: jest.fn(), + getTag: jest.fn(), }; return mock; diff --git a/src/plugins/saved_objects_tagging_oss/public/api.ts b/src/plugins/saved_objects_tagging_oss/public/api.ts index 987930af1e3e4..9d706d4f728c1 100644 --- a/src/plugins/saved_objects_tagging_oss/public/api.ts +++ b/src/plugins/saved_objects_tagging_oss/public/api.ts @@ -71,6 +71,13 @@ export type SavedObjectTagDecoratorTypeGuard = SavedObjectsTaggingApiUi['hasTagD * @public */ export interface SavedObjectsTaggingApiUi { + /** + * Return a Tag from an ID + * + * @param tagId + */ + getTag(tagId: string): Tag | undefined; + /** * Type-guard to safely manipulate tag-enhanced `SavedObject` from the `savedObject` plugin. * diff --git a/x-pack/plugins/global_search_bar/public/components/search_bar.scss b/x-pack/plugins/global_search_bar/public/components/search_bar.scss index eadac85c8be89..7e6c3ddaa3126 100644 --- a/x-pack/plugins/global_search_bar/public/components/search_bar.scss +++ b/x-pack/plugins/global_search_bar/public/components/search_bar.scss @@ -1,10 +1,32 @@ -//TODO add these overrides to EUI so that search behaves the same globally +.kbnSearchOption__tagsList { + display: inline-block; // Horizontally aligns the tag list to the 'Go to' badge when row is focused + line-height: $euiFontSizeM !important; + + .kbnSearchOption__tagsListItem { + display: inline-block; + max-width: 80px; + margin-right: $euiSizeS; + } +} + +.euiSelectableListItem-isFocused .kbnSearchOption__tagsList { + margin-right: $euiSizeXS; + border-right: $euiBorderThin; // Adds divider between the tag list and 'Go to' badge +} + +//TODO add these overrides to EUI so that search behaves the same globally (eui/issues/4363) .kbnSearchBar { width: 400px; max-width: 100%; will-change: width; } +@include euiBreakpoint('xs', 's') { + .kbnSearchOption__tagsList { + display: none; + } +} + @include euiBreakpoint('l', 'xl') { .kbnSearchBar:focus { animation: kbnAnimateSearchBar $euiAnimSpeedFast forwards; 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 0bbfe0347dd36..053d3b9a72e88 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 @@ -15,12 +15,14 @@ import { EuiSelectableTemplateSitewide, EuiSelectableTemplateSitewideOption, EuiText, + EuiBadge, + euiSelectableTemplateSitewideRenderOptions, } from '@elastic/eui'; import { METRIC_TYPE, UiCounterMetricType } from '@kbn/analytics'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { ApplicationStart } from 'kibana/public'; -import React, { useCallback, useRef, useState, useEffect } from 'react'; +import React, { ReactNode, useCallback, useRef, useState, useEffect } from 'react'; import useDebounce from 'react-use/lib/useDebounce'; import useEvent from 'react-use/lib/useEvent'; import useMountedState from 'react-use/lib/useMountedState'; @@ -30,10 +32,9 @@ import { GlobalSearchResult, GlobalSearchFindParams, } from '../../../global_search/public'; -import { SavedObjectTaggingPluginStart } from '../../../saved_objects_tagging/public'; +import { SavedObjectTaggingPluginStart, Tag } from '../../../saved_objects_tagging/public'; import { parseSearchParams } from '../search_syntax'; import { getSuggestions, SearchSuggestion } from '../suggestions'; - import './search_bar.scss'; interface Props { @@ -75,8 +76,64 @@ const sortByTitle = (a: GlobalSearchResult, b: GlobalSearchResult): number => { return 0; }; -const resultToOption = (result: GlobalSearchResult): EuiSelectableTemplateSitewideOption => { - const { id, title, url, icon, type, meta } = result; +const TagListWrapper = ({ children }: { children: ReactNode }) => ( + +); + +const buildListItem = ({ color, name, id }: Tag) => { + return ( +
  • + {name} +
    • + ); +}; + +const tagList = (tags: Tag[], searchTagIds: string[]) => { + const TAGS_TO_SHOW = 3; + const showOverflow = tags.length > TAGS_TO_SHOW; + + if (!showOverflow) return {tags.map(buildListItem)}; + + // float searched tags to the start of the list, actual order doesn't matter + tags.sort((a) => { + if (searchTagIds.find((id) => id === a.id)) return -1; + return 1; + }); + + const overflowList = tags.splice(TAGS_TO_SHOW); + const overflowMessage = i18n.translate('xpack.globalSearchBar.searchbar.overflowTagsAriaLabel', { + defaultMessage: '{n} more {n, plural, one {tag} other {tags}}: {tags}', + values: { + n: overflowList.length, + // @ts-ignore-line + tags: overflowList.map(({ name }) => name), + }, + }); + + return ( + + {tags.map(buildListItem)} +
  • + +{overflowList.length} +
    • +     + ); +}; + +const resultToOption = ( + result: GlobalSearchResult, + searchTagIds: string[], + getTag?: SavedObjectTaggingPluginStart['ui']['getTag'] +): EuiSelectableTemplateSitewideOption => { + const { id, title, url, icon, type, meta = {} } = result; + const { tagIds = [], categoryLabel = '' } = meta as { tagIds: string[]; categoryLabel: string }; // only displaying icons for applications const useIcon = type === 'application'; const option: EuiSelectableTemplateSitewideOption = { @@ -88,10 +145,13 @@ const resultToOption = (result: GlobalSearchResult): EuiSelectableTemplateSitewi 'data-test-subj': `nav-search-option`, }; - if (type === 'application') { - option.meta = [{ text: (meta?.categoryLabel as string) ?? '' }]; - } else { - option.meta = [{ text: cleanMeta(type) }]; + if (type === 'application') option.meta = [{ text: categoryLabel }]; + else option.meta = [{ text: cleanMeta(type) }]; + + if (getTag && tagIds.length) { + // TODO #85189 - refactor to use TagList instead of getTag + // Casting to Tag[] because we know all our IDs will be valid here, no need to check for undefined + option.append = tagList(tagIds.map(getTag) as Tag[], searchTagIds); } return option; @@ -120,11 +180,13 @@ export function SearchBar({ }: Props) { const isMounted = useMountedState(); const [searchValue, setSearchValue] = useState(''); + const [searchTerm, setSearchTerm] = useState(''); const [searchRef, setSearchRef] = useState(null); const [buttonRef, setButtonRef] = useState(null); const searchSubscription = useRef(null); const [options, _setOptions] = useState([]); const [searchableTypes, setSearchableTypes] = useState([]); + const UNKNOWN_TAG_ID = '__unknown__'; useEffect(() => { const fetch = async () => { @@ -135,9 +197,9 @@ export function SearchBar({ }, [globalSearch]); const loadSuggestions = useCallback( - (searchTerm: string) => { + (term: string) => { return getSuggestions({ - searchTerm, + searchTerm: term, searchableTypes, tagCache: taggingApi?.cache, }); @@ -146,13 +208,27 @@ export function SearchBar({ ); const setOptions = useCallback( - (_options: GlobalSearchResult[], suggestions: SearchSuggestion[]) => { + ( + _options: GlobalSearchResult[], + suggestions: SearchSuggestion[], + searchTagIds: string[] = [] + ) => { if (!isMounted()) { return; } - _setOptions([...suggestions.map(suggestionToOption), ..._options.map(resultToOption)]); + + _setOptions([ + ...suggestions.map(suggestionToOption), + ..._options.map((option) => + resultToOption( + option, + searchTagIds?.filter((id) => id !== UNKNOWN_TAG_ID) ?? [], + taggingApi?.ui.getTag + ) + ), + ]); }, - [isMounted, _setOptions] + [isMounted, _setOptions, taggingApi] ); useDebounce( @@ -174,7 +250,7 @@ export function SearchBar({ const tagIds = taggingApi && rawParams.filters.tags ? rawParams.filters.tags.map( - (tagName) => taggingApi.ui.getTagIdFromName(tagName) ?? '__unknown__' + (tagName) => taggingApi.ui.getTagIdFromName(tagName) ?? UNKNOWN_TAG_ID ) : undefined; const searchParams: GlobalSearchFindParams = { @@ -182,12 +258,17 @@ export function SearchBar({ 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 }) => { if (searchValue.length > 0) { aggregatedResults = [...results, ...aggregatedResults].sort(sortByScore); - setOptions(aggregatedResults, suggestions); + setOptions(aggregatedResults, suggestions, searchParams.tags); return; } @@ -196,7 +277,7 @@ export function SearchBar({ aggregatedResults = [...results, ...aggregatedResults].sort(sortByTitle); - setOptions(aggregatedResults, suggestions); + setOptions(aggregatedResults, suggestions, searchParams.tags); }, error: () => { // Not doing anything on error right now because it'll either just show the previous @@ -304,6 +385,7 @@ export function SearchBar({ options={options} popoverButtonBreakpoints={['xs', 's']} singleSelection={true} + renderOption={(option) => euiSelectableTemplateSitewideRenderOptions(option, searchTerm)} popoverButton={ ): SavedObjectsType => { return { @@ -24,12 +25,13 @@ const createType = (props: Partial): SavedObjectsType => { const createObject = ( props: Partial, - attributes: T + attributes: T, + references: SavedObjectReference[] = [] ): SavedObjectsFindResult => { return { id: 'id', type: 'dashboard', - references: [], + references, score: 100, ...props, attributes, @@ -65,6 +67,7 @@ describe('mapToResult', () => { url: '/dashboard/dash1', icon: 'dashboardApp', score: 42, + meta: { tagIds: [] }, }); }); @@ -198,7 +201,12 @@ describe('mapToResults', () => { { excerpt: 'titleC', title: 'foo', - } + }, + [ + { name: 'tag A', type: 'tag', id: '1' }, + { name: 'tag B', type: 'tag', id: '2' }, + { name: 'not-tag', type: 'not-tag', id: '1' }, + ] ), createObject( { @@ -220,6 +228,7 @@ describe('mapToResults', () => { type: 'typeA', url: '/type-a/resultA', score: 100, + meta: { tagIds: [] }, }, { id: 'resultC', @@ -227,6 +236,7 @@ describe('mapToResults', () => { type: 'typeC', url: '/type-c/resultC', score: 42, + meta: { tagIds: ['1', '2'] }, }, { id: 'resultB', @@ -234,6 +244,7 @@ describe('mapToResults', () => { type: 'typeB', url: '/type-b/resultB', score: 69, + meta: { tagIds: [] }, }, ]); }); @@ -271,6 +282,7 @@ describe('mapToResults', () => { type: 'typeA', url: '/type-a/resultA', score: 100, + meta: { tagIds: [] }, }, ]); }); diff --git a/x-pack/plugins/global_search_providers/server/providers/saved_objects/map_object_to_result.ts b/x-pack/plugins/global_search_providers/server/providers/saved_objects/map_object_to_result.ts index ec55a2a78fa9e..d16e05c1e9ee7 100644 --- a/x-pack/plugins/global_search_providers/server/providers/saved_objects/map_object_to_result.ts +++ b/x-pack/plugins/global_search_providers/server/providers/saved_objects/map_object_to_result.ts @@ -53,5 +53,8 @@ export const mapToResult = ( icon: type.management?.icon ?? undefined, url: getInAppUrl(object).path, score: object.score, + meta: { + tagIds: object.references.filter((ref) => ref.type === 'tag').map(({ id }) => id), + }, }; }; diff --git a/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts b/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts index 5d24b33f2619e..6bbda4f71a694 100644 --- a/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts +++ b/x-pack/plugins/global_search_providers/server/providers/saved_objects/provider.test.ts @@ -201,6 +201,7 @@ describe('savedObjectsResultProvider', () => { type: 'typeA', url: '/type-a/resultA', score: 50, + meta: { tagIds: [] }, }, { id: 'resultB', @@ -208,6 +209,7 @@ describe('savedObjectsResultProvider', () => { type: 'typeB', url: '/type-b/resultB', score: 78, + meta: { tagIds: [] }, }, ]); }); diff --git a/x-pack/plugins/saved_objects_tagging/public/index.ts b/x-pack/plugins/saved_objects_tagging/public/index.ts index 9519e40d1d2f5..195627b01f800 100644 --- a/x-pack/plugins/saved_objects_tagging/public/index.ts +++ b/x-pack/plugins/saved_objects_tagging/public/index.ts @@ -8,6 +8,7 @@ import { PluginInitializerContext } from '../../../../src/core/public'; import { SavedObjectTaggingPlugin } from './plugin'; export { SavedObjectTaggingPluginStart } from './types'; +export { Tag } from '../common'; export const plugin = (initializerContext: PluginInitializerContext) => new SavedObjectTaggingPlugin(initializerContext); diff --git a/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts b/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts index 4cadf6ea773e3..1af3d7ba1813f 100644 --- a/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts +++ b/x-pack/plugins/saved_objects_tagging/public/ui_api/index.ts @@ -8,7 +8,12 @@ import { OverlayStart } from 'src/core/public'; import { SavedObjectsTaggingApiUi } from '../../../../../src/plugins/saved_objects_tagging_oss/public'; import { TagsCapabilities } from '../../common'; import { ITagsCache, ITagInternalClient } from '../services'; -import { getTagIdsFromReferences, updateTagsReferences, convertTagNameToId } from '../utils'; +import { + getTagIdsFromReferences, + updateTagsReferences, + convertTagNameToId, + getTag, +} from '../utils'; import { getComponents } from './components'; import { buildGetTableColumnDefinition } from './get_table_column_definition'; import { buildGetSearchBarFilter } from './get_search_bar_filter'; @@ -41,5 +46,6 @@ export const getUiApi = ({ getTagIdsFromReferences, getTagIdFromName: (tagName: string) => convertTagNameToId(tagName, cache.getState()), updateTagsReferences, + getTag: (tagId: string) => getTag(tagId, cache.getState()), }; }; diff --git a/x-pack/plugins/saved_objects_tagging/public/utils.test.ts b/x-pack/plugins/saved_objects_tagging/public/utils.test.ts index 5a348892a4b9b..f88c7589d8c42 100644 --- a/x-pack/plugins/saved_objects_tagging/public/utils.test.ts +++ b/x-pack/plugins/saved_objects_tagging/public/utils.test.ts @@ -10,6 +10,7 @@ import { convertTagNameToId, byNameTagSorter, getTagIdsFromReferences, + getTag, } from './utils'; const createTag = (id: string, name: string = id) => ({ @@ -71,6 +72,15 @@ describe('convertTagNameToId', () => { }); }); +describe('getTag', () => { + it('returns the tag for the given id', () => { + expect(getTag('id-2', allTags)).toEqual(tag2); + }); + it('returns undefined if no tag was found', () => { + expect(getTag('id-4', allTags)).toBeUndefined(); + }); +}); + describe('byNameTagSorter', () => { it('sorts tags by name', () => { const tags = [ diff --git a/x-pack/plugins/saved_objects_tagging/public/utils.ts b/x-pack/plugins/saved_objects_tagging/public/utils.ts index 05f534a8ebe7f..082893e5768ad 100644 --- a/x-pack/plugins/saved_objects_tagging/public/utils.ts +++ b/x-pack/plugins/saved_objects_tagging/public/utils.ts @@ -49,6 +49,10 @@ export const byNameTagSorter = (tagA: Tag, tagB: Tag): number => { return tagA.name.localeCompare(tagB.name); }; +export const getTag = (tagId: string, allTags: Tag[]): Tag | undefined => { + return allTags.find(({ id }) => id === tagId); +}; + export const testSubjFriendly = (name: string) => { return name.replace(' ', '_'); };