From e9e5468a7422efed6e0e9b6c57ac2a785145d790 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 18 Jan 2022 21:13:02 +0800 Subject: [PATCH 1/8] fix: unable to show tooltip on columns and metrics --- .../src/components/ColumnOption.tsx | 10 ++++---- .../src/components/MetricOption.tsx | 6 ++--- .../components/DatasourcePanel/index.tsx | 23 +------------------ 3 files changed, 7 insertions(+), 32 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx index a42f0344acef..68a1c22cff33 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx @@ -27,7 +27,6 @@ import { ColumnMeta } from '../types'; export type ColumnOptionProps = { column: ColumnMeta; showType?: boolean; - showTooltip?: boolean; labelRef?: React.RefObject; }; @@ -41,7 +40,6 @@ export function ColumnOption({ column, labelRef, showType = false, - showTooltip = true, }: ColumnOptionProps) { const { expression, column_name, type_generic } = column; const hasExpression = expression && expression !== column_name; @@ -57,10 +55,10 @@ export function ColumnOption({ details={column.certification_details} /> )} - {showTooltip ? ( + {column.verbose_name ? ( @@ -68,12 +66,12 @@ export function ColumnOption({ className="m-r-5 option-label column-option-label" ref={labelRef} > - {column.verbose_name || column.column_name} + {column.verbose_name} ) : ( - {column.verbose_name || column.column_name} + {column.column_name} )} {column.description && ( diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx index b76f772b43ab..3d80863b8ee3 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx @@ -37,7 +37,6 @@ export interface MetricOptionProps { openInNewWindow?: boolean; showFormula?: boolean; showType?: boolean; - showTooltip?: boolean; url?: string; labelRef?: React.RefObject; } @@ -48,7 +47,6 @@ export function MetricOption({ openInNewWindow = false, showFormula = true, showType = false, - showTooltip = true, url = '', }: MetricOptionProps) { const verbose = metric.verbose_name || metric.metric_name || metric.label; @@ -72,10 +70,10 @@ export function MetricOption({ details={metric.certification_details} /> )} - {showTooltip ? ( + {metric.metric_name ? ( diff --git a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx index 0a3cf7b76674..d6bae2dc7298 100644 --- a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx +++ b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx @@ -123,32 +123,11 @@ const LabelContainer = (props: { className: string; }) => { const labelRef = useRef(null); - const [showTooltip, setShowTooltip] = useState(true); - const isLabelTruncated = () => - !!( - labelRef && - labelRef.current && - labelRef.current.scrollWidth > labelRef.current.clientWidth - ); - const handleShowTooltip = () => { - const shouldShowTooltip = isLabelTruncated(); - if (shouldShowTooltip !== showTooltip) { - setShowTooltip(shouldShowTooltip); - } - }; - const handleResetTooltip = () => { - setShowTooltip(true); - }; const extendedProps = { labelRef, - showTooltip, }; return ( - + {React.cloneElement(props.children, extendedProps)} ); From 412fe3c345255f0766eecf38e83c65acede87644 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 18 Jan 2022 21:38:43 +0800 Subject: [PATCH 2/8] wip --- .../controls/DndColumnSelectControl/OptionWrapper.tsx | 1 - .../explore/components/controls/OptionControls/index.tsx | 8 +------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx index 450a858b312d..18cd57645eca 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx @@ -144,7 +144,6 @@ export default function OptionWrapper( ); diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index 716e10a908b7..0e68b5d4d045 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -280,13 +280,7 @@ export const OptionControlLabel = ({ labelRef.current.scrollWidth > labelRef.current.clientWidth); if (savedMetric && hasMetricName) { - return ( - - ); + return ; } if (!shouldShowTooltip) { return {label}; From 05a6b26ea3da5f5532060e3aaf000bf1796f20f1 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 19 Jan 2022 23:32:53 +0800 Subject: [PATCH 3/8] fix truncated metric or column --- .../src/components/ColumnOption.tsx | 34 +++++----- .../src/components/MetricOption.tsx | 29 ++++---- .../src/components/labelUtils.ts | 67 +++++++++++++++++++ 3 files changed, 98 insertions(+), 32 deletions(-) create mode 100644 superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx index 68a1c22cff33..92b46a2e735a 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx @@ -16,13 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useEffect, useState } from 'react'; import { styled } from '@superset-ui/core'; import { Tooltip } from './Tooltip'; import { ColumnTypeLabel } from './ColumnTypeLabel'; import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; import { ColumnMeta } from '../types'; +import { getColumnLabelText, getColumnTooltipText } from './labelUtils'; export type ColumnOptionProps = { column: ColumnMeta; @@ -44,6 +45,11 @@ export function ColumnOption({ const { expression, column_name, type_generic } = column; const hasExpression = expression && expression !== column_name; const type = hasExpression ? 'expression' : type_generic; + const [tooltipText, setTooltipText] = useState(column.column_name); + + useEffect(() => { + setTooltipText(getColumnTooltipText(column, labelRef)); + }, [labelRef, column]); return ( @@ -55,25 +61,17 @@ export function ColumnOption({ details={column.certification_details} /> )} - {column.verbose_name ? ( - - - {column.verbose_name} - - - ) : ( + - {column.column_name} + {getColumnLabelText(column)} - )} + + {column.description && ( { + setTooltipText(getMeticTooltipText(metric, labelRef)); + }, [labelRef, metric]); + return ( {showType && } @@ -70,22 +77,16 @@ export function MetricOption({ details={metric.certification_details} /> )} - {metric.metric_name ? ( - - - {link} - - - ) : ( + {link} - )} + {metric.description && ( ): boolean => + !!( + labelRef && + labelRef.current && + labelRef.current.scrollWidth > labelRef.current.clientWidth + ); + +export const getColumnLabelText = (column: ColumnMeta): string => + column.verbose_name || column.column_name; + +export const getColumnTooltipText = ( + column: ColumnMeta, + labelRef?: React.RefObject, +): string => { + // don't show tooltip if it hasn't verbose_name and hasn't truncated + if (!column.verbose_name && !isLabelTruncated(labelRef)) { + return ''; + } + + if (isLabelTruncated(labelRef) && column.verbose_name) { + return `verbose name: ${column.verbose_name}`; + } + + return `column name: ${column.column_name}`; +}; + +type MetricType = Omit & { label?: string }; + +export const getMeticTooltipText = ( + metric: MetricType, + labelRef?: React.RefObject, +): string => { + // don't show tooltip if it hasn't verbose_name, label and hasn't truncated + if (!metric.verbose_name && !metric.label && !isLabelTruncated(labelRef)) { + return ''; + } + + if (isLabelTruncated(labelRef) && metric.verbose_name) { + return `verbose name: ${metric.verbose_name}`; + } + + if (isLabelTruncated(labelRef) && metric.label) { + return `label name: ${metric.label}`; + } + + return `metric name: ${metric.metric_name}`; +}; From c8597e3fd3718b9df16c9a97b4022ca06a311eff Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 20 Jan 2022 18:09:25 +0800 Subject: [PATCH 4/8] add test --- .../src/components/labelUtils.ts | 11 +- .../test/components/labelUtils.test.tsx | 155 ++++++++++++++++++ .../emitFilterControl.test.tsx | 2 +- 3 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts index 25d22945cfe5..408bc73b7eed 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { t } from '@superset-ui/core'; import { ColumnMeta, Metric } from '@superset-ui/chart-controls'; export const isLabelTruncated = (labelRef?: React.RefObject): boolean => @@ -38,10 +39,10 @@ export const getColumnTooltipText = ( } if (isLabelTruncated(labelRef) && column.verbose_name) { - return `verbose name: ${column.verbose_name}`; + return t('verbose name: %s', column.verbose_name); } - return `column name: ${column.column_name}`; + return t('column name: %s', column.column_name); }; type MetricType = Omit & { label?: string }; @@ -56,12 +57,12 @@ export const getMeticTooltipText = ( } if (isLabelTruncated(labelRef) && metric.verbose_name) { - return `verbose name: ${metric.verbose_name}`; + return t('verbose name: %s', metric.verbose_name); } if (isLabelTruncated(labelRef) && metric.label) { - return `label name: ${metric.label}`; + return t('label name: %s', metric.label); } - return `metric name: ${metric.metric_name}`; + return t('metric name: %s', metric.metric_name); }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx new file mode 100644 index 000000000000..8d93fbf250e2 --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx @@ -0,0 +1,155 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + getColumnLabelText, + getColumnTooltipText, + getMeticTooltipText, +} from '../../src/components/labelUtils'; + +test("should get column name when column doesn't have verbose_name", () => { + expect( + getColumnLabelText({ + id: 123, + column_name: 'column name', + verbose_name: '', + }), + ).toBe('column name'); +}); + +test('should get verbose name when column have verbose_name', () => { + expect( + getColumnLabelText({ + id: 123, + column_name: 'column name', + verbose_name: 'verbose name', + }), + ).toBe('verbose name'); +}); + +test('should get empty string as tooltip', () => { + const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; + expect( + getColumnTooltipText( + { + id: 123, + column_name: 'column name', + verbose_name: '', + }, + ref, + ), + ).toBe(''); +}); + +test('should get column name as tooltip when it verbose name', () => { + const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; + expect( + getColumnTooltipText( + { + id: 123, + column_name: 'column name', + verbose_name: 'verbose name', + }, + ref, + ), + ).toBe('column name: column name'); +}); + +test('should get column name as tooltip if it overflowed', () => { + const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; + expect( + getColumnTooltipText( + { + id: 123, + column_name: 'long long long long column name', + verbose_name: '', + }, + ref, + ), + ).toBe('column name: long long long long column name'); +}); + +test('should get verbose name as tooltip if it overflowed', () => { + const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; + expect( + getColumnTooltipText( + { + id: 123, + column_name: 'long long long long column name', + verbose_name: 'long long long long verbose name', + }, + ref, + ), + ).toBe('verbose name: long long long long verbose name'); +}); + +test('should get empty string as tooltip in metric', () => { + const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; + expect( + getMeticTooltipText( + { + metric_name: 'count', + label: '', + verbose_name: '', + }, + ref, + ), + ).toBe(''); +}); + +test('should get metric name(sql alias) as tooltip in metric', () => { + const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; + expect( + getMeticTooltipText( + { + metric_name: 'count', + label: 'count(*)', + verbose_name: 'count(*)', + }, + ref, + ), + ).toBe('metric name: count'); +}); + +test('should get verbose name as tooltip in metric if it overflowed', () => { + const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; + expect( + getMeticTooltipText( + { + metric_name: 'count', + label: '', + verbose_name: 'longlonglonglonglong verbose metric', + }, + ref, + ), + ).toBe('verbose name: longlonglonglonglong verbose metric'); +}); + +test('should get label name as tooltip in metric if it overflowed', () => { + const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; + expect( + getMeticTooltipText( + { + metric_name: 'count', + label: 'longlonglonglonglong metric label', + verbose_name: '', + }, + ref, + ), + ).toBe('label name: longlonglonglonglong metric label'); +}); diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/emitFilterControl.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/emitFilterControl.test.tsx index 019263ff8e57..6070ccccfdee 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/emitFilterControl.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/emitFilterControl.test.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { emitFilterControl } from '../../src/shared-controls/emitFilterControl'; +import { emitFilterControl } from '@superset-ui/chart-controls'; describe('isFeatureFlagEnabled', () => { it('returns empty array for unset feature flag', () => { From 55683118e9da89ed6eb94147e898f6f0c8965b59 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 20 Jan 2022 21:43:08 +0800 Subject: [PATCH 5/8] update panel if resize --- .../explore/components/DatasourcePanel/index.tsx | 14 ++++++++++++-- .../components/ExploreViewContainer/index.jsx | 9 ++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx index d6bae2dc7298..caf8c1ff59c6 100644 --- a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx +++ b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx @@ -45,6 +45,8 @@ export interface Props { datasource: DatasourceControl; }; actions: Partial & Pick; + // we use this props control force update when this panel resize + shouldForceUpdate?: number; } const Button = styled.button` @@ -141,6 +143,7 @@ export default function DataSourcePanel({ datasource, controls: { datasource: datasourceControl }, actions, + shouldForceUpdate, }: Props) { const { columns: _columns, metrics } = datasource; @@ -288,7 +291,10 @@ export default function DataSourcePanel({ )} {metricSlice.map(m => ( - + {enableExploreDnd ? ( {columnSlice.map(col => ( - + {enableExploreDnd ? ( )} - setSidebarWidths(LocalStorageKeys.datasource_width, d) - } + onResizeStop={(evt, direction, ref, d) => { + setshouldForceUpdate(d?.width); + setSidebarWidths(LocalStorageKeys.datasource_width, d); + }} defaultSize={{ width: getSidebarWidths(LocalStorageKeys.datasource_width), height: '100%', @@ -559,6 +561,7 @@ function ExploreViewContainer(props) { datasource={props.datasource} controls={props.controls} actions={props.actions} + shouldForceUpdate={shouldForceUpdate} /> {isCollapsed ? ( From cc2b81ca478b68b0da601738e2aab95f54eea81d Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 20 Jan 2022 21:45:59 +0800 Subject: [PATCH 6/8] typo --- .../src/explore/components/ExploreViewContainer/index.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index c003abd5675f..780217bb664f 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -210,7 +210,7 @@ function ExploreViewContainer(props) { const [showingModal, setShowingModal] = useState(false); const [isCollapsed, setIsCollapsed] = useState(false); - const [shouldForceUpdate, setshouldForceUpdate] = useState(-1); + const [shouldForceUpdate, setShouldForceUpdate] = useState(-1); const theme = useTheme(); const width = `${windowSize.width}px`; @@ -528,7 +528,7 @@ function ExploreViewContainer(props) { )} { - setshouldForceUpdate(d?.width); + setShouldForceUpdate(d?.width); setSidebarWidths(LocalStorageKeys.datasource_width, d); }} defaultSize={{ From c6dc69c8134dc7fd676914a7a3a466c6b9c46966 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Mon, 7 Feb 2022 17:42:12 +0800 Subject: [PATCH 7/8] show verbose name and ds name --- .../src/components/ColumnOption.tsx | 8 +-- .../src/components/MetricOption.tsx | 8 +-- .../{labelUtils.ts => labelUtils.tsx} | 33 +++++++--- .../test/components/labelUtils.test.tsx | 66 ++++++++++++++----- 4 files changed, 79 insertions(+), 36 deletions(-) rename superset-frontend/packages/superset-ui-chart-controls/src/components/{labelUtils.ts => labelUtils.tsx} (75%) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx index 92b46a2e735a..6757219e4553 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx @@ -16,14 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, ReactNode } from 'react'; import { styled } from '@superset-ui/core'; import { Tooltip } from './Tooltip'; import { ColumnTypeLabel } from './ColumnTypeLabel'; import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; import { ColumnMeta } from '../types'; -import { getColumnLabelText, getColumnTooltipText } from './labelUtils'; +import { getColumnLabelText, getColumnTooltipNode } from './labelUtils'; export type ColumnOptionProps = { column: ColumnMeta; @@ -45,10 +45,10 @@ export function ColumnOption({ const { expression, column_name, type_generic } = column; const hasExpression = expression && expression !== column_name; const type = hasExpression ? 'expression' : type_generic; - const [tooltipText, setTooltipText] = useState(column.column_name); + const [tooltipText, setTooltipText] = useState(column.column_name); useEffect(() => { - setTooltipText(getColumnTooltipText(column, labelRef)); + setTooltipText(getColumnTooltipNode(column, labelRef)); }, [labelRef, column]); return ( diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx index 68f8e8cf1a00..8d03cf884b26 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx @@ -16,13 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, ReactNode } from 'react'; import { styled, Metric, SafeMarkdown } from '@superset-ui/core'; import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; import { ColumnTypeLabel } from './ColumnTypeLabel'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; import Tooltip from './Tooltip'; -import { getMeticTooltipText } from './labelUtils'; +import { getMeticTooltipNode } from './labelUtils'; const FlexRowContainer = styled.div` align-items: center; @@ -61,10 +61,10 @@ export function MetricOption({ const warningMarkdown = metric.warning_markdown || metric.warning_text; - const [tooltipText, setTooltipText] = useState(metric.metric_name); + const [tooltipText, setTooltipText] = useState(metric.metric_name); useEffect(() => { - setTooltipText(getMeticTooltipText(metric, labelRef)); + setTooltipText(getMeticTooltipNode(metric, labelRef)); }, [labelRef, metric]); return ( diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx similarity index 75% rename from superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts rename to superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx index 408bc73b7eed..eae09652546e 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx @@ -16,6 +16,8 @@ * specific language governing permissions and limitations * under the License. */ +import React, { ReactNode } from 'react'; + import { t } from '@superset-ui/core'; import { ColumnMeta, Metric } from '@superset-ui/chart-controls'; @@ -29,35 +31,46 @@ export const isLabelTruncated = (labelRef?: React.RefObject): boolean => export const getColumnLabelText = (column: ColumnMeta): string => column.verbose_name || column.column_name; -export const getColumnTooltipText = ( +export const getColumnTooltipNode = ( column: ColumnMeta, labelRef?: React.RefObject, -): string => { +): ReactNode => { // don't show tooltip if it hasn't verbose_name and hasn't truncated if (!column.verbose_name && !isLabelTruncated(labelRef)) { - return ''; + return null; } - if (isLabelTruncated(labelRef) && column.verbose_name) { - return t('verbose name: %s', column.verbose_name); + if (column.verbose_name) { + return ( + <> +
{t('column name: %s', column.column_name)}
+
{t('verbose name: %s', column.verbose_name)}
+ + ); } + // show column name in tooltip when column truncated return t('column name: %s', column.column_name); }; type MetricType = Omit & { label?: string }; -export const getMeticTooltipText = ( +export const getMeticTooltipNode = ( metric: MetricType, labelRef?: React.RefObject, -): string => { +): ReactNode => { // don't show tooltip if it hasn't verbose_name, label and hasn't truncated if (!metric.verbose_name && !metric.label && !isLabelTruncated(labelRef)) { - return ''; + return null; } - if (isLabelTruncated(labelRef) && metric.verbose_name) { - return t('verbose name: %s', metric.verbose_name); + if (metric.verbose_name) { + return ( + <> +
{t('metric name: %s', metric.metric_name)}
+
{t('verbose name: %s', metric.verbose_name)}
+ + ); } if (isLabelTruncated(labelRef) && metric.label) { diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx index 8d93fbf250e2..8f31fd3d67c8 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx @@ -16,10 +16,12 @@ * specific language governing permissions and limitations * under the License. */ +import React from 'react'; + import { getColumnLabelText, - getColumnTooltipText, - getMeticTooltipText, + getColumnTooltipNode, + getMeticTooltipNode, } from '../../src/components/labelUtils'; test("should get column name when column doesn't have verbose_name", () => { @@ -42,10 +44,10 @@ test('should get verbose name when column have verbose_name', () => { ).toBe('verbose name'); }); -test('should get empty string as tooltip', () => { +test('should get null as tooltip', () => { const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; expect( - getColumnTooltipText( + getColumnTooltipNode( { id: 123, column_name: 'column name', @@ -53,13 +55,20 @@ test('should get empty string as tooltip', () => { }, ref, ), - ).toBe(''); + ).toBe(null); }); test('should get column name as tooltip when it verbose name', () => { + const rvNode = ( + <> +
column name: column name
+
verbose name: verbose name
+ + ); + const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; expect( - getColumnTooltipText( + getColumnTooltipNode( { id: 123, column_name: 'column name', @@ -67,13 +76,13 @@ test('should get column name as tooltip when it verbose name', () => { }, ref, ), - ).toBe('column name: column name'); + ).toStrictEqual(rvNode); }); test('should get column name as tooltip if it overflowed', () => { const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; expect( - getColumnTooltipText( + getColumnTooltipNode( { id: 123, column_name: 'long long long long column name', @@ -85,9 +94,16 @@ test('should get column name as tooltip if it overflowed', () => { }); test('should get verbose name as tooltip if it overflowed', () => { + const rvNode = ( + <> +
column name: long long long long column name
+
verbose name: long long long long verbose name
+ + ); + const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; expect( - getColumnTooltipText( + getColumnTooltipNode( { id: 123, column_name: 'long long long long column name', @@ -95,13 +111,13 @@ test('should get verbose name as tooltip if it overflowed', () => { }, ref, ), - ).toBe('verbose name: long long long long verbose name'); + ).toStrictEqual(rvNode); }); -test('should get empty string as tooltip in metric', () => { +test('should get null as tooltip in metric', () => { const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; expect( - getMeticTooltipText( + getMeticTooltipNode( { metric_name: 'count', label: '', @@ -109,13 +125,20 @@ test('should get empty string as tooltip in metric', () => { }, ref, ), - ).toBe(''); + ).toBe(null); }); test('should get metric name(sql alias) as tooltip in metric', () => { + const rvNode = ( + <> +
metric name: count
+
verbose name: count(*)
+ + ); + const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; expect( - getMeticTooltipText( + getMeticTooltipNode( { metric_name: 'count', label: 'count(*)', @@ -123,13 +146,20 @@ test('should get metric name(sql alias) as tooltip in metric', () => { }, ref, ), - ).toBe('metric name: count'); + ).toStrictEqual(rvNode); }); test('should get verbose name as tooltip in metric if it overflowed', () => { + const rvNode = ( + <> +
metric name: count
+
verbose name: longlonglonglonglong verbose metric
+ + ); + const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; expect( - getMeticTooltipText( + getMeticTooltipNode( { metric_name: 'count', label: '', @@ -137,13 +167,13 @@ test('should get verbose name as tooltip in metric if it overflowed', () => { }, ref, ), - ).toBe('verbose name: longlonglonglonglong verbose metric'); + ).toStrictEqual(rvNode); }); test('should get label name as tooltip in metric if it overflowed', () => { const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; expect( - getMeticTooltipText( + getMeticTooltipNode( { metric_name: 'count', label: 'longlonglonglonglong metric label', From e4e1eca6901c4c523a4e1b5ba6e98dcc90fc3a0e Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Mon, 7 Feb 2022 21:33:26 +0800 Subject: [PATCH 8/8] typos --- .../src/components/MetricOption.tsx | 4 ++-- .../src/components/labelUtils.tsx | 2 +- .../test/components/labelUtils.test.tsx | 18 +++++++++--------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx index 8d03cf884b26..38b8cb6fc163 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx @@ -22,7 +22,7 @@ import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; import { ColumnTypeLabel } from './ColumnTypeLabel'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; import Tooltip from './Tooltip'; -import { getMeticTooltipNode } from './labelUtils'; +import { getMetricTooltipNode } from './labelUtils'; const FlexRowContainer = styled.div` align-items: center; @@ -64,7 +64,7 @@ export function MetricOption({ const [tooltipText, setTooltipText] = useState(metric.metric_name); useEffect(() => { - setTooltipText(getMeticTooltipNode(metric, labelRef)); + setTooltipText(getMetricTooltipNode(metric, labelRef)); }, [labelRef, metric]); return ( diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx index eae09652546e..ee738e119568 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx @@ -55,7 +55,7 @@ export const getColumnTooltipNode = ( type MetricType = Omit & { label?: string }; -export const getMeticTooltipNode = ( +export const getMetricTooltipNode = ( metric: MetricType, labelRef?: React.RefObject, ): ReactNode => { diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx index 8f31fd3d67c8..b5e5cd6adfa2 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx @@ -21,7 +21,7 @@ import React from 'react'; import { getColumnLabelText, getColumnTooltipNode, - getMeticTooltipNode, + getMetricTooltipNode, } from '../../src/components/labelUtils'; test("should get column name when column doesn't have verbose_name", () => { @@ -58,7 +58,7 @@ test('should get null as tooltip', () => { ).toBe(null); }); -test('should get column name as tooltip when it verbose name', () => { +test('should get column name and verbose name when it has a verbose name', () => { const rvNode = ( <>
column name: column name
@@ -93,7 +93,7 @@ test('should get column name as tooltip if it overflowed', () => { ).toBe('column name: long long long long column name'); }); -test('should get verbose name as tooltip if it overflowed', () => { +test('should get column name and verbose name as tooltip if it overflowed', () => { const rvNode = ( <>
column name: long long long long column name
@@ -117,7 +117,7 @@ test('should get verbose name as tooltip if it overflowed', () => { test('should get null as tooltip in metric', () => { const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; expect( - getMeticTooltipNode( + getMetricTooltipNode( { metric_name: 'count', label: '', @@ -128,7 +128,7 @@ test('should get null as tooltip in metric', () => { ).toBe(null); }); -test('should get metric name(sql alias) as tooltip in metric', () => { +test('should get metric name and verbose name as tooltip in metric', () => { const rvNode = ( <>
metric name: count
@@ -138,7 +138,7 @@ test('should get metric name(sql alias) as tooltip in metric', () => { const ref = { current: { scrollWidth: 100, clientWidth: 100 } }; expect( - getMeticTooltipNode( + getMetricTooltipNode( { metric_name: 'count', label: 'count(*)', @@ -149,7 +149,7 @@ test('should get metric name(sql alias) as tooltip in metric', () => { ).toStrictEqual(rvNode); }); -test('should get verbose name as tooltip in metric if it overflowed', () => { +test('should get metric name and verbose name in tooltip if it overflowed', () => { const rvNode = ( <>
metric name: count
@@ -159,7 +159,7 @@ test('should get verbose name as tooltip in metric if it overflowed', () => { const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; expect( - getMeticTooltipNode( + getMetricTooltipNode( { metric_name: 'count', label: '', @@ -173,7 +173,7 @@ test('should get verbose name as tooltip in metric if it overflowed', () => { test('should get label name as tooltip in metric if it overflowed', () => { const ref = { current: { scrollWidth: 200, clientWidth: 100 } }; expect( - getMeticTooltipNode( + getMetricTooltipNode( { metric_name: 'count', label: 'longlonglonglonglong metric label',