-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Combine siblings in autocomplete #8610
Changes from 8 commits
0975526
60eee46
0116d15
4aa73e1
844552a
e72aaea
92dd8db
5f85017
91649e2
6953c63
f6571f0
7707b54
931570a
5b2395c
4d573d7
45b8ded
fe9c179
e89afc2
5620c5e
97e2b84
7c8d0b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,8 @@ import merge from 'deepmerge'; | |
import { unionBy, keyBy, values } from 'lodash'; | ||
import { useLocation } from 'react-router-dom'; | ||
import * as QueryString from 'query-string'; | ||
import { Dataset, Entity, MatchedField, Maybe, SiblingProperties } from '../../../types.generated'; | ||
import { Dataset, Entity, Maybe, SiblingProperties } from '../../../types.generated'; | ||
import { GenericEntityProperties } from './types'; | ||
|
||
export function stripSiblingsFromEntity(entity: any) { | ||
return { | ||
|
@@ -202,53 +203,56 @@ export const combineEntityDataWithSiblings = <T>(baseEntity: T): T => { | |
return { [baseEntityKey]: combinedBaseEntity } as unknown as T; | ||
}; | ||
|
||
export type CombinedSearchResult = { | ||
export type CombinedEntityResult = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. combineSiblingEntities is now more generalized and less about search specifically so other use cases can just pass in some entities and combine siblings (as long as you provided a gql entity that actually had siblings). Should make it more reusable. |
||
entity: Entity; | ||
matchedFields: MatchedField[]; | ||
matchedEntities?: Entity[]; | ||
matchedEntities?: Array<Entity>; | ||
}; | ||
|
||
export function combineSiblingsInSearchResults( | ||
results: | ||
| { | ||
entity: Entity; | ||
matchedFields: MatchedField[]; | ||
}[] | ||
| undefined, | ||
) { | ||
const combinedResults: CombinedSearchResult[] | undefined = []; | ||
const siblingsToPair: Record<string, CombinedSearchResult> = {}; | ||
|
||
// set sibling associations | ||
results?.forEach((result) => { | ||
if (result.entity.urn in siblingsToPair) { | ||
// filter from repeating | ||
// const siblingsCombinedResult = siblingsToPair[result.entity.urn]; | ||
// siblingsCombinedResult.matchedEntities?.push(result.entity); | ||
type CombineOptions = { | ||
combine?: boolean; | ||
}; | ||
|
||
export function combineSiblingEntities( | ||
entities?: Array<Entity>, | ||
{ combine = true }: CombineOptions = {}, | ||
): Array<CombinedEntityResult> { | ||
const combinedResults: CombinedEntityResult[] = []; | ||
const siblingsToPair: Set<string> = new Set(); | ||
|
||
entities?.forEach((entityReadOnly) => { | ||
if (!combine) { | ||
combinedResults.push({ entity: entityReadOnly }); | ||
return; | ||
} | ||
|
||
if (siblingsToPair.has(entityReadOnly.urn)) { | ||
return; | ||
} | ||
|
||
const combinedResult: CombinedSearchResult = result; | ||
const { entity }: { entity: any } = result; | ||
const siblingUrns = entity?.siblings?.siblings?.map((sibling) => sibling.urn) || []; | ||
const combinedResult: CombinedEntityResult = { entity: entityReadOnly }; | ||
const entity = entityReadOnly as GenericEntityProperties; | ||
const siblings = entity.siblings?.siblings ?? []; | ||
const isPrimary = entity.siblings?.isPrimary; | ||
const siblingUrns = siblings.map((sibling) => sibling?.urn) || []; | ||
|
||
if (siblingUrns.length > 0) { | ||
combinedResult.matchedEntities = entity.siblings.isPrimary | ||
? [stripSiblingsFromEntity(entity), ...entity.siblings.siblings] | ||
: [...entity.siblings.siblings, stripSiblingsFromEntity(entity)]; | ||
combinedResult.matchedEntities = isPrimary | ||
? [stripSiblingsFromEntity(entity), ...siblings] | ||
: [...siblings, stripSiblingsFromEntity(entity)]; | ||
|
||
combinedResult.matchedEntities = combinedResult.matchedEntities.filter( | ||
(resultToFilter) => (resultToFilter as Dataset).exists, | ||
); | ||
|
||
siblingUrns.forEach((urn) => { | ||
siblingsToPair[urn] = combinedResult; | ||
}); | ||
siblingUrns.forEach((urn) => urn && siblingsToPair.add(urn)); | ||
} | ||
|
||
combinedResults.push(combinedResult); | ||
}); | ||
|
||
return combinedResults; | ||
} | ||
|
||
// used to determine whether sibling entities should be shown merged or not | ||
export const SEPARATE_SIBLINGS_URL_PARAM = 'separate_siblings'; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ import { Input, AutoComplete, Button } from 'antd'; | |
import { CloseCircleFilled, SearchOutlined } from '@ant-design/icons'; | ||
import styled from 'styled-components/macro'; | ||
import { useHistory } from 'react-router'; | ||
import { AutoCompleteResultForEntity, Entity, EntityType, FacetFilterInput, ScenarioType } from '../../types.generated'; | ||
import { AutoCompleteResultForEntity, EntityType, FacetFilterInput, ScenarioType } from '../../types.generated'; | ||
import EntityRegistry from '../entity/EntityRegistry'; | ||
import filterSearchQuery from './utils/filterSearchQuery'; | ||
import { ANTD_GRAY, ANTD_GRAY_V2 } from '../entity/shared/constants'; | ||
|
@@ -23,6 +23,7 @@ import { navigateToSearchUrl } from './utils/navigateToSearchUrl'; | |
import { getQuickFilterDetails } from './autoComplete/quickFilters/utils'; | ||
import ViewAllSearchItem from './ViewAllSearchItem'; | ||
import { ViewSelect } from '../entity/view/select/ViewSelect'; | ||
import { combineSiblingsInAutoComplete } from './utils/combineSiblingsInAutoComplete'; | ||
|
||
const StyledAutoComplete = styled(AutoComplete)` | ||
width: 100%; | ||
|
@@ -88,15 +89,6 @@ const QUICK_FILTER_AUTO_COMPLETE_OPTION = { | |
], | ||
}; | ||
|
||
const renderItem = (query: string, entity: Entity) => { | ||
return { | ||
value: entity.urn, | ||
label: <AutoCompleteItem query={query} entity={entity} />, | ||
type: entity.type, | ||
style: { padding: '12px 12px 12px 16px' }, | ||
}; | ||
}; | ||
|
||
const renderRecommendedQuery = (query: string) => { | ||
return { | ||
value: query, | ||
|
@@ -123,6 +115,7 @@ interface Props { | |
hideRecommendations?: boolean; | ||
showQuickFilters?: boolean; | ||
viewsEnabled?: boolean; | ||
combineSiblings?: boolean; | ||
setIsSearchBarFocused?: (isSearchBarFocused: boolean) => void; | ||
onFocus?: () => void; | ||
onBlur?: () => void; | ||
|
@@ -149,6 +142,7 @@ export const SearchBar = ({ | |
hideRecommendations, | ||
showQuickFilters, | ||
viewsEnabled = false, | ||
combineSiblings = false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we default want to not combine siblings in our search bar? is there somewhere where we particularly don't want them combined? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right the idea was to make siblings an opt-in behavior, where the home/searchResults pages have that behavior but not assume that all use cases in the UI would get that behavior was my thought. |
||
setIsSearchBarFocused, | ||
onFocus, | ||
onBlur, | ||
|
@@ -227,14 +221,26 @@ export const SearchBar = ({ | |
]; | ||
}, [showQuickFilters, suggestions.length, effectiveQuery, selectedQuickFilter, entityRegistry]); | ||
|
||
const autoCompleteEntityOptions = useMemo( | ||
() => | ||
suggestions.map((entity: AutoCompleteResultForEntity) => ({ | ||
label: <SectionHeader entityType={entity.type} />, | ||
options: [...entity.entities.map((e: Entity) => renderItem(effectiveQuery, e))], | ||
})), | ||
[effectiveQuery, suggestions], | ||
); | ||
const autoCompleteEntityOptions = useMemo(() => { | ||
return suggestions.map((suggestion: AutoCompleteResultForEntity) => { | ||
const combinedSuggestion = combineSiblingsInAutoComplete(suggestion, { combine: combineSiblings }); | ||
return { | ||
label: <SectionHeader entityType={combinedSuggestion.type} />, | ||
options: combinedSuggestion.combinedEntities.map((combinedEntity) => ({ | ||
value: combinedEntity.entity.urn, | ||
label: ( | ||
<AutoCompleteItem | ||
query={effectiveQuery} | ||
entity={combinedEntity.entity} | ||
siblings={combineSiblings ? combinedEntity.matchedEntities : undefined} | ||
/> | ||
), | ||
type: combinedEntity.entity.type, | ||
style: { padding: '12px 12px 12px 16px' }, | ||
})), | ||
}; | ||
}); | ||
}, [combineSiblings, effectiveQuery, suggestions]); | ||
|
||
const previousSelectedQuickFilterValue = usePrevious(selectedQuickFilter?.value); | ||
useEffect(() => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import React from 'react'; | ||
import styled from 'styled-components'; | ||
import { Image } from 'antd'; | ||
import { Entity } from '../../../types.generated'; | ||
import { getPlatformName } from '../../entity/shared/utils'; | ||
import { useEntityRegistry } from '../../useEntityRegistry'; | ||
import { IconStyleType } from '../../entity/Entity'; | ||
|
||
const PreviewImage = styled(Image)` | ||
width: auto; | ||
object-fit: contain; | ||
background-color: transparent; | ||
`; | ||
|
||
type Props = { | ||
entity: Entity; | ||
scale: number; | ||
}; | ||
|
||
const DEFAULT_ICON_SIZE = 12; | ||
const DEFAULT_PREVIEW_IMAGE_SIZE = 22; | ||
|
||
const AutoCompleteEntityIcon = ({ entity, scale }: Props) => { | ||
const entityRegistry = useEntityRegistry(); | ||
|
||
const iconFontSize = Math.floor(DEFAULT_ICON_SIZE * scale); | ||
const previewImageSize = Math.floor(DEFAULT_PREVIEW_IMAGE_SIZE * scale); | ||
|
||
const genericEntityProps = entityRegistry.getGenericEntityProperties(entity.type, entity); | ||
const platformLogoUrl = genericEntityProps?.platform?.properties?.logoUrl; | ||
const platformName = getPlatformName(genericEntityProps); | ||
const icon = | ||
(platformLogoUrl && ( | ||
<PreviewImage | ||
preview={false} | ||
src={platformLogoUrl} | ||
alt={platformName || ''} | ||
style={{ | ||
width: previewImageSize, | ||
height: previewImageSize, | ||
}} | ||
/> | ||
)) || | ||
entityRegistry.getIcon(entity.type, iconFontSize, IconStyleType.ACCENT); | ||
|
||
return icon; | ||
}; | ||
|
||
export default AutoCompleteEntityIcon; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like we have a conflict from my changes to display consistent combined search results for sibling entities in this PR: https://github.com/datahub-project/datahub/pull/8602/files#diff-dbeb733a628595000d108fe2f57b5c464d52b6da55f57d57ad4bc6e246af3914
so let's make sure we keep that functionlity! the main thing I did is simply combine the siblings entity data in
combineSiblingsInSearchResults
and refactored a function above it to make it reusable between its two usesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think I got the conflict resolved 👍