From bf5cd42d9963daa5cb1c72442afd60d9325c545c Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Wed, 9 Nov 2022 10:19:41 -0500 Subject: [PATCH 1/8] fix: Drill to detail blocked by tooltip --- .../src/BigNumber/BigNumberWithTrendline/transformProps.ts | 1 - superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts index 96a7d38e9886c..9ad8602e7b38f 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts @@ -232,7 +232,6 @@ export default function transformProps( appendToBody: true, show: !inContextMenu, trigger: 'axis', - confine: true, formatter: renderTooltipFactory(formatTime, headerFormatter), }, aria: { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts b/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts index c74e3d78a0434..5f4cde11e4c4c 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts @@ -23,7 +23,7 @@ export const defaultGrid = { }; export const defaultTooltip = { - confine: true, + confine: false, }; export const defaultYAxis = { From c8ce2ab9a811c857f08eb1550a428776e6a28d09 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Thu, 17 Nov 2022 11:53:52 +0200 Subject: [PATCH 2/8] implement improved positioning algo --- .../plugin-chart-echarts/src/constants.ts | 7 +++ .../plugin-chart-echarts/src/defaults.ts | 55 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts b/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts index ec956a9591764..f25d727e2f5bf 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts @@ -107,3 +107,10 @@ export const DEFAULT_TITLE_FORM_DATA: EchartsTitleFormData = { }; export { DEFAULT_FORM_DATA } from './Timeseries/constants'; + +// How far away from the mouse should the tooltip be +export const TOOLTIP_POINTER_MARGIN = 10; + +// If no satisfactory position can be found, how far away +// from the edge of the window should the tooltip be kept +export const TOOLTIP_OVERFLOW_MARGIN = 5; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts b/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts index 5f4cde11e4c4c..11f85fbbcc9b1 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts @@ -1,4 +1,6 @@ +import { CallbackDataParams } from 'echarts/types/src/util/types'; import { LegendOrientation } from './types'; +import { TOOLTIP_POINTER_MARGIN, TOOLTIP_OVERFLOW_MARGIN } from './constants'; /** * Licensed to the Apache Software Foundation (ASF) under one @@ -24,6 +26,59 @@ export const defaultGrid = { export const defaultTooltip = { confine: false, + position: ( + canvasMousePos: [number, number], + params: CallbackDataParams, + tooltipDom: HTMLElement, + rect: any, + sizes: { contentSize: [number, number]; viewSize: [number, number] }, + ) => { + // algorithm copy-pasted from here: + // https://github.com/apache/echarts/issues/5004#issuecomment-559668309 + + // The chart canvas position + const canvasRect = tooltipDom.parentElement + ?.getElementsByTagName('canvas')[0] + .getBoundingClientRect(); + + // The mouse coordinates relative to the whole window + // The first parameter to the position function is the mouse position relative to the canvas + const mouseX = canvasMousePos[0] + (canvasRect?.x || 0); + const mouseY = canvasMousePos[1] + (canvasRect?.y || 0); + + // The width and height of the tooltip dom element + const tooltipWidth = sizes.contentSize[0]; + const tooltipHeight = sizes.contentSize[1]; + + // Start by placing the tooltip top and right relative to the mouse position + let xPos = mouseX + TOOLTIP_POINTER_MARGIN; + let yPos = mouseY - TOOLTIP_POINTER_MARGIN - tooltipHeight; + + // The tooltip is overflowing past the right edge of the window + if (xPos + tooltipWidth >= document.documentElement.clientWidth) { + // Attempt to place the tooltip to the left of the mouse position + xPos = mouseX - TOOLTIP_POINTER_MARGIN - tooltipWidth; + + // The tooltip is overflowing past the left edge of the window + if (xPos <= 0) + // Place the tooltip a fixed distance from the left edge of the window + xPos = TOOLTIP_OVERFLOW_MARGIN; + } + + // The tooltip is overflowing past the top edge of the window + if (yPos <= 0) { + // Attempt to place the tooltip to the bottom of the mouse position + yPos = mouseY + TOOLTIP_POINTER_MARGIN; + + // The tooltip is overflowing past the bottom edge of the window + if (yPos + tooltipHeight >= document.documentElement.clientHeight) + // Place the tooltip a fixed distance from the top edge of the window + yPos = TOOLTIP_OVERFLOW_MARGIN; + } + + // Return the position (converted back to a relative position on the canvas) + return [xPos - (canvasRect?.x || 0), yPos - (canvasRect?.y || 0)]; + }, }; export const defaultYAxis = { From 45c7df7ae937e3200c03bf766b34d702be070af8 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 23 Nov 2022 09:58:34 +0200 Subject: [PATCH 3/8] add reference to chart div --- .../plugins/plugin-chart-echarts/package.json | 1 + .../src/BoxPlot/EchartsBoxPlot.tsx | 2 + .../src/BoxPlot/transformProps.ts | 8 ++- .../plugin-chart-echarts/src/BoxPlot/types.ts | 21 ++++--- .../src/Funnel/EchartsFunnel.tsx | 2 + .../src/Funnel/transformProps.ts | 8 ++- .../plugin-chart-echarts/src/Funnel/types.ts | 22 +++---- .../src/Gauge/EchartsGauge.tsx | 2 + .../src/Gauge/transformProps.ts | 6 ++ .../plugin-chart-echarts/src/Gauge/types.ts | 19 +++--- .../src/Graph/EchartsGraph.tsx | 2 + .../src/Graph/transformProps.ts | 9 ++- .../plugin-chart-echarts/src/Graph/types.ts | 29 +++++----- .../EchartsMixedTimeseries.tsx | 6 +- .../src/MixedTimeseries/transformProps.ts | 13 ++++- .../src/MixedTimeseries/types.ts | 43 +++++++------- .../src/Pie/EchartsPie.tsx | 2 + .../src/Pie/transformProps.ts | 8 ++- .../plugin-chart-echarts/src/Pie/types.ts | 24 ++++---- .../src/Radar/EchartsRadar.tsx | 2 + .../src/Radar/transformProps.ts | 8 ++- .../plugin-chart-echarts/src/Radar/types.ts | 19 +++--- .../src/Timeseries/EchartsTimeseries.tsx | 5 +- .../src/Timeseries/transformProps.ts | 9 ++- .../src/Timeseries/types.ts | 40 +++++++------ .../src/Tree/EchartsTree.tsx | 12 +++- .../src/Tree/transformProps.ts | 15 +++-- .../plugin-chart-echarts/src/Tree/types.ts | 14 +++++ .../src/Treemap/EchartsTreemap.tsx | 14 +++-- .../src/Treemap/transformProps.ts | 8 ++- .../plugin-chart-echarts/src/Treemap/types.ts | 11 +++- .../src/components/Echart.tsx | 5 ++ .../plugin-chart-echarts/src/constants.ts | 8 +-- .../plugin-chart-echarts/src/defaults.ts | 1 - .../plugins/plugin-chart-echarts/src/types.ts | 45 ++++++++++---- .../plugin-chart-echarts/src/utils/tooltip.ts | 58 +++++++++++++++++++ 36 files changed, 346 insertions(+), 155 deletions(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts diff --git a/superset-frontend/plugins/plugin-chart-echarts/package.json b/superset-frontend/plugins/plugin-chart-echarts/package.json index 02da13b0d12bc..b370863cb9bb2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/package.json +++ b/superset-frontend/plugins/plugin-chart-echarts/package.json @@ -34,6 +34,7 @@ "peerDependencies": { "@superset-ui/chart-controls": "*", "@superset-ui/core": "*", + "zrender": "*", "react": "^16.13.1" } } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx index 29c4e2a6e62a9..135d31317e227 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/EchartsBoxPlot.tsx @@ -31,6 +31,7 @@ export default function EchartsBoxPlot(props: BoxPlotChartTransformedProps) { groupby, selectedValues, formData, + refs, } = props; const handleChange = useCallback( (values: string[]) => { @@ -72,6 +73,7 @@ export default function EchartsBoxPlot(props: BoxPlotChartTransformedProps) { return ( { + extends BaseChartProps { formData: BoxPlotQueryFormData; - queriesData: ChartDataResponseResult[]; } export type BoxPlotChartTransformedProps = - EChartTransformedProps; + BaseTransformedProps & + CrossFilterTransformedProps & + ContextMenuTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx index 52de923659ff9..c492500e8e9a7 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/EchartsFunnel.tsx @@ -31,6 +31,7 @@ export default function EchartsFunnel(props: FunnelChartTransformedProps) { groupby, selectedValues, formData, + refs, } = props; const handleChange = useCallback( (values: string[]) => { @@ -72,6 +73,7 @@ export default function EchartsFunnel(props: FunnelChartTransformedProps) { return ( @@ -212,7 +215,7 @@ export default function transformProps( ...defaultGrid, }, tooltip: { - ...defaultTooltip, + position: getDefaultPosition(refs), show: !inContextMenu, trigger: 'item', formatter: (params: any) => @@ -240,5 +243,6 @@ export default function transformProps( groupby, selectedValues, onContextMenu, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/types.ts index 0d1f3caf5e5ad..841722ce4bc5b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Funnel/types.ts @@ -16,21 +16,20 @@ * specific language governing permissions and limitations * under the License. */ +import { QueryFormData } from '@superset-ui/core'; import { - ChartDataResponseResult, - ChartProps, - QueryFormData, -} from '@superset-ui/core'; -import { - EchartsLegendFormData, - EChartTransformedProps, + BaseChartProps, + BaseTransformedProps, + ContextMenuTransformedProps, + CrossFilterTransformedProps, + LegendFormData, LegendOrientation, LegendType, } from '../types'; import { DEFAULT_LEGEND_FORM_DATA } from '../constants'; export type EchartsFunnelFormData = QueryFormData & - EchartsLegendFormData & { + LegendFormData & { colorScheme?: string; groupby: QueryFormData[]; labelLine: boolean; @@ -54,9 +53,8 @@ export enum EchartsFunnelLabelTypeType { } export interface EchartsFunnelChartProps - extends ChartProps { + extends BaseChartProps { formData: EchartsFunnelFormData; - queriesData: ChartDataResponseResult[]; } // @ts-ignore @@ -76,4 +74,6 @@ export const DEFAULT_FORM_DATA: EchartsFunnelFormData = { }; export type FunnelChartTransformedProps = - EChartTransformedProps; + BaseTransformedProps & + CrossFilterTransformedProps & + ContextMenuTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx index 118783b47eb96..7ffb571b79481 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/EchartsGauge.tsx @@ -31,6 +31,7 @@ export default function EchartsGauge(props: GaugeChartTransformedProps) { groupby, selectedValues, formData: { emitFilter }, + refs, } = props; const handleChange = useCallback( (values: string[]) => { @@ -72,6 +73,7 @@ export default function EchartsGauge(props: GaugeChartTransformedProps) { return ( { const { name, value } = params; return `${name} : ${formatValue(value as number)}`; @@ -300,6 +304,7 @@ export default function transformProps( axisTick, pointer, detail, + // @ts-ignore tooltip, radius: Math.min(width, height) / 2 - axisLabelDistance - axisTickDistance, @@ -327,5 +332,6 @@ export default function transformProps( groupby, selectedValues: filterState.selectedValues || [], onContextMenu, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/types.ts index 4824d579c4040..9f2c08fd5f0b0 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Gauge/types.ts @@ -16,13 +16,13 @@ * specific language governing permissions and limitations * under the License. */ +import { QueryFormColumn, QueryFormData } from '@superset-ui/core'; import { - ChartDataResponseResult, - ChartProps, - QueryFormColumn, - QueryFormData, -} from '@superset-ui/core'; -import { EChartTransformedProps } from '../types'; + BaseChartProps, + BaseTransformedProps, + ContextMenuTransformedProps, + CrossFilterTransformedProps, +} from '../types'; import { DEFAULT_LEGEND_FORM_DATA } from '../constants'; export type AxisTickLineStyle = { @@ -80,10 +80,11 @@ export const DEFAULT_FORM_DATA: Partial = { }; export interface EchartsGaugeChartProps - extends ChartProps { + extends BaseChartProps { formData: EchartsGaugeFormData; - queriesData: ChartDataResponseResult[]; } export type GaugeChartTransformedProps = - EChartTransformedProps; + BaseTransformedProps & + ContextMenuTransformedProps & + CrossFilterTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx index 0f09fe2386f27..4f83d1bcaf578 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/EchartsGraph.tsx @@ -34,6 +34,7 @@ export default function EchartsGraph({ echartOptions, formData, onContextMenu, + refs, }: GraphChartTransformedProps) { const eventHandlers: EventHandlers = { contextmenu: (e: Event) => { @@ -68,6 +69,7 @@ export default function EchartsGraph({ }; return ( ; @@ -158,7 +160,7 @@ function getCategoryName(columnName: string, name?: DataRecordValue) { } export default function transformProps( - chartProps: ChartProps, + chartProps: EchartsGraphChartProps, ): GraphChartTransformedProps { const { width, height, formData, queriesData, hooks, inContextMenu } = chartProps; @@ -294,11 +296,13 @@ export default function transformProps( }, ]; + const refs: Refs = {}; const echartOptions: EChartsCoreOption = { animationDuration: DEFAULT_GRAPH_SERIES_OPTION.animationDuration, animationEasing: DEFAULT_GRAPH_SERIES_OPTION.animationEasing, tooltip: { show: !inContextMenu, + position: getDefaultPosition(refs), formatter: (params: any): string => edgeFormatter( params.data.source, @@ -322,5 +326,6 @@ export default function transformProps( formData, echartOptions, onContextMenu, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts index 70a068c977d3c..95dc386eb238a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/types.ts @@ -16,16 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import { - PlainObject, - QueryFormData, - BinaryQueryObjectFilterClause, -} from '@superset-ui/core'; +import { QueryFormData } from '@superset-ui/core'; import { GraphNodeItemOption } from 'echarts/types/src/chart/graph/GraphSeries'; import { SeriesTooltipOption } from 'echarts/types/src/util/types'; import { - EchartsLegendFormData, - EchartsProps, + BaseChartProps, + BaseTransformedProps, + ContextMenuTransformedProps, + LegendFormData, LegendOrientation, LegendType, } from '../types'; @@ -34,7 +32,7 @@ import { DEFAULT_LEGEND_FORM_DATA } from '../constants'; export type EdgeSymbol = 'none' | 'circle' | 'arrow'; export type EchartsGraphFormData = QueryFormData & - EchartsLegendFormData & { + LegendFormData & { source: string; target: string; sourceCategory?: string; @@ -85,11 +83,10 @@ export type tooltipFormatParams = { data: { [name: string]: string }; }; -export type GraphChartTransformedProps = EchartsProps & { - formData: PlainObject; - onContextMenu?: ( - clientX: number, - clientY: number, - filters?: BinaryQueryObjectFilterClause[], - ) => void; -}; +export interface EchartsGraphChartProps + extends BaseChartProps { + formData: EchartsGraphFormData; +} + +export type GraphChartTransformedProps = + BaseTransformedProps & ContextMenuTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx index 8a5421d21763a..1d1cd6547752d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx @@ -43,6 +43,7 @@ export default function EchartsMixedTimeseries({ onContextMenu, xValueFormatter, xAxis, + refs, }: EchartsMixedTimeseriesChartTransformedProps) { const isFirstQuery = useCallback( (seriesIndex: number) => seriesIndex < seriesBreakdown, @@ -61,7 +62,7 @@ export default function EchartsMixedTimeseries({ const currentGroupBy = isFirstQuery(seriesIndex) ? groupby : groupbyB; const currentLabelMap = isFirstQuery(seriesIndex) ? labelMap : labelMapB; const groupbyValues = values - .map(value => currentLabelMap[value]) + .map(value => currentLabelMap?.[value]) .filter(value => !!value); setDataMask({ @@ -100,7 +101,7 @@ export default function EchartsMixedTimeseries({ const eventHandlers: EventHandlers = { click: props => { const { seriesName, seriesIndex } = props; - const values: string[] = Object.values(selectedValues); + const values: string[] = Object.values(selectedValues || {}); if (values.includes(seriesName)) { handleChange( values.filter(v => v !== seriesName), @@ -162,6 +163,7 @@ export default function EchartsMixedTimeseries({ return ( { formData: EchartsMixedTimeseriesFormData; - queriesData: ChartDataResponseResult[]; } export type EchartsMixedTimeseriesChartTransformedProps = - EChartTransformedProps & { - emitFilterB: boolean; - groupbyB: QueryFormColumn[]; - labelMapB: Record; - seriesBreakdown: number; - xValueFormatter: TimeFormatter | StringConstructor; - xAxis: { - label: string; - type: AxisType; + BaseTransformedProps & + ContextMenuTransformedProps & + CrossFilterTransformedProps & { + emitFilterB: boolean; + groupbyB: QueryFormColumn[]; + labelMapB: Record; + seriesBreakdown: number; + xValueFormatter: TimeFormatter | StringConstructor; + xAxis: { + label: string; + type: AxisType; + }; }; - }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/EchartsPie.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/EchartsPie.tsx index 37606ac6f8764..6de4c8423dc49 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/EchartsPie.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/EchartsPie.tsx @@ -31,6 +31,7 @@ export default function EchartsPie(props: PieChartTransformedProps) { groupby, selectedValues, formData, + refs, } = props; const handleChange = useCallback( (values: string[]) => { @@ -72,6 +73,7 @@ export default function EchartsPie(props: PieChartTransformedProps) { return ( formatPieLabel({ @@ -341,5 +344,6 @@ export default function transformProps( groupby, selectedValues, onContextMenu, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts index e31127f7b39f3..4b45751a23cb0 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/types.ts @@ -16,22 +16,20 @@ * specific language governing permissions and limitations * under the License. */ +import { QueryFormColumn, QueryFormData } from '@superset-ui/core'; import { - ChartDataResponseResult, - ChartProps, - QueryFormColumn, - QueryFormData, -} from '@superset-ui/core'; -import { - EchartsLegendFormData, - EChartTransformedProps, + BaseChartProps, + BaseTransformedProps, + ContextMenuTransformedProps, + CrossFilterTransformedProps, + LegendFormData, LegendOrientation, LegendType, } from '../types'; import { DEFAULT_LEGEND_FORM_DATA } from '../constants'; export type EchartsPieFormData = QueryFormData & - EchartsLegendFormData & { + LegendFormData & { colorScheme?: string; currentOwnValue?: string[] | null; donut: boolean; @@ -59,9 +57,9 @@ export enum EchartsPieLabelType { KeyValuePercent = 'key_value_percent', } -export interface EchartsPieChartProps extends ChartProps { +export interface EchartsPieChartProps + extends BaseChartProps { formData: EchartsPieFormData; - queriesData: ChartDataResponseResult[]; } // @ts-ignore @@ -84,4 +82,6 @@ export const DEFAULT_FORM_DATA: EchartsPieFormData = { }; export type PieChartTransformedProps = - EChartTransformedProps; + BaseTransformedProps & + ContextMenuTransformedProps & + CrossFilterTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx index bcba60f5cbbae..1291b69c12404 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Radar/EchartsRadar.tsx @@ -31,6 +31,7 @@ export default function EchartsRadar(props: RadarChartTransformedProps) { groupby, selectedValues, formData, + refs, } = props; const handleChange = useCallback( (values: string[]) => { @@ -72,6 +73,7 @@ export default function EchartsRadar(props: RadarChartTransformedProps) { return ( ; export type EchartsRadarFormData = QueryFormData & - EchartsLegendFormData & { + LegendFormData & { colorScheme?: string; columnConfig?: RadarColumnConfig; currentOwnValue?: string[] | null; @@ -57,9 +58,9 @@ export enum EchartsRadarLabelType { KeyValue = 'key_value', } -export interface EchartsRadarChartProps extends ChartProps { +export interface EchartsRadarChartProps + extends BaseChartProps { formData: EchartsRadarFormData; - queriesData: ChartDataResponseResult[]; } // @ts-ignore @@ -78,4 +79,6 @@ export const DEFAULT_FORM_DATA: EchartsRadarFormData = { }; export type RadarChartTransformedProps = - EChartTransformedProps; + BaseTransformedProps & + ContextMenuTransformedProps & + CrossFilterTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx index a178e5d05dd41..2ffdb0e87bdc9 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx @@ -48,9 +48,12 @@ export default function EchartsTimeseries({ onContextMenu, xValueFormatter, xAxis, + refs, }: TimeseriesChartTransformedProps) { const { emitFilter, stack } = formData; const echartRef = useRef(null); + // eslint-disable-next-line no-param-reassign + refs.echartRef = echartRef; const lastTimeRef = useRef(Date.now()); const lastSelectedLegend = useRef(''); const clickTimer = useRef>(); @@ -256,7 +259,7 @@ export default function EchartsTimeseries({ { @@ -463,5 +465,6 @@ export default function transformProps( label: xAxisLabel, type: xAxisType, }, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts index f8545307efb38..82a204e38d7f9 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts @@ -16,22 +16,24 @@ * specific language governing permissions and limitations * under the License. */ +import { OptionName } from 'echarts/types/src/util/types'; import { AnnotationLayer, - ChartDataResponseResult, - ChartProps, + AxisType, + ContributionType, QueryFormColumn, QueryFormData, - TimeGranularity, - ContributionType, TimeFormatter, - AxisType, + TimeGranularity, } from '@superset-ui/core'; import { - EchartsLegendFormData, - EChartTransformedProps, - EchartsTitleFormData, + BaseChartProps, + BaseTransformedProps, + ContextMenuTransformedProps, + CrossFilterTransformedProps, + LegendFormData, StackType, + TitleFormData, } from '../types'; export enum OrientationType { @@ -85,20 +87,22 @@ export type EchartsTimeseriesFormData = QueryFormData & { showExtraControls: boolean; percentageThreshold: number; orientation?: OrientationType; -} & EchartsLegendFormData & - EchartsTitleFormData; +} & LegendFormData & + TitleFormData; export interface EchartsTimeseriesChartProps - extends ChartProps { + extends BaseChartProps { formData: EchartsTimeseriesFormData; - queriesData: ChartDataResponseResult[]; } export type TimeseriesChartTransformedProps = - EChartTransformedProps & { - xValueFormatter: TimeFormatter | StringConstructor; - xAxis: { - label: string; - type: AxisType; + BaseTransformedProps & + ContextMenuTransformedProps & + CrossFilterTransformedProps & { + legendData?: OptionName[]; + xValueFormatter: TimeFormatter | StringConstructor; + xAxis: { + label: string; + type: AxisType; + }; }; - }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/EchartsTree.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/EchartsTree.tsx index 9b42c6e553576..b1b3f8e896e04 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/EchartsTree.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/EchartsTree.tsx @@ -21,9 +21,17 @@ import { EchartsProps } from '../types'; import Echart from '../components/Echart'; export default function EchartsGraph({ + echartOptions, height, + refs, width, - echartOptions, }: EchartsProps) { - return ; + return ( + + ); } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts index c89fbe8b37049..4648b3c0d6a04 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { ChartProps, getMetricLabel, DataRecordValue } from '@superset-ui/core'; +import { getMetricLabel, DataRecordValue } from '@superset-ui/core'; import { EChartsCoreOption, TreeSeriesOption } from 'echarts'; import { TreeSeriesCallbackDataParams, @@ -24,12 +24,14 @@ import { } from 'echarts/types/src/chart/tree/TreeSeries'; import { OptionName } from 'echarts/types/src/util/types'; import { - EchartsTreeFormData, DEFAULT_FORM_DATA as DEFAULT_GRAPH_FORM_DATA, + EchartsTreeChartProps, + EchartsTreeFormData, TreeDataRecord, + TreeTransformedProps, } from './types'; import { DEFAULT_TREE_SERIES_OPTION } from './constants'; -import { EchartsProps } from '../types'; +import { Refs } from '../types'; export function formatTooltip({ params, @@ -49,8 +51,11 @@ export function formatTooltip({ ].join(''); } -export default function transformProps(chartProps: ChartProps): EchartsProps { +export default function transformProps( + chartProps: EchartsTreeChartProps, +): TreeTransformedProps { const { width, height, formData, queriesData } = chartProps; + const refs: Refs = {}; const data: TreeDataRecord[] = queriesData[0].data || []; const { @@ -212,8 +217,10 @@ export default function transformProps(chartProps: ChartProps): EchartsProps { }; return { + formData, width, height, echartOptions, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts index 81db2d59508fd..d965a602e4be2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts @@ -16,7 +16,9 @@ * specific language governing permissions and limitations * under the License. */ +import { ChartDataResponseResult } from '@superset-ui/core'; import { TreeSeriesNodeItemOption } from 'echarts/types/src/chart/tree/TreeSeries'; +import { BaseChartProps, BaseTransformedProps } from '../types'; export type EchartsTreeFormData = { id: string; @@ -50,6 +52,18 @@ export const DEFAULT_FORM_DATA: EchartsTreeFormData = { emphasis: 'descendant', }; +interface TreeChartDataResponseResult extends ChartDataResponseResult { + data: TreeDataRecord[]; +} + +export interface EchartsTreeChartProps + extends BaseChartProps { + formData: EchartsTreeFormData; + queriesData: TreeChartDataResponseResult[]; +} + export type TreeDataRecord = Record & { children: TreeSeriesNodeItemOption[]; }; + +export type TreeTransformedProps = BaseTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx index 1ff112cedd65a..8559688939b06 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/EchartsTreemap.tsx @@ -28,15 +28,16 @@ import { extractTreePathInfo } from './constants'; import { TreemapTransformedProps } from './types'; export default function EchartsTreemap({ - height, - width, echartOptions, - setDataMask, - labelMap, - groupby, - selectedValues, formData, + groupby, + height, + labelMap, onContextMenu, + refs, + setDataMask, + selectedValues, + width, }: TreemapTransformedProps) { const handleChange = useCallback( (values: string[]) => { @@ -113,6 +114,7 @@ export default function EchartsTreemap({ return ( @@ -309,7 +310,7 @@ export default function transformProps( const echartOptions: EChartsCoreOption = { tooltip: { - ...defaultTooltip, + position: getDefaultPosition(refs), show: !inContextMenu, trigger: 'item', formatter: (params: any) => @@ -332,5 +333,6 @@ export default function transformProps( groupby, selectedValues: filterState.selectedValues || [], onContextMenu, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/types.ts index 9120fb72f726d..c318b2ac2a366 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/types.ts @@ -24,7 +24,12 @@ import { QueryFormMetric, } from '@superset-ui/core'; import { CallbackDataParams } from 'echarts/types/src/util/types'; -import { EChartTransformedProps, LabelPositionEnum } from '../types'; +import { + BaseTransformedProps, + ContextMenuTransformedProps, + CrossFilterTransformedProps, + LabelPositionEnum, +} from '../types'; export type EchartsTreemapFormData = QueryFormData & { colorScheme?: string; @@ -73,4 +78,6 @@ export interface TreemapSeriesCallbackDataParams extends CallbackDataParams { } export type TreemapTransformedProps = - EChartTransformedProps; + BaseTransformedProps & + ContextMenuTransformedProps & + CrossFilterTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx index 0ef7704311c21..aaae39b8bde3e 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx @@ -42,10 +42,15 @@ function Echart( eventHandlers, zrEventHandlers, selectedValues = {}, + refs, }: EchartsProps, ref: React.Ref, ) { const divRef = useRef(null); + if (refs) { + // eslint-disable-next-line no-param-reassign + refs.divRef = divRef; + } const chartRef = useRef(); const currentSelection = useMemo( () => Object.keys(selectedValues) || [], diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts b/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts index f25d727e2f5bf..1c20128b67c64 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/constants.ts @@ -20,8 +20,8 @@ import { JsonValue, t, TimeGranularity } from '@superset-ui/core'; import { ReactNode } from 'react'; import { - EchartsLegendFormData, - EchartsTitleFormData, + LegendFormData, + TitleFormData, LabelPositionEnum, LegendOrientation, LegendType, @@ -91,14 +91,14 @@ export const TIMEGRAIN_TO_TIMESTAMP = { [TimeGranularity.YEAR]: 3600 * 1000 * 24 * 31 * 12, }; -export const DEFAULT_LEGEND_FORM_DATA: EchartsLegendFormData = { +export const DEFAULT_LEGEND_FORM_DATA: LegendFormData = { legendMargin: null, legendOrientation: LegendOrientation.Top, legendType: LegendType.Scroll, showLegend: true, }; -export const DEFAULT_TITLE_FORM_DATA: EchartsTitleFormData = { +export const DEFAULT_TITLE_FORM_DATA: TitleFormData = { xAxisTitle: '', xAxisTitleMargin: 0, yAxisTitle: '', diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts b/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts index 11f85fbbcc9b1..d76de5b53db84 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/defaults.ts @@ -25,7 +25,6 @@ export const defaultGrid = { }; export const defaultTooltip = { - confine: false, position: ( canvasMousePos: [number, number], params: CallbackDataParams, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts index 8c20543e78e84..57ed645839992 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts @@ -16,15 +16,17 @@ * specific language governing permissions and limitations * under the License. */ +import React, { RefObject } from 'react'; import { + BinaryQueryObjectFilterClause, + ChartDataResponseResult, + ChartProps, HandlerFunction, QueryFormColumn, - BinaryQueryObjectFilterClause, SetDataMaskHook, } from '@superset-ui/core'; import { EChartsCoreOption, ECharts } from 'echarts'; import { TooltipMarker } from 'echarts/types/src/util/format'; -import { OptionName } from 'echarts/types/src/util/types'; import { AreaChartExtraControlsValue } from './constants'; export type EchartsStylesProps = { @@ -32,6 +34,11 @@ export type EchartsStylesProps = { width: number; }; +export type Refs = { + echartRef?: React.Ref; + divRef?: RefObject; +}; + export interface EchartsProps { height: number; width: number; @@ -40,6 +47,7 @@ export interface EchartsProps { zrEventHandlers?: EventHandlers; selectedValues?: Record; forceClear?: boolean; + refs: Refs; } export interface EchartsHandler { @@ -78,7 +86,7 @@ export type ForecastValue = { forecastUpper?: number; }; -export type EchartsLegendFormData = { +export type LegendFormData = { legendMargin: number | null | string; legendOrientation: LegendOrientation; legendType: LegendType; @@ -103,26 +111,41 @@ export enum LabelPositionEnum { InsideBottomRight = 'insideBottomRight', } -export interface EChartTransformedProps { +export interface BaseChartProps extends ChartProps { + queriesData: ChartDataResponseResult[]; +} + +export interface BaseTransformedProps { + echartOptions: EChartsCoreOption; formData: F; height: number; + onContextMenu?: ( + clientX: number, + clientY: number, + filters?: BinaryQueryObjectFilterClause[], + ) => void; + refs: Refs; width: number; - echartOptions: EChartsCoreOption; +} + +export type CrossFilterTransformedProps = { emitFilter: boolean; - setDataMask: SetDataMaskHook; - setControlValue?: HandlerFunction; - labelMap: Record; groupby: QueryFormColumn[]; + labelMap: Record; + setControlValue?: HandlerFunction; + setDataMask: SetDataMaskHook; selectedValues: Record; - legendData?: OptionName[]; +}; + +export type ContextMenuTransformedProps = { onContextMenu?: ( clientX: number, clientY: number, filters?: BinaryQueryObjectFilterClause[], ) => void; -} +}; -export interface EchartsTitleFormData { +export interface TitleFormData { xAxisTitle: string; xAxisTitleMargin: number; yAxisTitle: string; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts new file mode 100644 index 0000000000000..2b445855184b4 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts @@ -0,0 +1,58 @@ +import ZRText from 'zrender/src/graphic/Text'; +import { CallbackDataParams } from 'echarts/types/src/util/types'; +import { TOOLTIP_OVERFLOW_MARGIN, TOOLTIP_POINTER_MARGIN } from '../constants'; +import { Refs } from '../types'; + +export function getDefaultPosition(refs: Refs) { + return ( + canvasMousePos: [number, number], + params: CallbackDataParams, + tooltipDom: ZRText | HTMLDivElement | null, + rect: any, + sizes: { contentSize: [number, number]; viewSize: [number, number] }, + ) => { + // algorithm partially based on this snippet: + // https://github.com/apache/echarts/issues/5004#issuecomment-559668309 + + // The chart canvas position + const divRect = refs.divRef?.current?.getBoundingClientRect(); + + // The mouse coordinates relative to the whole window + // The first parameter to the position function is the mouse position relative to the canvas + const mouseX = canvasMousePos[0] + (divRect?.x || 0); + const mouseY = canvasMousePos[1] + (divRect?.y || 0); + + // The width and height of the tooltip dom element + const tooltipWidth = sizes.contentSize[0]; + const tooltipHeight = sizes.contentSize[1]; + + // Start by placing the tooltip top and right relative to the mouse position + let xPos = mouseX + TOOLTIP_POINTER_MARGIN; + let yPos = mouseY - TOOLTIP_POINTER_MARGIN - tooltipHeight; + + // The tooltip is overflowing past the right edge of the window + if (xPos + tooltipWidth >= document.documentElement.clientWidth) { + // Attempt to place the tooltip to the left of the mouse position + xPos = mouseX - TOOLTIP_POINTER_MARGIN - tooltipWidth; + + // The tooltip is overflowing past the left edge of the window + if (xPos <= 0) + // Place the tooltip a fixed distance from the left edge of the window + xPos = TOOLTIP_OVERFLOW_MARGIN; + } + + // The tooltip is overflowing past the top edge of the window + if (yPos <= 0) { + // Attempt to place the tooltip to the bottom of the mouse position + yPos = mouseY + TOOLTIP_POINTER_MARGIN; + + // The tooltip is overflowing past the bottom edge of the window + if (yPos + tooltipHeight >= document.documentElement.clientHeight) + // Place the tooltip a fixed distance from the top edge of the window + yPos = TOOLTIP_OVERFLOW_MARGIN; + } + + // Return the position (converted back to a relative position on the canvas) + return [xPos - (divRect?.x || 0), yPos - (divRect?.y || 0)]; + }; +} From f8a1e8ddbb0313f8c1dab8efcffcb3957abfe963 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 23 Nov 2022 10:06:29 +0200 Subject: [PATCH 4/8] add license header --- .../plugin-chart-echarts/src/utils/tooltip.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts index 2b445855184b4..1c116cffd598d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts @@ -1,3 +1,22 @@ +/** + * 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 ZRText from 'zrender/src/graphic/Text'; import { CallbackDataParams } from 'echarts/types/src/util/types'; import { TOOLTIP_OVERFLOW_MARGIN, TOOLTIP_POINTER_MARGIN } from '../constants'; From 93ce8da90035a072859922c1b8cf56082a16cd7d Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 23 Nov 2022 12:37:34 +0200 Subject: [PATCH 5/8] fix tests and do more cleanup --- .../plugins/plugin-chart-echarts/package.json | 1 - .../BigNumberTotal/transformProps.ts | 9 +- .../src/BigNumber/BigNumberViz.tsx | 70 ++++------ .../BigNumberWithTrendline/transformProps.ts | 7 +- .../src/BigNumber/types.ts | 60 ++++++-- .../src/Tree/constants.ts | 16 +++ .../src/Tree/controlPanel.tsx | 2 +- .../src/Tree/transformProps.ts | 5 +- .../plugin-chart-echarts/src/Tree/types.ts | 26 +--- .../src/utils/eventHandlers.ts | 13 +- .../plugin-chart-echarts/src/utils/tooltip.ts | 3 +- .../test/BigNumber/transformProps.test.ts | 8 +- .../test/Graph/transformProps.test.ts | 132 +++++++++++++----- .../test/Tree/transformProps.test.ts | 128 +++++++++-------- 14 files changed, 297 insertions(+), 183 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/package.json b/superset-frontend/plugins/plugin-chart-echarts/package.json index b370863cb9bb2..02da13b0d12bc 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/package.json +++ b/superset-frontend/plugins/plugin-chart-echarts/package.json @@ -34,7 +34,6 @@ "peerDependencies": { "@superset-ui/chart-controls": "*", "@superset-ui/core": "*", - "zrender": "*", "react": "^16.13.1" } } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts index 343865483172b..e690b1ef52bb9 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts @@ -23,10 +23,13 @@ import { extractTimegrain, QueryFormData, } from '@superset-ui/core'; -import { BigNumberTotalChartProps } from '../types'; +import { BigNumberTotalChartProps, BigNumberVizProps } from '../types'; import { getDateFormatter, parseMetricValue } from '../utils'; +import { Refs } from '../../types'; -export default function transformProps(chartProps: BigNumberTotalChartProps) { +export default function transformProps( + chartProps: BigNumberTotalChartProps, +): BigNumberVizProps { const { width, height, queriesData, formData, rawFormData, hooks } = chartProps; const { @@ -38,6 +41,7 @@ export default function transformProps(chartProps: BigNumberTotalChartProps) { timeFormat, yAxisFormat, } = formData; + const refs: Refs = {}; const { data = [], coltypes = [] } = queriesData[0]; const granularity = extractTimegrain(rawFormData as QueryFormData); const metricName = getMetricLabel(metric); @@ -76,5 +80,6 @@ export default function transformProps(chartProps: BigNumberTotalChartProps) { subheaderFontSize, subheader: formattedSubheader, onContextMenu, + refs, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx index b7516561cd9b3..669926d58ba8e 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx @@ -20,17 +20,14 @@ import React, { MouseEvent } from 'react'; import { t, getNumberFormatter, - NumberFormatter, smartDateVerboseFormatter, - TimeFormatter, computeMaxFontSize, BRAND_COLOR, styled, BinaryQueryObjectFilterClause, } from '@superset-ui/core'; -import { EChartsCoreOption } from 'echarts'; import Echart from '../components/Echart'; -import { BigNumberWithTrendlineFormData, TimeSeriesDatum } from './types'; +import { BigNumberVizProps } from './types'; import { EventHandlers } from '../types'; const defaultNumberFormatter = getNumberFormatter(); @@ -44,36 +41,7 @@ const PROPORTION = { TRENDLINE: 0.3, }; -type BigNumberVisProps = { - className?: string; - width: number; - height: number; - bigNumber?: number | null; - bigNumberFallback?: TimeSeriesDatum; - headerFormatter: NumberFormatter | TimeFormatter; - formatTime: TimeFormatter; - headerFontSize: number; - kickerFontSize: number; - subheader: string; - subheaderFontSize: number; - showTimestamp?: boolean; - showTrendLine?: boolean; - startYAxisAtZero?: boolean; - timeRangeFixed?: boolean; - timestamp?: number; - trendLineData?: TimeSeriesDatum[]; - mainColor: string; - echartOptions: EChartsCoreOption; - onContextMenu?: ( - clientX: number, - clientY: number, - filters?: BinaryQueryObjectFilterClause[], - ) => void; - xValueFormatter?: TimeFormatter; - formData?: BigNumberWithTrendlineFormData; -}; - -class BigNumberVis extends React.PureComponent { +class BigNumberVis extends React.PureComponent { static defaultProps = { className: '', headerFormatter: defaultNumberFormatter, @@ -108,7 +76,7 @@ class BigNumberVis extends React.PureComponent { renderFallbackWarning() { const { bigNumberFallback, formatTime, showTimestamp } = this.props; - if (!bigNumberFallback || showTimestamp) return null; + if (!formatTime || !bigNumberFallback || showTimestamp) return null; return ( { renderKicker(maxHeight: number) { const { timestamp, showTimestamp, formatTime, width } = this.props; - if (!showTimestamp) return null; + if ( + !formatTime || + !showTimestamp || + typeof timestamp === 'string' || + typeof timestamp === 'boolean' + ) + return null; const text = timestamp === null ? '' : formatTime(timestamp); @@ -155,6 +129,7 @@ class BigNumberVis extends React.PureComponent { renderHeader(maxHeight: number) { const { bigNumber, headerFormatter, width } = this.props; + // @ts-ignore const text = bigNumber === null ? t('No data') : headerFormatter(bigNumber); const container = this.createTemporaryContainer(); @@ -231,7 +206,7 @@ class BigNumberVis extends React.PureComponent { } renderTrendline(maxHeight: number) { - const { width, trendLineData, echartOptions } = this.props; + const { width, trendLineData, echartOptions, refs } = this.props; // if can't find any non-null values, no point rendering the trendline if (!trendLineData?.some(d => d[1] !== null)) { @@ -264,12 +239,15 @@ class BigNumberVis extends React.PureComponent { }; return ( - + echartOptions && ( + + ) ); } @@ -292,7 +270,9 @@ class BigNumberVis extends React.PureComponent {
{this.renderFallbackWarning()} {this.renderKicker( - Math.ceil(kickerFontSize * (1 - PROPORTION.TRENDLINE) * height), + Math.ceil( + (kickerFontSize || 0) * (1 - PROPORTION.TRENDLINE) * height, + ), )} {this.renderHeader( Math.ceil(headerFontSize * (1 - PROPORTION.TRENDLINE) * height), @@ -311,7 +291,7 @@ class BigNumberVis extends React.PureComponent { return (
{this.renderFallbackWarning()} - {this.renderKicker(kickerFontSize * height)} + {this.renderKicker((kickerFontSize || 0) * height)} {this.renderHeader(Math.ceil(headerFontSize * height))} {this.renderSubheader(Math.ceil(subheaderFontSize * height))}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts index c70e005829d18..eedd19917022f 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts @@ -30,6 +30,7 @@ import { } from '@superset-ui/core'; import { EChartsCoreOption, graphic } from 'echarts'; import { + BigNumberVizProps, BigNumberDatum, BigNumberWithTrendlineChartProps, TimeSeriesDatum, @@ -60,7 +61,7 @@ const formatPercentChange = getNumberFormatter( export default function transformProps( chartProps: BigNumberWithTrendlineChartProps, -) { +): BigNumberVizProps { const { width, height, @@ -103,7 +104,7 @@ export default function transformProps( const mainColor = `rgb(${r}, ${g}, ${b})`; const xAxisLabel = getXAxisLabel(rawFormData) as string; - let trendLineData; + let trendLineData: TimeSeriesDatum[] | undefined; let percentChange = 0; let bigNumber = data.length === 0 ? null : data[0][metricName]; let timestamp = data.length === 0 ? null : data[0][xAxisLabel]; @@ -144,6 +145,7 @@ export default function transformProps( } } sortedData.reverse(); + // @ts-ignore trendLineData = showTrendLine ? sortedData : undefined; } @@ -249,6 +251,7 @@ export default function transformProps( width, height, bigNumber, + // @ts-ignore bigNumberFallback, className, headerFormatter, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts index 60c43770e37c4..90b852b01e4e8 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts @@ -17,12 +17,17 @@ * under the License. */ +import { EChartsCoreOption } from 'echarts'; import { + BinaryQueryObjectFilterClause, ChartDataResponseResult, - ChartProps, + DataRecordValue, + NumberFormatter, QueryFormData, QueryFormMetric, + TimeFormatter, } from '@superset-ui/core'; +import { BaseChartProps, Refs } from '../types'; export interface BigNumberDatum { [key: string]: number | null; @@ -43,15 +48,50 @@ export type BigNumberWithTrendlineFormData = BigNumberTotalFormData & { compareLag?: string | number; }; -export type BigNumberTotalChartProps = ChartProps & { - formData: BigNumberTotalFormData; - queriesData: (ChartDataResponseResult & { - data?: BigNumberDatum[]; - })[]; -}; +export interface BigNumberTotalChartDataResponseResult + extends ChartDataResponseResult { + data: BigNumberDatum[]; +} -export type BigNumberWithTrendlineChartProps = BigNumberTotalChartProps & { - formData: BigNumberWithTrendlineFormData; -}; +export type BigNumberTotalChartProps = + BaseChartProps & { + formData: BigNumberTotalFormData; + queriesData: BigNumberTotalChartDataResponseResult[]; + }; + +export type BigNumberWithTrendlineChartProps = + BaseChartProps & { + formData: BigNumberWithTrendlineFormData; + }; export type TimeSeriesDatum = [number, number | null]; + +export type BigNumberVizProps = { + className?: string; + width: number; + height: number; + bigNumber?: DataRecordValue; + bigNumberFallback?: TimeSeriesDatum; + headerFormatter: NumberFormatter | TimeFormatter; + formatTime?: TimeFormatter; + headerFontSize: number; + kickerFontSize?: number; + subheader: string; + subheaderFontSize: number; + showTimestamp?: boolean; + showTrendLine?: boolean; + startYAxisAtZero?: boolean; + timeRangeFixed?: boolean; + timestamp?: DataRecordValue; + trendLineData?: TimeSeriesDatum[]; + mainColor?: string; + echartOptions?: EChartsCoreOption; + onContextMenu?: ( + clientX: number, + clientY: number, + filters?: BinaryQueryObjectFilterClause[], + ) => void; + xValueFormatter?: TimeFormatter; + formData?: BigNumberWithTrendlineFormData; + refs: Refs; +}; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/constants.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/constants.ts index 463835966cc71..35567c3fc5939 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/constants.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/constants.ts @@ -17,6 +17,7 @@ * under the License. */ import { TreeSeriesOption } from 'echarts'; +import { EchartsTreeFormData } from './types'; export const DEFAULT_TREE_SERIES_OPTION: TreeSeriesOption = { label: { @@ -28,3 +29,18 @@ export const DEFAULT_TREE_SERIES_OPTION: TreeSeriesOption = { animationEasing: 'cubicOut', lineStyle: { color: 'source', width: 1.5 }, }; + +export const DEFAULT_FORM_DATA: Partial = { + id: '', + parent: '', + name: '', + rootNodeId: '', + layout: 'orthogonal', + orient: 'LR', + symbol: 'emptyCircle', + symbolSize: 7, + roam: true, + nodeLabelPosition: 'left', + childLabelPosition: 'bottom', + emphasis: 'descendant', +}; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/controlPanel.tsx index 79e0639277a40..f8035203fe6b3 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/controlPanel.tsx @@ -24,7 +24,7 @@ import { sections, sharedControls, } from '@superset-ui/chart-controls'; -import { DEFAULT_FORM_DATA } from './types'; +import { DEFAULT_FORM_DATA } from './constants'; const requiredEntity = { ...sharedControls.entity, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts index 4648b3c0d6a04..2e9037be80cef 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/transformProps.ts @@ -24,13 +24,12 @@ import { } from 'echarts/types/src/chart/tree/TreeSeries'; import { OptionName } from 'echarts/types/src/util/types'; import { - DEFAULT_FORM_DATA as DEFAULT_GRAPH_FORM_DATA, EchartsTreeChartProps, EchartsTreeFormData, TreeDataRecord, TreeTransformedProps, } from './types'; -import { DEFAULT_TREE_SERIES_OPTION } from './constants'; +import { DEFAULT_FORM_DATA, DEFAULT_TREE_SERIES_OPTION } from './constants'; import { Refs } from '../types'; export function formatTooltip({ @@ -72,7 +71,7 @@ export default function transformProps( nodeLabelPosition, childLabelPosition, emphasis, - }: EchartsTreeFormData = { ...DEFAULT_GRAPH_FORM_DATA, ...formData }; + }: EchartsTreeFormData = { ...DEFAULT_FORM_DATA, ...formData }; const metricLabel = getMetricLabel(metric); const nameColumn = name || id; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts index d965a602e4be2..0fde0cde2a177 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/types.ts @@ -16,11 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { ChartDataResponseResult } from '@superset-ui/core'; +import { OptionName } from 'echarts/types/src/util/types'; +import { ChartDataResponseResult, QueryFormData } from '@superset-ui/core'; import { TreeSeriesNodeItemOption } from 'echarts/types/src/chart/tree/TreeSeries'; import { BaseChartProps, BaseTransformedProps } from '../types'; -export type EchartsTreeFormData = { +export type EchartsTreeFormData = QueryFormData & { id: string; parent: string; name: string; @@ -37,22 +38,7 @@ export type EchartsTreeFormData = { emphasis: 'none' | 'ancestor' | 'descendant'; }; -export const DEFAULT_FORM_DATA: EchartsTreeFormData = { - id: '', - parent: '', - name: '', - rootNodeId: '', - layout: 'orthogonal', - orient: 'LR', - symbol: 'emptyCircle', - symbolSize: 7, - roam: true, - nodeLabelPosition: 'left', - childLabelPosition: 'bottom', - emphasis: 'descendant', -}; - -interface TreeChartDataResponseResult extends ChartDataResponseResult { +export interface TreeChartDataResponseResult extends ChartDataResponseResult { data: TreeDataRecord[]; } @@ -62,8 +48,8 @@ export interface EchartsTreeChartProps queriesData: TreeChartDataResponseResult[]; } -export type TreeDataRecord = Record & { - children: TreeSeriesNodeItemOption[]; +export type TreeDataRecord = Record & { + children?: TreeSeriesNodeItemOption[]; }; export type TreeTransformedProps = BaseTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts index d7c552edfcad2..0d26b92d08df4 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts @@ -17,7 +17,11 @@ * under the License. */ import { BinaryQueryObjectFilterClause } from '@superset-ui/core'; -import { EChartTransformedProps, EventHandlers } from '../types'; +import { + BaseTransformedProps, + CrossFilterTransformedProps, + EventHandlers, +} from '../types'; export type Event = { name: string; @@ -40,8 +44,9 @@ export const clickEventHandler = export const contextMenuEventHandler = ( - groupby: EChartTransformedProps['groupby'], - onContextMenu: EChartTransformedProps['onContextMenu'], + groupby: (BaseTransformedProps & + CrossFilterTransformedProps)['groupby'], + onContextMenu: BaseTransformedProps['onContextMenu'], labelMap: Record, ) => (e: Event) => { @@ -65,7 +70,7 @@ export const contextMenuEventHandler = }; export const allEventHandlers = ( - transformedProps: EChartTransformedProps, + transformedProps: BaseTransformedProps & CrossFilterTransformedProps, handleChange: (values: string[]) => void, ) => { const { groupby, selectedValues, onContextMenu, labelMap } = transformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts index 1c116cffd598d..d2f6890430c4a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/tooltip.ts @@ -17,7 +17,6 @@ * under the License. */ -import ZRText from 'zrender/src/graphic/Text'; import { CallbackDataParams } from 'echarts/types/src/util/types'; import { TOOLTIP_OVERFLOW_MARGIN, TOOLTIP_POINTER_MARGIN } from '../constants'; import { Refs } from '../types'; @@ -26,7 +25,7 @@ export function getDefaultPosition(refs: Refs) { return ( canvasMousePos: [number, number], params: CallbackDataParams, - tooltipDom: ZRText | HTMLDivElement | null, + tooltipDom: HTMLDivElement | null, rect: any, sizes: { contentSize: [number, number]; viewSize: [number, number] }, ) => { diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts index f138765987d5c..ce00bb71906f1 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/transformProps.test.ts @@ -25,6 +25,7 @@ import transformProps from '../../src/BigNumber/BigNumberWithTrendline/transform import { BigNumberDatum, BigNumberWithTrendlineChartProps, + BigNumberWithTrendlineFormData, } from '../../src/BigNumber/types'; const formData = { @@ -44,7 +45,8 @@ const formData = { datasource: 'test_datasource', }; -const rawFormData = { +const rawFormData: BigNumberWithTrendlineFormData = { + colorPicker: { b: 0, g: 0, r: 0 }, datasource: '1__table', metric: 'value', color_picker: { @@ -129,7 +131,8 @@ describe('BigNumberWithTrendline', () => { expect(transformed.bigNumber).toStrictEqual(1.2345); expect(transformed.bigNumberFallback).not.toBeNull(); - // should successfully formatTime by ganularity + // should successfully formatTime by granularity + // @ts-ignore expect(transformed.formatTime(new Date('2020-01-01'))).toStrictEqual( '2020-01-01 00:00:00', ); @@ -150,6 +153,7 @@ describe('BigNumberWithTrendline', () => { }, }; const transformed = transformProps(propsWithDatasource); + // @ts-ignore expect(transformed.headerFormatter(transformed.bigNumber)).toStrictEqual( '1.23', ); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts index 480f4828f3378..879c4ffc4bd91 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts @@ -16,24 +16,98 @@ * specific language governing permissions and limitations * under the License. */ -import { ChartProps, supersetTheme } from '@superset-ui/core'; +import { + ChartProps, + ChartDataResponseResult, + DatasourceType, + GenericDataType, + supersetTheme, +} from '@superset-ui/core'; +import { LegendOrientation, LegendType } from '../../src/types'; import transformProps from '../../src/Graph/transformProps'; import { DEFAULT_GRAPH_SERIES_OPTION } from '../../src/Graph/constants'; +import { + EchartsGraphChartProps, + EchartsGraphFormData, +} from '../../src/Graph/types'; describe('EchartsGraph transformProps', () => { + const baseFormData: EchartsGraphFormData = { + baseEdgeWidth: 0, + baseNodeSize: 0, + draggable: false, + edgeLength: 0, + edgeSymbol: '', + friction: 0, + gravity: 0, + legendMargin: 0, + legendOrientation: LegendOrientation.Left, + legendType: LegendType.Scroll, + repulsion: 0, + roam: 'scale', + showLegend: false, + showSymbolThreshold: 0, + viz_type: 'echarts_graph', + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'count', + source: 'source_column', + target: 'target_column', + category: null, + }; + const baseResponseResult: ChartDataResponseResult = { + error: null, + is_cached: false, + query: '', + rowcount: 2, + stacktrace: null, + status: 'success', + from_dttm: null, + to_dttm: null, + coltypes: [ + GenericDataType.STRING, + GenericDataType.STRING, + GenericDataType.NUMERIC, + ], + cached_dttm: null, + annotation_data: {}, + cache_key: '', + cache_timeout: 0, + colnames: ['source_column', 'target_column', 'count'], + data: [], + }; + const baseChartProps: EchartsGraphChartProps = { + annotationData: {}, + appSection: undefined, + behaviors: [], + datasource: { + id: 1, + name: 'foo', + type: DatasourceType.Table, + columns: [], + metrics: [], + }, + filterState: {}, + hooks: {}, + inContextMenu: false, + initialValues: {}, + inputRef: undefined, + isRefreshing: false, + ownState: {}, + rawDatasource: {}, + rawFormData: baseFormData, + formData: baseFormData, + width: 800, + height: 600, + queriesData: [baseResponseResult], + theme: supersetTheme, + }; + it('should transform chart props for viz without category', () => { - const formData = { - colorScheme: 'bnbColors', - datasource: '3__table', - granularity_sqla: 'ds', - metric: 'count', - source: 'source_column', - target: 'target_column', - category: null, - }; - const queriesData = [ + const queriesData: ChartDataResponseResult[] = [ { - colnames: ['source_column', 'target_column', 'count'], + ...baseResponseResult, data: [ { source_column: 'source_value_1', @@ -48,16 +122,11 @@ describe('EchartsGraph transformProps', () => { ], }, ]; - const chartPropsConfig = { - formData, - width: 800, - height: 600, + const chartProps = new ChartProps({ + ...baseChartProps, queriesData, - theme: supersetTheme, - }; - - const chartProps = new ChartProps(chartPropsConfig); - expect(transformProps(chartProps)).toEqual( + }); + expect(transformProps(chartProps as EchartsGraphChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, @@ -151,18 +220,14 @@ describe('EchartsGraph transformProps', () => { }); it('should transform chart props for viz with category and falsey normalization', () => { - const formData = { - colorScheme: 'bnbColors', - datasource: '3__table', - granularity_sqla: 'ds', - metric: 'count', - source: 'source_column', - target: 'target_column', + const formData: EchartsGraphFormData = { + ...baseFormData, sourceCategory: 'source_category_column', targetCategory: 'target_category_column', }; const queriesData = [ { + ...baseResponseResult, colnames: [ 'source_column', 'target_column', @@ -188,16 +253,13 @@ describe('EchartsGraph transformProps', () => { ], }, ]; - const chartPropsConfig = { + const chartProps = new ChartProps({ + ...baseChartProps, formData, - width: 800, - height: 600, queriesData, - theme: supersetTheme, - }; + }); - const chartProps = new ChartProps(chartPropsConfig); - expect(transformProps(chartProps)).toEqual( + expect(transformProps(chartProps as EchartsGraphChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts index ad06455cb11f7..579de3bd16866 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts @@ -16,11 +16,24 @@ * specific language governing permissions and limitations * under the License. */ -import { ChartProps, supersetTheme } from '@superset-ui/core'; +import { DatasourceType, supersetTheme } from '@superset-ui/core'; import transformProps from '../../src/Tree/transformProps'; +import { + EchartsTreeFormData, + EchartsTreeChartProps, + TreeChartDataResponseResult, +} from '../../src/Tree/types'; describe('EchartsTree transformProps', () => { - const formData = { + const baseFormData: EchartsTreeFormData = { + childLabelPosition: 'top', + emphasis: 'ancestor', + layout: 'radial', + nodeLabelPosition: 'top', + orient: 'LR', + roam: 'scale', + symbol: '', + symbolSize: 0, colorScheme: 'bnbColors', datasource: '3__table', granularity_sqla: 'ds', @@ -29,20 +42,38 @@ describe('EchartsTree transformProps', () => { parent: 'relation_column', name: 'name_column', rootNodeId: '1', + viz_type: 'tree', }; - const chartPropsConfig = { - formData, - width: 800, + const baseChartProps: EchartsTreeChartProps = { + annotationData: {}, + behaviors: [], + datasource: { + id: 1, + name: 'foo', + type: DatasourceType.Table, + columns: [], + metrics: [], + }, + filterState: {}, + formData: baseFormData, height: 600, + hooks: {}, + initialValues: {}, + ownState: {}, + rawDatasource: {}, + rawFormData: baseFormData, theme: supersetTheme, + width: 800, + queriesData: [], }; it('should transform when parent present before child', () => { - const queriesData = [ + const queriesData: TreeChartDataResponseResult[] = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ { id_column: '1', + // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -69,7 +100,10 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + const chartProps: EchartsTreeChartProps = { + ...baseChartProps, + queriesData, + }; expect(transformProps(chartProps)).toEqual( expect.objectContaining({ width: 800, @@ -104,12 +138,13 @@ describe('EchartsTree transformProps', () => { ); }); it('should transform when child is present before parent', () => { - const queriesData = [ + const queriesData: TreeChartDataResponseResult[] = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ { id_column: '1', + // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -136,7 +171,10 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + const chartProps: EchartsTreeChartProps = { + ...baseChartProps, + queriesData, + }; expect(transformProps(chartProps)).toEqual( expect.objectContaining({ width: 800, @@ -175,27 +213,16 @@ describe('EchartsTree transformProps', () => { }); it('ignore node if not attached to root', () => { const formData = { - colorScheme: 'bnbColors', - datasource: '3__table', - granularity_sqla: 'ds', - metric: 'count', - id: 'id_column', - parent: 'relation_column', - name: 'name_column', + ...baseFormData, rootNodeId: '2', }; - const chartPropsConfig = { - formData, - width: 800, - height: 600, - theme: supersetTheme, - }; - const queriesData = [ + const queriesData: TreeChartDataResponseResult[] = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ { id_column: '1', + // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -222,7 +249,12 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + const chartProps: EchartsTreeChartProps = { + ...baseChartProps, + formData, + rawFormData: formData, + queriesData, + }; expect(transformProps(chartProps)).toEqual( expect.objectContaining({ width: 800, @@ -255,27 +287,17 @@ describe('EchartsTree transformProps', () => { ); }); it('should transform props if name column is not specified', () => { - const formData = { - colorScheme: 'bnbColors', - datasource: '3__table', - granularity_sqla: 'ds', - metric: 'count', - id: 'id_column', - parent: 'relation_column', + const formData: EchartsTreeFormData = { + ...baseFormData, rootNodeId: '1', }; - const chartPropsConfig = { - formData, - width: 800, - height: 600, - theme: supersetTheme, - }; - const queriesData = [ + const queriesData: TreeChartDataResponseResult[] = [ { colnames: ['id_column', 'relation_column', 'count'], data: [ { id_column: '1', + // @ts-ignore relation_column: null, count: 10, }, @@ -298,7 +320,12 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + const chartProps: EchartsTreeChartProps = { + ...baseChartProps, + formData, + rawFormData: formData, + queriesData, + }; expect(transformProps(chartProps)).toEqual( expect.objectContaining({ width: 800, @@ -337,22 +364,7 @@ describe('EchartsTree transformProps', () => { ); }); it('should find root node with null parent when root node name is not provided', () => { - const formData = { - colorScheme: 'bnbColors', - datasource: '3__table', - granularity_sqla: 'ds', - metric: 'count', - id: 'id_column', - parent: 'relation_column', - name: 'name_column', - }; - const chartPropsConfig = { - formData, - width: 800, - height: 600, - theme: supersetTheme, - }; - const queriesData = [ + const queriesData: TreeChartDataResponseResult[] = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ @@ -376,6 +388,7 @@ describe('EchartsTree transformProps', () => { }, { id_column: '1', + // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -384,7 +397,10 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + const chartProps: EchartsTreeChartProps = { + ...baseChartProps, + queriesData, + }; expect(transformProps(chartProps)).toEqual( expect.objectContaining({ width: 800, From 26ee293135c30ab7b84799fac7c4e73db9916541 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 23 Nov 2022 13:04:09 +0200 Subject: [PATCH 6/8] more fixing --- .../test/Graph/transformProps.test.ts | 131 +++++------------ .../test/Tree/transformProps.test.ts | 139 ++++++++---------- 2 files changed, 98 insertions(+), 172 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts index 879c4ffc4bd91..2401206c283aa 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts @@ -16,98 +16,26 @@ * specific language governing permissions and limitations * under the License. */ -import { - ChartProps, - ChartDataResponseResult, - DatasourceType, - GenericDataType, - supersetTheme, -} from '@superset-ui/core'; -import { LegendOrientation, LegendType } from '../../src/types'; +import { ChartProps, SqlaFormData, supersetTheme } from '@superset-ui/core'; import transformProps from '../../src/Graph/transformProps'; import { DEFAULT_GRAPH_SERIES_OPTION } from '../../src/Graph/constants'; -import { - EchartsGraphChartProps, - EchartsGraphFormData, -} from '../../src/Graph/types'; +import { EchartsGraphChartProps } from '../../src/Graph/types'; describe('EchartsGraph transformProps', () => { - const baseFormData: EchartsGraphFormData = { - baseEdgeWidth: 0, - baseNodeSize: 0, - draggable: false, - edgeLength: 0, - edgeSymbol: '', - friction: 0, - gravity: 0, - legendMargin: 0, - legendOrientation: LegendOrientation.Left, - legendType: LegendType.Scroll, - repulsion: 0, - roam: 'scale', - showLegend: false, - showSymbolThreshold: 0, - viz_type: 'echarts_graph', - colorScheme: 'bnbColors', - datasource: '3__table', - granularity_sqla: 'ds', - metric: 'count', - source: 'source_column', - target: 'target_column', - category: null, - }; - const baseResponseResult: ChartDataResponseResult = { - error: null, - is_cached: false, - query: '', - rowcount: 2, - stacktrace: null, - status: 'success', - from_dttm: null, - to_dttm: null, - coltypes: [ - GenericDataType.STRING, - GenericDataType.STRING, - GenericDataType.NUMERIC, - ], - cached_dttm: null, - annotation_data: {}, - cache_key: '', - cache_timeout: 0, - colnames: ['source_column', 'target_column', 'count'], - data: [], - }; - const baseChartProps: EchartsGraphChartProps = { - annotationData: {}, - appSection: undefined, - behaviors: [], - datasource: { - id: 1, - name: 'foo', - type: DatasourceType.Table, - columns: [], - metrics: [], - }, - filterState: {}, - hooks: {}, - inContextMenu: false, - initialValues: {}, - inputRef: undefined, - isRefreshing: false, - ownState: {}, - rawDatasource: {}, - rawFormData: baseFormData, - formData: baseFormData, - width: 800, - height: 600, - queriesData: [baseResponseResult], - theme: supersetTheme, - }; - it('should transform chart props for viz without category', () => { - const queriesData: ChartDataResponseResult[] = [ + const formData: SqlaFormData = { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'count', + source: 'source_column', + target: 'target_column', + category: null, + viz_type: 'graph', + }; + const queriesData = [ { - ...baseResponseResult, + colnames: ['source_column', 'target_column', 'count'], data: [ { source_column: 'source_value_1', @@ -122,10 +50,15 @@ describe('EchartsGraph transformProps', () => { ], }, ]; - const chartProps = new ChartProps({ - ...baseChartProps, + const chartPropsConfig = { + formData, + width: 800, + height: 600, queriesData, - }); + theme: supersetTheme, + }; + + const chartProps = new ChartProps(chartPropsConfig); expect(transformProps(chartProps as EchartsGraphChartProps)).toEqual( expect.objectContaining({ width: 800, @@ -220,14 +153,19 @@ describe('EchartsGraph transformProps', () => { }); it('should transform chart props for viz with category and falsey normalization', () => { - const formData: EchartsGraphFormData = { - ...baseFormData, + const formData: SqlaFormData = { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'count', + source: 'source_column', + target: 'target_column', sourceCategory: 'source_category_column', targetCategory: 'target_category_column', + viz_type: 'graph', }; const queriesData = [ { - ...baseResponseResult, colnames: [ 'source_column', 'target_column', @@ -253,12 +191,15 @@ describe('EchartsGraph transformProps', () => { ], }, ]; - const chartProps = new ChartProps({ - ...baseChartProps, + const chartPropsConfig = { formData, + width: 800, + height: 600, queriesData, - }); + theme: supersetTheme, + }; + const chartProps = new ChartProps(chartPropsConfig); expect(transformProps(chartProps as EchartsGraphChartProps)).toEqual( expect.objectContaining({ width: 800, diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts index 579de3bd16866..6a56c997c22b6 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Tree/transformProps.test.ts @@ -16,24 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { DatasourceType, supersetTheme } from '@superset-ui/core'; +import { ChartProps, supersetTheme } from '@superset-ui/core'; import transformProps from '../../src/Tree/transformProps'; -import { - EchartsTreeFormData, - EchartsTreeChartProps, - TreeChartDataResponseResult, -} from '../../src/Tree/types'; +import { EchartsTreeChartProps } from '../../src/Tree/types'; describe('EchartsTree transformProps', () => { - const baseFormData: EchartsTreeFormData = { - childLabelPosition: 'top', - emphasis: 'ancestor', - layout: 'radial', - nodeLabelPosition: 'top', - orient: 'LR', - roam: 'scale', - symbol: '', - symbolSize: 0, + const formData = { colorScheme: 'bnbColors', datasource: '3__table', granularity_sqla: 'ds', @@ -42,38 +30,20 @@ describe('EchartsTree transformProps', () => { parent: 'relation_column', name: 'name_column', rootNodeId: '1', - viz_type: 'tree', }; - const baseChartProps: EchartsTreeChartProps = { - annotationData: {}, - behaviors: [], - datasource: { - id: 1, - name: 'foo', - type: DatasourceType.Table, - columns: [], - metrics: [], - }, - filterState: {}, - formData: baseFormData, + const chartPropsConfig = { + formData, + width: 800, height: 600, - hooks: {}, - initialValues: {}, - ownState: {}, - rawDatasource: {}, - rawFormData: baseFormData, theme: supersetTheme, - width: 800, - queriesData: [], }; it('should transform when parent present before child', () => { - const queriesData: TreeChartDataResponseResult[] = [ + const queriesData = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ { id_column: '1', - // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -100,11 +70,8 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps: EchartsTreeChartProps = { - ...baseChartProps, - queriesData, - }; - expect(transformProps(chartProps)).toEqual( + const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + expect(transformProps(chartProps as EchartsTreeChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, @@ -138,13 +105,12 @@ describe('EchartsTree transformProps', () => { ); }); it('should transform when child is present before parent', () => { - const queriesData: TreeChartDataResponseResult[] = [ + const queriesData = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ { id_column: '1', - // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -171,11 +137,8 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps: EchartsTreeChartProps = { - ...baseChartProps, - queriesData, - }; - expect(transformProps(chartProps)).toEqual( + const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + expect(transformProps(chartProps as EchartsTreeChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, @@ -213,16 +176,27 @@ describe('EchartsTree transformProps', () => { }); it('ignore node if not attached to root', () => { const formData = { - ...baseFormData, + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'count', + id: 'id_column', + parent: 'relation_column', + name: 'name_column', rootNodeId: '2', }; - const queriesData: TreeChartDataResponseResult[] = [ + const chartPropsConfig = { + formData, + width: 800, + height: 600, + theme: supersetTheme, + }; + const queriesData = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ { id_column: '1', - // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -249,13 +223,8 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps: EchartsTreeChartProps = { - ...baseChartProps, - formData, - rawFormData: formData, - queriesData, - }; - expect(transformProps(chartProps)).toEqual( + const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + expect(transformProps(chartProps as EchartsTreeChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, @@ -287,17 +256,27 @@ describe('EchartsTree transformProps', () => { ); }); it('should transform props if name column is not specified', () => { - const formData: EchartsTreeFormData = { - ...baseFormData, + const formData = { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'count', + id: 'id_column', + parent: 'relation_column', rootNodeId: '1', }; - const queriesData: TreeChartDataResponseResult[] = [ + const chartPropsConfig = { + formData, + width: 800, + height: 600, + theme: supersetTheme, + }; + const queriesData = [ { colnames: ['id_column', 'relation_column', 'count'], data: [ { id_column: '1', - // @ts-ignore relation_column: null, count: 10, }, @@ -320,13 +299,8 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps: EchartsTreeChartProps = { - ...baseChartProps, - formData, - rawFormData: formData, - queriesData, - }; - expect(transformProps(chartProps)).toEqual( + const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + expect(transformProps(chartProps as EchartsTreeChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, @@ -364,7 +338,22 @@ describe('EchartsTree transformProps', () => { ); }); it('should find root node with null parent when root node name is not provided', () => { - const queriesData: TreeChartDataResponseResult[] = [ + const formData = { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'count', + id: 'id_column', + parent: 'relation_column', + name: 'name_column', + }; + const chartPropsConfig = { + formData, + width: 800, + height: 600, + theme: supersetTheme, + }; + const queriesData = [ { colnames: ['id_column', 'relation_column', 'name_column', 'count'], data: [ @@ -388,7 +377,6 @@ describe('EchartsTree transformProps', () => { }, { id_column: '1', - // @ts-ignore relation_column: null, name_column: 'root', count: 10, @@ -397,11 +385,8 @@ describe('EchartsTree transformProps', () => { }, ]; - const chartProps: EchartsTreeChartProps = { - ...baseChartProps, - queriesData, - }; - expect(transformProps(chartProps)).toEqual( + const chartProps = new ChartProps({ ...chartPropsConfig, queriesData }); + expect(transformProps(chartProps as EchartsTreeChartProps)).toEqual( expect.objectContaining({ width: 800, height: 600, From 58ccf03990e1260328a2c25aee30613e606d06f5 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 23 Nov 2022 14:57:14 +0200 Subject: [PATCH 7/8] add refs to BigNumberWithTrendline --- .../src/BigNumber/BigNumberWithTrendline/transformProps.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts index eedd19917022f..e668a75fbeb58 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.ts @@ -36,6 +36,8 @@ import { TimeSeriesDatum, } from '../types'; import { getDateFormatter, parseMetricValue } from '../utils'; +import { getDefaultPosition } from '../../utils/tooltip'; +import { Refs } from '../../types'; const defaultNumberFormatter = getNumberFormatter(); export function renderTooltipFactory( @@ -96,6 +98,7 @@ export default function transformProps( from_dttm: fromDatetime, to_dttm: toDatetime, } = queriesData[0]; + const refs: Refs = {}; const metricName = getMetricLabel(metric); const compareLag = Number(compareLag_) || 0; let formattedSubheader = subheader; @@ -231,6 +234,7 @@ export default function transformProps( bottom: 0, }, tooltip: { + position: getDefaultPosition(refs), appendToBody: true, show: !inContextMenu, trigger: 'axis', @@ -269,5 +273,6 @@ export default function transformProps( echartOptions, onContextMenu, xValueFormatter: formatTime, + refs, }; } From 7ad716d7756f767e84a2b84724d2aa65f51f03a8 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 23 Nov 2022 15:53:23 +0200 Subject: [PATCH 8/8] add cleanup --- .../plugins/plugin-chart-echarts/src/components/Echart.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx index aaae39b8bde3e..011d62abacec4 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx @@ -111,6 +111,7 @@ function Echart( // did mount useEffect(() => { handleSizeChange({ width, height }); + return () => chartRef.current?.dispose(); }, []); useLayoutEffect(() => {