Skip to content
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.

Sign up for GitHub

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

feat(viz-gallery): add 'feature' tag and fuzzy search weighting #18662

Merged
merged 19 commits into from
Mar 11, 2022
Merged

feat(viz-gallery): add 'feature' tag and fuzzy search weighting #18662

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7e33dfc
feat(viz-gallery): add 'feature' tag and fuzzy search weighting
stephenLYZ Feb 10, 2022
8b2f857
add search weight
stephenLYZ Feb 10, 2022
994c400
Update superset-frontend/src/explore/components/controls/VizTypeContr…
stephenLYZ Feb 11, 2022
aad71a0
Update superset-frontend/src/explore/components/controls/VizTypeContr…
stephenLYZ Feb 11, 2022
88c6728
Update superset-frontend/src/explore/components/controls/VizTypeContr…
stephenLYZ Feb 11, 2022
698f8cd
Update superset-frontend/src/explore/components/controls/VizTypeContr…
stephenLYZ Feb 11, 2022
306ffb3
some improvements
stephenLYZ Feb 11, 2022
cab3d57
take metadata out
stephenLYZ Feb 22, 2022
778b59a
use chartLabel enum to unify
stephenLYZ Feb 24, 2022
fb29a0f
Merge branch 'master' into feat-label-search
stephenLYZ Feb 28, 2022
b099697
add test chart
stephenLYZ Mar 8, 2022
7c86a41
Merge branch 'master' into feat-label-search
stephenLYZ Mar 10, 2022
18727a8
Merge branch 'feat-label-search' of github.com:stephenLYZ/superset in…
stephenLYZ Mar 10, 2022
25a1cbe
fix cache
stephenLYZ Mar 10, 2022
3f4a56f
Resolving TS Lint issue
rusackas Mar 10, 2022
653667d
Appeasing the linter
rusackas Mar 10, 2022
910bf2c
Removing one example implementation
rusackas Mar 11, 2022
cf7cb8c
Removing another example label implementation
rusackas Mar 11, 2022
e65142c
Removing the third example label implementation
rusackas Mar 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { Behavior } from '../types/Base';
import { Behavior, ChartLabel } from '../types/Base';

interface LookupTable {
[key: string]: boolean;
Expand All @@ -40,10 +40,13 @@ export interface ChartMetadataConfig {
thumbnail: string;
useLegacyApi?: boolean;
behaviors?: Behavior[];
deprecated?: boolean;
exampleGallery?: ExampleImage[];
tags?: string[];
category?: string | null;
label?: {
name?: ChartLabel;
description?: string;
} | null;
}

export default class ChartMetadata {
Expand Down Expand Up @@ -71,14 +74,17 @@ export default class ChartMetadata {

enableNoResults: boolean;

deprecated: boolean;

exampleGallery: ExampleImage[];

tags: string[];

category: string | null;

label?: {
name?: ChartLabel;
description?: string;
} | null;

constructor(config: ChartMetadataConfig) {
const {
name,
Expand All @@ -92,10 +98,10 @@ export default class ChartMetadata {
behaviors = [],
datasourceCount = 1,
enableNoResults = true,
deprecated = false,
exampleGallery = [],
tags = [],
category = null,
label = null,
} = config;

this.name = name;
Expand All @@ -118,10 +124,10 @@ export default class ChartMetadata {
this.behaviors = behaviors;
this.datasourceCount = datasourceCount;
this.enableNoResults = enableNoResults;
this.deprecated = deprecated;
this.exampleGallery = exampleGallery;
this.tags = tags;
this.category = category;
this.label = label;
}

canBeAnnotationType(type: string): boolean {
Expand Down
18 changes: 18 additions & 0 deletions superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,22 @@ export interface PlainObject {
[key: string]: any;
}

export enum ChartLabel {
VERIFIED = 'VERIFIED',
DEPRECATED = 'DEPRECATED',
FEATURED = 'FEATURED',
}

export const ChartLabelWeight = {
[ChartLabel.DEPRECATED]: {
weight: -0.1,
},
[ChartLabel.VERIFIED]: {
weight: 0.2,
},
[ChartLabel.FEATURED]: {
weight: 0.1,
},
};

export default {};
7 changes: 7 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import {
AnnotationType,
Behavior,
ChartLabel,
ChartMetadata,
ChartPlugin,
FeatureFlag,
Expand Down Expand Up @@ -92,6 +93,12 @@ export default class EchartsTimeseriesBarChartPlugin extends ChartPlugin<
t('Popular'),
],
thumbnail,
label: {
name: ChartLabel.VERIFIED,
description: t(
'This chart was tested and verified, so the overall experience should be stable.',
),
},
}),
transformProps: barTransformProps,
});
Expand Down
7 changes: 7 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Line/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import {
AnnotationType,
Behavior,
ChartLabel,
ChartMetadata,
ChartPlugin,
FeatureFlag,
Expand Down Expand Up @@ -85,6 +86,12 @@ export default class EchartsTimeseriesLineChartPlugin extends ChartPlugin<
t('Popular'),
],
thumbnail,
label: {
name: ChartLabel.FEATURED,
description: t(
'This chart was tested and verified, so the overall experience should be stable.',
),
},
}),
transformProps: lineTransformProps,
});
Expand Down
7 changes: 7 additions & 0 deletions superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/SmoothLine/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import {
AnnotationType,
Behavior,
ChartLabel,
ChartMetadata,
ChartPlugin,
FeatureFlag,
Expand Down Expand Up @@ -85,6 +86,12 @@ export default class EchartsTimeseriesSmoothLineChartPlugin extends ChartPlugin<
t('Transformable'),
],
thumbnail,
label: {
name: ChartLabel.DEPRECATED,
description: t(
'This chart was tested and verified, so the overall experience should be stable.',
),
},
}),
transformProps: smoothTransformProps,
});
Expand Down
7 changes: 6 additions & 1 deletion superset-frontend/plugins/plugin-chart-echarts/src/Treemap/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
* specific language governing permissions and limitations
* under the License.
*/
import { Behavior, ChartMetadata, ChartPlugin, t } from '@superset-ui/core';
import {
Behavior,
ChartMetadata,
ChartPlugin,
t,
} from '@superset-ui/core';
rusackas marked this conversation as resolved.
Show resolved Hide resolved
import buildQuery from './buildQuery';
import controlPanel from './controlPanel';
import transformProps from './transformProps';
Expand Down
73 changes: 71 additions & 2 deletions superset-frontend/src/explore/components/controls/VizTypeControl/VizTypeGallery.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ import {
ChartMetadata,
SupersetTheme,
useTheme,
ChartLabel,
ChartLabelWeight,
} from '@superset-ui/core';
import { AntdCollapse } from 'src/components';
import { Tooltip } from 'src/components/Tooltip';
import { Input } from 'src/components/Input';
import Label from 'src/components/Label';
import { usePluginContext } from 'src/components/DynamicPlugins';
Expand Down Expand Up @@ -310,6 +313,7 @@ const Examples = styled.div`
const thumbnailContainerCss = (theme: SupersetTheme) => css`
cursor: pointer;
width: ${theme.gridUnit * THUMBNAIL_GRID_UNITS}px;
position: relative;

img {
min-width: ${theme.gridUnit * THUMBNAIL_GRID_UNITS}px;
Expand All @@ -333,6 +337,38 @@ const thumbnailContainerCss = (theme: SupersetTheme) => css`
}
`;

const HighlightLabel = styled.div`
${({ theme }) => `
border: 1px solid ${theme.colors.primary.dark1};
box-sizing: border-box;
border-radius: ${theme.gridUnit}px;
background: ${theme.colors.grayscale.light5};
line-height: ${theme.gridUnit * 2.5}px;
color: ${theme.colors.primary.dark1};
font-size: ${theme.typography.sizes.s}px;
font-weight: ${theme.typography.weights.bold};
text-align: center;
padding: ${theme.gridUnit * 0.5}px ${theme.gridUnit}px;
text-transform: uppercase;
cursor: pointer;

div {
transform: scale(0.83,0.83);
}
`}
`;

const ThumbnailLabelWrapper = styled.div`
position: absolute;
right: ${({ theme }) => theme.gridUnit}px;
top: ${({ theme }) => theme.gridUnit * 19}px;
`;

const TitleLabelWrapper = styled.div`
display: inline-block !important;
margin-left: ${({ theme }) => theme.gridUnit * 2}px;
`;

function vizSortFactor(entry: VizEntry) {
if (typesWithDefaultOrder.has(entry.key)) {
return DEFAULT_ORDER.indexOf(entry.key);
Expand Down Expand Up @@ -378,6 +414,13 @@ const Thumbnail: React.FC<ThumbnailProps> = ({
>
{type.name}
</div>
{type.label?.name && (
<ThumbnailLabelWrapper>
<HighlightLabel>
<div>{t(type.label?.name)}</div>
</HighlightLabel>
</ThumbnailLabelWrapper>
)}
</div>
);
};
Expand Down Expand Up @@ -460,7 +503,8 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) {
.map(([key, value]) => ({ key, value }))
.filter(
({ value }) =>
nativeFilterGate(value.behaviors || []) && !value.deprecated,
nativeFilterGate(value.behaviors || []) &&
value.label?.name !== ChartLabel.DEPRECATED,
);
result.sort((a, b) => vizSortFactor(a) - vizSortFactor(b));
return result;
Expand Down Expand Up @@ -545,7 +589,18 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) {
if (searchInputValue.trim() === '') {
return [];
}
return fuse.search(searchInputValue).map(result => result.item);
return fuse
.search(searchInputValue)
.map(result => result.item)
.sort((a, b) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort is new. Before we were using the numeric weighting. Does the weighting still work, or does this search "win" in terms of ranking things?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now we use CHART_LABEL_ORDER array to determine the search weight, and the larger the index, the larger the weight

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of using this sort based on the label's place in the array order, we can define the labels with some basic (flexible/configurable) weighting, like this

const chartLabels = {
    Featured: {
        weight: 0.3
    },
    Depracated: {
        weight: -0.3
    }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better (and I think is sufficient for the feature), but I wish there was a way to avoid using sort and instead have this be just one more factor in the search result, like the weight for keys that fuse supports. This might be a bit heavy-handed in terms of search weighting, but... the feature works!

const aName = a.value?.label?.name;
const bName = b.value?.label?.name;
const aOrder =
aName && ChartLabelWeight[aName] ? ChartLabelWeight[aName].weight : 0;
const bOrder =
bName && ChartLabelWeight[bName] ? ChartLabelWeight[bName].weight : 0;
return bOrder - aOrder;
});
}, [searchInputValue, fuse]);

const focusSearch = useCallback(() => {
Expand Down Expand Up @@ -739,9 +794,23 @@ export default function VizTypeGallery(props: VizTypeGalleryProps) {
<SectionTitle
css={css`
grid-area: viz-name;
position: relative;
`}
>
{selectedVizMetadata?.name}
{selectedVizMetadata?.label?.name && (
<Tooltip
id="viz-badge-tooltip"
placement="top"
title={selectedVizMetadata.label?.description}
>
<TitleLabelWrapper>
<HighlightLabel>
<div>{t(selectedVizMetadata.label?.name)}</div>
</HighlightLabel>
</TitleLabelWrapper>
</Tooltip>
)}
</SectionTitle>
<TagsWrapper>
{selectedVizMetadata?.tags.map(tag => (
Expand Down