diff --git a/docs/developer_docs/extensions/components/alert.mdx b/docs/developer_docs/extensions/components/alert.mdx index 296a6f120f14..5a48438b61ac 100644 --- a/docs/developer_docs/extensions/components/alert.mdx +++ b/docs/developer_docs/extensions/components/alert.mdx @@ -114,8 +114,8 @@ function MyExtension() { ## Source Links -- [Story file](https://github.com/apache/superset/blob/master/superset-frontend/packages/superset-core/src/ui/components/Alert/Alert.stories.tsx) -- [Component source](https://github.com/apache/superset/blob/master/superset-frontend/packages/superset-core/src/ui/components/Alert/index.tsx) +- [Story file](https://github.com/apache/superset/blob/master/superset-frontend/packages/superset-core/src/components/Alert/Alert.stories.tsx) +- [Component source](https://github.com/apache/superset/blob/master/superset-frontend/packages/superset-core/src/components/Alert/index.tsx) --- diff --git a/docs/developer_docs/extensions/components/index.mdx b/docs/developer_docs/extensions/components/index.mdx index 715e8dd4fbbe..78c076cc1963 100644 --- a/docs/developer_docs/extensions/components/index.mdx +++ b/docs/developer_docs/extensions/components/index.mdx @@ -47,8 +47,8 @@ export function MyExtensionPanel() { Components in `@apache-superset/core/components` are automatically documented here. To add a new extension component: -1. Add the component to `superset-frontend/packages/superset-core/src/ui/components/` -2. Export it from `superset-frontend/packages/superset-core/src/ui/components/index.ts` +1. Add the component to `superset-frontend/packages/superset-core/src/components/` +2. Export it from `superset-frontend/packages/superset-core/src/components/index.ts` 3. Create a Storybook story with an `Interactive` export: ```tsx diff --git a/superset-frontend/packages/superset-ui-core/src/query/extractQueryFields.ts b/superset-frontend/packages/superset-ui-core/src/query/extractQueryFields.ts index 24c3303b2fd2..1eb0b15b93c9 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/extractQueryFields.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/extractQueryFields.ts @@ -47,6 +47,8 @@ export default function extractQueryFields( metric: 'metrics', metric_2: 'metrics', secondary_metric: 'metrics', + left_metric: 'metrics', + right_metric: 'metrics', x: 'metrics', y: 'metrics', size: 'metrics', diff --git a/superset-frontend/packages/superset-ui-core/test/query/extractQueryFields.test.ts b/superset-frontend/packages/superset-ui-core/test/query/extractQueryFields.test.ts index a55fb9e54086..8db7c63bb559 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/extractQueryFields.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/extractQueryFields.test.ts @@ -59,6 +59,16 @@ describe('extractQueryFields', () => { ).toEqual(['metric_1', 'metric_2', 'my_custom_metric']); }); + test('should extract butterfly chart metrics', () => { + expect( + extractQueryFields({ + groupby: ['category'], + left_metric: 'left_sum', + right_metric: 'right_sum', + }).metrics, + ).toEqual(['left_sum', 'right_sum']); + }); + test('should extract columns', () => { expect(extractQueryFields({ columns: 'col_1' })).toEqual({ columns: ['col_1'], diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/Butterfly.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/Butterfly.tsx index 0b523313c906..04395bb5afed 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/Butterfly.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/Butterfly.tsx @@ -16,15 +16,44 @@ * specific language governing permissions and limitations * under the License. */ +import { allEventHandlers, type Event } from '../utils/eventHandlers'; import Echart from '../components/Echart'; -import { ButterflyTransformedProps } from './types'; import { EventHandlers } from '../types'; +import { ButterflyTransformedProps } from './types'; + +type ButterflyChartEvent = { + name?: string; + data?: { name?: string }; + event?: Event['event']; +}; + +function getCategoryKey(params: ButterflyChartEvent): string { + return params.data?.name ?? params.name ?? ''; +} export default function Butterfly(props: ButterflyTransformedProps) { - const { height, width, echartOptions, refs, onLegendStateChanged, formData } = - props; + const { + height, + width, + echartOptions, + selectedValues, + refs, + onLegendStateChanged, + formData, + } = props; + + const { click, contextmenu } = allEventHandlers(props); const eventHandlers: EventHandlers = { + click: (params: ButterflyChartEvent) => { + click({ name: getCategoryKey(params) }); + }, + contextmenu: (params: ButterflyChartEvent) => { + contextmenu({ + ...params, + name: getCategoryKey(params), + }); + }, legendselectchanged: payload => { onLegendStateChanged?.(payload.selected); }, @@ -43,6 +72,7 @@ export default function Butterfly(props: ButterflyTransformedProps) { width={width} echartOptions={echartOptions} eventHandlers={eventHandlers} + selectedValues={selectedValues} vizType={formData.vizType} /> ); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/example-dark.png b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/example-dark.png new file mode 100644 index 000000000000..7effea144dac Binary files /dev/null and b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/example-dark.png differ diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/example.png b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/example.png new file mode 100644 index 000000000000..6a6e8a14f13e Binary files /dev/null and b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/example.png differ diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/thumbnail-dark.png b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/thumbnail-dark.png new file mode 100644 index 000000000000..29261f4d623f Binary files /dev/null and b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/thumbnail-dark.png differ diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/thumbnail.png b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/thumbnail.png new file mode 100644 index 000000000000..a98220c6040a Binary files /dev/null and b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/images/thumbnail.png differ diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/index.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/index.ts index db4a35c984dc..cc94ec201e0c 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/index.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/index.ts @@ -17,11 +17,15 @@ * under the License. */ import { t } from '@apache-superset/core/translation'; -import { ChartMetadata, ChartPlugin } from '@superset-ui/core'; +import { Behavior, ChartMetadata, ChartPlugin } from '@superset-ui/core'; import buildQuery from './buildQuery'; import controlPanel from './controlPanel'; import transformProps from './transformProps'; import { EchartsButterflyChartProps, EchartsButterflyFormData } from './types'; +import example from './images/example.png'; +import exampleDark from './images/example-dark.png'; +import thumbnail from './images/thumbnail.png'; +import thumbnailDark from './images/thumbnail-dark.png'; export default class EchartsButterflyChartPlugin extends ChartPlugin< EchartsButterflyFormData, @@ -33,12 +37,18 @@ export default class EchartsButterflyChartPlugin extends ChartPlugin< controlPanel, loadChart: () => import('./Butterfly'), metadata: new ChartMetadata({ + behaviors: [ + Behavior.InteractiveChart, + Behavior.DrillToDetail, + Behavior.DrillBy, + ], credits: ['https://echarts.apache.org'], category: t('Comparison'), description: t( 'A butterfly chart compares two metrics across categories using horizontal bars ' + 'that extend left and right from a central axis.', ), + exampleGallery: [{ url: example, urlDark: exampleDark }], name: t('Butterfly Chart'), tags: [ t('Categorical'), @@ -46,7 +56,8 @@ export default class EchartsButterflyChartPlugin extends ChartPlugin< t('ECharts'), t('Multi-Variables'), ], - thumbnail: '', + thumbnail, + thumbnailDark, }), transformProps, }); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/transformProps.ts index a6e20fa23eb4..6dc025b84506 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/transformProps.ts @@ -34,8 +34,13 @@ import { DEFAULT_FORM_DATA } from './constants'; import { defaultGrid } from '../defaults'; import { getDefaultTooltip } from '../utils/tooltip'; import { Refs } from '../types'; -import { NULL_STRING } from '../constants'; -import { getChartPadding, getLegendProps } from '../utils/series'; +import { OpacityEnum } from '../constants'; +import { + getChartPadding, + getLegendProps, + getColtypesMapping, + extractGroupbyLabel, +} from '../utils/series'; import { resolveLegendLayout } from '../utils/legendLayout'; import { convertInteger } from '../utils/convertInteger'; @@ -44,19 +49,11 @@ type EChartsOption = ComposeOption; const LABEL_LEFT = { position: 'left' as const }; const LABEL_RIGHT = { position: 'right' as const }; -function formatCategory(value: unknown): string { - if (value == null) { - return NULL_STRING; - } - if (typeof value === 'string' || typeof value === 'number') { - return String(value); - } - return String(value); -} - function formatTooltip( params: CallbackDataParams[], formatter: NumberFormatter | CurrencyFormatter, + categoryLabels: string[], + categoryByKey: Map, ) { const axisParams = params.filter( param => param.seriesName && typeof param.value === 'number', @@ -65,7 +62,13 @@ function formatTooltip( return ''; } - const title = axisParams[0].name; + const { dataIndex, name } = axisParams[0]; + const title = + (typeof dataIndex === 'number' + ? categoryLabels.at(dataIndex) + : undefined) ?? + (typeof name === 'string' ? categoryByKey.get(name) : undefined) ?? + name; const rows = axisParams.map(param => [ param.seriesName!, formatter(Math.abs(param.value as number)), @@ -86,6 +89,8 @@ export default function transformProps( hooks, theme, inContextMenu, + filterState, + emitCrossFilters, } = chartProps; const refs: Refs = {}; const { data = [] } = queriesData[0]; @@ -117,32 +122,75 @@ export default function transformProps( ...formData, }; - const groupbyColumn = ensureIsArray(groupby)[0]; - const categoryLabel = getColumnLabel(groupbyColumn); const leftMetricLabel = leftMetric ? getMetricLabel(leftMetric) : ''; const rightMetricLabel = rightMetric ? getMetricLabel(rightMetric) : ''; const leftSeriesName = leftLabel || leftMetricLabel; const rightSeriesName = rightLabel || rightMetricLabel; + const coltypeMapping = getColtypesMapping(queriesData[0]); + const groupbyColumns = ensureIsArray(groupby); + const groupbyLabels = groupbyColumns.map(getColumnLabel); + const defaultFormatter = currencyFormat?.symbol ? new CurrencyFormatter({ d3Format: xAxisFormat, currency: currencyFormat }) : getNumberFormatter(xAxisFormat); - const categories = data.map(row => formatCategory(row[categoryLabel])); - const leftData = data.map(row => { - const value = Number(row[leftMetricLabel] ?? 0); - return { - value: -Math.abs(value), - label: LABEL_LEFT, - }; - }); - const rightData = data.map(row => { - const value = Number(row[rightMetricLabel] ?? 0); - return { - value: Math.abs(value), - label: LABEL_RIGHT, - }; + const categories = data.map(datum => + extractGroupbyLabel({ datum, groupby: groupbyLabels, coltypeMapping }), + ); + const categoryKeys = data.map((datum, index) => { + const label = categories.at(index) ?? ''; + return `${label}__${JSON.stringify( + groupbyLabels.map(col => + Object.hasOwn(datum, col) ? datum[col] : undefined, + ), + )}`; }); + const categoryByKey = new Map( + categoryKeys.flatMap((key, index) => { + const label = categories.at(index); + return label === undefined ? [] : [[key, label] as const]; + }), + ); + + const labelMap = data.reduce>( + (acc, datum, index) => { + const uniqueKey = categoryKeys.at(index); + if (uniqueKey === undefined) { + return acc; + } + acc[uniqueKey] = groupbyLabels.map(col => + Object.hasOwn(datum, col) ? (datum[col] as string) : '', + ); + return acc; + }, + {}, + ); + const selectedValues = (filterState.selectedValues || []).reduce( + (acc: Record, value: string) => { + const index = categoryKeys.indexOf(value); + return index >= 0 ? { ...acc, [index]: value } : acc; + }, + {}, + ); + const getOpacity = (categoryKey: string) => + filterState.selectedValues?.length && + !filterState.selectedValues.includes(categoryKey) + ? OpacityEnum.SemiTransparent + : OpacityEnum.NonTransparent; + + const leftData = data.map((row, i) => ({ + name: categoryKeys[i], + value: -Math.abs(Number(row[leftMetricLabel] ?? 0)), + label: LABEL_LEFT, + itemStyle: { opacity: getOpacity(categoryKeys[i]) }, + })); + const rightData = data.map((row, i) => ({ + name: categoryKeys[i], + value: Math.abs(Number(row[rightMetricLabel] ?? 0)), + label: LABEL_RIGHT, + itemStyle: { opacity: getOpacity(categoryKeys[i]) }, + })); const labelFormatter = (params: CallbackDataParams) => { const value = Math.abs(params.value as number); @@ -280,6 +328,8 @@ export default function transformProps( formatTooltip( ensureIsArray(params) as CallbackDataParams[], defaultFormatter, + categories, + categoryByKey, ), }, series, @@ -294,5 +344,10 @@ export default function transformProps( setDataMask, onContextMenu, onLegendStateChanged, + groupby: groupbyColumns, + labelMap, + selectedValues, + emitCrossFilters, + coltypeMapping, }; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/types.ts index ac5daebd3942..060a042658af 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Butterfly/types.ts @@ -24,7 +24,12 @@ import { QueryFormMetric, RgbaColor, } from '@superset-ui/core'; -import { BaseTransformedProps, LegendFormData, TitleFormData } from '../types'; +import { + BaseTransformedProps, + LegendFormData, + TitleFormData, + CrossFilterTransformedProps, +} from '../types'; export type EchartsButterflyFormData = QueryFormData & LegendFormData & @@ -49,4 +54,4 @@ export interface EchartsButterflyChartProps extends ChartProps { } export type ButterflyTransformedProps = - BaseTransformedProps; + BaseTransformedProps & CrossFilterTransformedProps; diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/Butterfly.test.tsx b/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/Butterfly.test.tsx new file mode 100644 index 000000000000..7b27d40b711b --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/Butterfly.test.tsx @@ -0,0 +1,202 @@ +/** + * 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 { render } from '@testing-library/react'; +import { ChartProps } from '@superset-ui/core'; +import { supersetTheme } from '@apache-superset/core/theme'; +import Butterfly from '../../src/Butterfly/Butterfly'; +import transformProps from '../../src/Butterfly/transformProps'; +import { EchartsButterflyChartProps } from '../../src/Butterfly/types'; +import Echart from '../../src/components/Echart'; +import { EventHandlers } from '../../src/types'; + +jest.mock('../../src/components/Echart', () => ({ + __esModule: true, + default: jest.fn(() => null), +})); + +const mockedEchart = jest.mocked(Echart); + +const data = [ + { category: 'A', left_sum: 10, right_sum: 25 }, + { category: 'B', left_sum: 5, right_sum: 19 }, +]; + +const categoryKeyA = 'A__["A"]'; +const categoryKeyB = 'B__["B"]'; + +function setup( + overrides: { + filterState?: { selectedValues?: string[] }; + onLegendStateChanged?: jest.Mock; + } = {}, +) { + const onContextMenu = jest.fn(); + const setDataMask = jest.fn(); + const onLegendStateChanged = overrides.onLegendStateChanged ?? jest.fn(); + const chartProps = { + ...new ChartProps({ + formData: { + groupby: ['category'], + left_metric: 'left_sum', + right_metric: 'right_sum', + viz_type: 'butterfly', + }, + width: 800, + height: 600, + queriesData: [{ data }], + theme: supersetTheme, + hooks: { onContextMenu, setDataMask, onLegendStateChanged }, + }), + filterState: overrides.filterState ?? {}, + emitCrossFilters: true, + } as unknown as EchartsButterflyChartProps; + + const transformed = transformProps(chartProps); + render( + , + ); + + const lastCall = mockedEchart.mock.calls[mockedEchart.mock.calls.length - 1]; + const { eventHandlers, selectedValues } = lastCall[0] as { + eventHandlers: EventHandlers; + selectedValues: Record; + }; + return { + eventHandlers, + onContextMenu, + setDataMask, + onLegendStateChanged, + selectedValues, + }; +} + +beforeEach(() => { + mockedEchart.mockClear(); +}); + +test('context menu exposes drill to detail for the selected category', () => { + const { eventHandlers, onContextMenu } = setup(); + + eventHandlers.contextmenu({ + name: 'A', + data: { name: categoryKeyA }, + event: { stop: jest.fn(), event: { clientX: 10, clientY: 20 } }, + }); + + expect(onContextMenu).toHaveBeenCalledTimes(1); + const [x, y, payload] = onContextMenu.mock.calls[0]; + expect(x).toBe(10); + expect(y).toBe(20); + expect(payload.drillToDetail).toEqual([ + expect.objectContaining({ + col: 'category', + op: '==', + val: 'A', + formattedVal: 'A', + }), + ]); +}); + +test('context menu exposes drill by for the selected category', () => { + const { eventHandlers, onContextMenu } = setup(); + + eventHandlers.contextmenu({ + name: 'A', + data: { name: categoryKeyA }, + event: { stop: jest.fn(), event: { clientX: 10, clientY: 20 } }, + }); + + const payload = onContextMenu.mock.calls[0][2]; + expect(payload.drillBy).toEqual({ + filters: [ + expect.objectContaining({ + col: 'category', + op: '==', + val: 'A', + formattedVal: 'A', + }), + ], + groupbyFieldName: 'groupby', + }); +}); + +test('click emits cross-filter for the selected category', () => { + const { eventHandlers, setDataMask } = setup(); + + eventHandlers.click({ name: 'B', data: { name: categoryKeyB } }); + + expect(setDataMask).toHaveBeenCalledWith( + expect.objectContaining({ + extraFormData: { + filters: [{ col: 'category', op: 'IN', val: ['B'] }], + }, + filterState: { + value: [['B']], + selectedValues: [categoryKeyB], + }, + }), + ); +}); + +test('click clears cross-filter when the category is already selected', () => { + const { eventHandlers, setDataMask } = setup({ + filterState: { selectedValues: [categoryKeyB] }, + }); + + eventHandlers.click({ name: 'B', data: { name: categoryKeyB } }); + + expect(setDataMask).toHaveBeenCalledWith( + expect.objectContaining({ + extraFormData: { + filters: [], + }, + filterState: { + value: null, + selectedValues: null, + }, + }), + ); +}); + +test('legend selection forwards legend state to the chart hook', () => { + const onLegendStateChanged = jest.fn(); + const { eventHandlers } = setup({ onLegendStateChanged }); + const selected = { left_sum: true, right_sum: false }; + + eventHandlers.legendselectchanged({ selected }); + eventHandlers.legendselectall({ selected }); + eventHandlers.legendinverseselect({ selected }); + + expect(onLegendStateChanged).toHaveBeenCalledTimes(3); + expect(onLegendStateChanged).toHaveBeenCalledWith(selected); +}); + +test('passes selectedValues through to the chart component', () => { + const { selectedValues } = setup({ + filterState: { selectedValues: [categoryKeyA] }, + }); + + expect(selectedValues).toEqual({ 0: categoryKeyA }); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/transformProps.test.ts index e80f04aed9b6..97ab29c70895 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Butterfly/transformProps.test.ts @@ -18,25 +18,40 @@ */ import { ChartProps } from '@superset-ui/core'; import { supersetTheme } from '@apache-superset/core/theme'; +import type { CallbackDataParams } from 'echarts/types/src/util/types'; import { EchartsButterflyChartProps, ButterflyTransformedProps, } from '../../src/Butterfly/types'; import transformProps from '../../src/Butterfly/transformProps'; -import { NULL_STRING } from '../../src/constants'; +import { NULL_STRING, OpacityEnum } from '../../src/constants'; -type SeriesDataPoint = { value?: number } | number; +const categoryKeyA = 'A__["A"]'; +const categoryKeyB = 'B__["B"]'; + +type SeriesDataPoint = { + name?: string; + value?: number; + itemStyle?: { opacity?: number }; +}; type ButterflyTestSeries = { name?: string; data?: SeriesDataPoint[]; itemStyle?: { color?: string }; - label?: { show?: boolean }; + label?: { + show?: boolean; + formatter?: (params: CallbackDataParams) => string; + }; }; type ButterflyTestEchartOptions = { series?: ButterflyTestSeries[]; - xAxis?: { name?: string; nameGap?: number }; + xAxis?: { + name?: string; + nameGap?: number; + axisLabel?: { formatter?: (value: number) => string }; + }; yAxis?: { name?: string; nameGap?: number; @@ -45,7 +60,10 @@ type ButterflyTestEchartOptions = { }; legend?: { orient?: string; data?: string[] }; grid?: { left?: number; top?: number }; - tooltip?: { show?: boolean }; + tooltip?: { + show?: boolean; + formatter?: (params: CallbackDataParams | CallbackDataParams[]) => string; + }; }; const getEchartOptions = ( @@ -55,13 +73,7 @@ const getEchartOptions = ( const extractSeriesValues = (props: ButterflyTransformedProps) => { const series = getEchartOptions(props).series ?? []; - return series.map(item => - (item.data ?? []).map(entry => - typeof entry === 'object' && entry !== null && 'value' in entry - ? entry.value - : entry, - ), - ); + return series.map(item => (item.data ?? []).map(entry => entry.value)); }; const extractSeriesNames = (props: ButterflyTransformedProps) => { @@ -88,19 +100,22 @@ const createChartProps = ( overrides: Record = {}, queryData: Record[] = data, ) => - new ChartProps({ - formData: { ...formData, ...overrides }, - width: 800, - height: 600, - queriesData: [{ data: queryData }], - theme: supersetTheme, - ...((overrides.hooks ? { hooks: overrides.hooks } : {}) as object), - }); + ({ + ...new ChartProps({ + formData: { ...formData, ...overrides }, + width: 800, + height: 600, + queriesData: [{ data: queryData }], + theme: supersetTheme, + ...((overrides.hooks ? { hooks: overrides.hooks } : {}) as object), + }), + filterState: overrides.filterState ?? {}, + emitCrossFilters: overrides.emitCrossFilters, + inContextMenu: overrides.inContextMenu, + }) as unknown as EchartsButterflyChartProps; test('transforms chart props into diverging bar series', () => { - const transformedProps = transformProps( - createChartProps() as unknown as EchartsButterflyChartProps, - ); + const transformedProps = transformProps(createChartProps()); expect(extractSeriesValues(transformedProps)).toEqual([ [-10, -5], @@ -108,11 +123,23 @@ test('transforms chart props into diverging bar series', () => { ]); }); +test('assigns composite category keys to each bar data point', () => { + const transformedProps = transformProps(createChartProps()); + const series = getEchartOptions(transformedProps).series ?? []; + + expect(series[0]?.data?.map(point => point.name)).toEqual([ + categoryKeyA, + categoryKeyB, + ]); + expect(series[1]?.data?.map(point => point.name)).toEqual([ + categoryKeyA, + categoryKeyB, + ]); +}); + test('uses absolute values for negative right-side metrics', () => { const transformedProps = transformProps( - createChartProps({}, [ - { category: 'A', left_sum: -8, right_sum: -15 }, - ]) as unknown as EchartsButterflyChartProps, + createChartProps({}, [{ category: 'A', left_sum: -8, right_sum: -15 }]), ); expect(extractSeriesValues(transformedProps)).toEqual([[-8], [15]]); @@ -122,7 +149,7 @@ test('formats null categories and missing metric values', () => { const transformedProps = transformProps( createChartProps({}, [ { category: null, left_sum: undefined, right_sum: 7 }, - ]) as unknown as EchartsButterflyChartProps, + ]), ); const { yAxis } = getEchartOptions(transformedProps); @@ -141,7 +168,7 @@ test('applies custom series labels, colors, and axis titles', () => { right_color: { r: 0, g: 255, b: 0 }, x_axis_label: 'Value axis', y_axis_label: 'Category axis', - }) as unknown as EchartsButterflyChartProps, + }), ); const { series, xAxis, yAxis } = getEchartOptions(transformedProps); @@ -163,7 +190,7 @@ test('applies legend orientation, sort, and axis margin settings', () => { xAxisLabelRotation: 45, x_axis_title_margin: 60, y_axis_title_margin: 80, - }) as unknown as EchartsButterflyChartProps, + }), ); const { legend, xAxis, yAxis, grid } = getEchartOptions(transformedProps); @@ -178,9 +205,7 @@ test('applies legend orientation, sort, and axis margin settings', () => { test('hides value labels when showValue is false', () => { const transformedProps = transformProps( - createChartProps({ - showValue: false, - }) as unknown as EchartsButterflyChartProps, + createChartProps({ showValue: false }), ); const { series } = getEchartOptions(transformedProps); @@ -188,15 +213,129 @@ test('hides value labels when showValue is false', () => { expect(series?.[1]?.label?.show).toBe(false); }); -test('hides tooltip while the context menu is open', () => { +test('hides zero value labels but keeps non-zero labels', () => { const transformedProps = transformProps( - createChartProps({}, data) as unknown as EchartsButterflyChartProps, + createChartProps({}, [{ category: 'A', left_sum: 0, right_sum: 12 }]), + ); + const formatter = + getEchartOptions(transformedProps).series?.[0]?.label?.formatter; + + expect(formatter?.({ value: 0 } as CallbackDataParams)).toBe(''); + expect(formatter?.({ value: -10 } as CallbackDataParams)).toBe('10'); +}); + +test('formats axis and tooltip values as absolute numbers', () => { + const transformedProps = transformProps(createChartProps()); + const { xAxis, tooltip } = getEchartOptions(transformedProps); + + expect(xAxis?.axisLabel?.formatter?.(-25)).toBe('25'); + + const tooltipHtml = tooltip?.formatter?.([ + { + name: categoryKeyA, + dataIndex: 0, + seriesName: 'left_sum', + value: -10, + } as CallbackDataParams, + { + name: categoryKeyA, + dataIndex: 0, + seriesName: 'right_sum', + value: 25, + } as CallbackDataParams, + ]); + + expect(tooltipHtml).toContain('A'); + expect(tooltipHtml).not.toContain(categoryKeyA); + expect(tooltipHtml).toContain('left_sum'); + expect(tooltipHtml).toContain('right_sum'); + expect(tooltipHtml).toContain('10'); + expect(tooltipHtml).toContain('25'); +}); + +test('shows the category label in the tooltip when ECharts reports a unique key', () => { + const transformedProps = transformProps(createChartProps()); + const tooltipHtml = getEchartOptions(transformedProps).tooltip?.formatter?.({ + name: categoryKeyA, + seriesName: 'left_sum', + value: -10, + } as CallbackDataParams); + + expect(tooltipHtml).toContain('A'); + expect(tooltipHtml).not.toContain(categoryKeyA); +}); + +test('hides tooltip while the context menu is open', () => { + const transformedProps = transformProps(createChartProps()); + const withContextMenu = transformProps( + createChartProps({ inContextMenu: true }), ); - const withContextMenu = transformProps({ - ...createChartProps(), - inContextMenu: true, - } as unknown as EchartsButterflyChartProps); expect(getEchartOptions(transformedProps).tooltip?.show).toBe(true); expect(getEchartOptions(withContextMenu).tooltip?.show).toBe(false); }); + +test('builds labelMap and groupby for drill and cross-filter handlers', () => { + const transformedProps = transformProps(createChartProps()); + + expect(transformedProps.groupby).toEqual(['category']); + expect(transformedProps.labelMap).toEqual({ + 'A__["A"]': ['A'], + 'B__["B"]': ['B'], + }); +}); + +test('uses unique keys for interactions and readable labels on the y-axis', () => { + const transformedProps = transformProps( + createChartProps({ groupby: ['country', 'state'] }, [ + { country: 'US', state: 'CA', left_sum: 4, right_sum: 6 }, + { country: 'US', state: 'NY', left_sum: 8, right_sum: 3 }, + ]), + ); + const series = getEchartOptions(transformedProps).series ?? []; + const firstKey = 'US, CA__["US","CA"]'; + const secondKey = 'US, NY__["US","NY"]'; + + expect(firstKey).not.toBe(secondKey); + expect(series[0]?.data?.map(point => point.name)).toEqual([ + firstKey, + secondKey, + ]); + expect(transformedProps.labelMap).toEqual({ + [firstKey]: ['US', 'CA'], + [secondKey]: ['US', 'NY'], + }); + expect(getEchartOptions(transformedProps).yAxis?.data).toEqual([ + 'US, CA', + 'US, NY', + ]); +}); + +test('dims unselected categories when a cross-filter is active', () => { + const transformedProps = transformProps( + createChartProps({ + filterState: { selectedValues: [categoryKeyA] }, + }), + ); + const series = getEchartOptions(transformedProps).series ?? []; + + expect(series[0]?.data?.[0]?.itemStyle?.opacity).toBe( + OpacityEnum.NonTransparent, + ); + expect(series[0]?.data?.[1]?.itemStyle?.opacity).toBe( + OpacityEnum.SemiTransparent, + ); + expect(series[1]?.data?.[1]?.itemStyle?.opacity).toBe( + OpacityEnum.SemiTransparent, + ); +}); + +test('maps selectedValues to category indexes', () => { + const transformedProps = transformProps( + createChartProps({ + filterState: { selectedValues: [categoryKeyB] }, + }), + ); + + expect(transformedProps.selectedValues).toEqual({ 1: categoryKeyB }); +}); diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx index 5ccb7fc546d4..73bce191e29c 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx @@ -19,7 +19,12 @@ import { Router } from 'react-router-dom'; import { createMemoryHistory } from 'history'; import { getExtensionsRegistry, VizType } from '@superset-ui/core'; -import { render, screen, userEvent } from 'spec/helpers/testing-library'; +import { + fireEvent, + render, + screen, + userEvent, +} from 'spec/helpers/testing-library'; import { enableMobileConsumptionFlag, mockMobileMatchMedia, @@ -106,6 +111,13 @@ jest.mock('src/dashboard/components/FiltersBadge', () => ({ ), })); +jest.mock('./SliceInfo', () => ({ + __esModule: true, + default: ({ slice }: { slice: { description: string } }) => ( +
{slice.description}
+ ), +})); + jest.mock('src/dashboard/util/isEmbedded', () => ({ isEmbedded: jest.fn().mockReturnValue(false), })); @@ -571,6 +583,172 @@ test('Correct actions to "SliceHeaderControls"', () => { expect(props.handleToggleFullSize).toHaveBeenCalledTimes(1); }); +test('Should show chart description info icon when description exists and is collapsed', () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + expect(screen.getByTestId('chart-description-info-icon')).toBeInTheDocument(); +}); + +test('Should hide chart description info icon when description is expanded', () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: true, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + expect( + screen.queryByTestId('chart-description-info-icon'), + ).not.toBeInTheDocument(); +}); + +test('Should hide chart description info icon when chart has no description', () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: '', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + expect( + screen.queryByTestId('chart-description-info-icon'), + ).not.toBeInTheDocument(); +}); + +test('Chart description icon is a keyboard-focusable button', () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + const icon = screen.getByRole('button', { name: 'Chart description' }); + icon.focus(); + expect(icon).toHaveFocus(); +}); + +test('Should show chart description in popover on hover', async () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + + expect(screen.queryByTestId('slice-info')).not.toBeInTheDocument(); + + await userEvent.hover(screen.getByTestId('chart-description-info-icon')); + + expect(await screen.findByTestId('slice-info')).toHaveTextContent( + 'Test chart description', + ); +}); + +test('Should show chart description in popover on click', async () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + + expect(screen.queryByTestId('slice-info')).not.toBeInTheDocument(); + + await userEvent.click(screen.getByTestId('chart-description-info-icon')); + + expect(await screen.findByTestId('slice-info')).toHaveTextContent( + 'Test chart description', + ); +}); + +test('Should open chart description popover with Enter', async () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + + const icon = screen.getByRole('button', { name: 'Chart description' }); + expect(screen.queryByTestId('slice-info')).not.toBeInTheDocument(); + + // user-event v12 (pinned in this repo) doesn't expose .keyboard(); use + // fireEvent to dispatch keydown directly to the focused icon. + icon.focus(); + fireEvent.keyDown(icon, { key: 'Enter' }); + expect(await screen.findByTestId('slice-info')).toHaveTextContent( + 'Test chart description', + ); +}); + +test('Should open chart description popover with Space', async () => { + const props = createProps({ + slice: { + ...createProps().slice, + description: 'Test chart description', + }, + isExpanded: false, + }); + render(, { + useRedux: true, + useRouter: true, + initialState, + }); + + const icon = screen.getByRole('button', { name: 'Chart description' }); + expect(screen.queryByTestId('slice-info')).not.toBeInTheDocument(); + + icon.focus(); + fireEvent.keyDown(icon, { key: ' ' }); + expect(await screen.findByTestId('slice-info')).toHaveTextContent( + 'Test chart description', + ); +}); + test('Add extension to SliceHeader', () => { const extensionsRegistry = getExtensionsRegistry(); extensionsRegistry.set('dashboard.slice.header', () => ( diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceInfo.test.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceInfo.test.tsx new file mode 100644 index 000000000000..9306174a37a1 --- /dev/null +++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceInfo.test.tsx @@ -0,0 +1,64 @@ +/** + * 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 { render, screen } from 'spec/helpers/testing-library'; +import SliceInfo from './SliceInfo'; + +jest.mock('@superset-ui/core/components/SafeMarkdown/SafeMarkdown', () => ({ + SafeMarkdown: ({ source }: { source: string }) => ( +
{source}
+ ), +})); + +const setup = (description = 'Default description') => + render(); + +test('Should render chart description', () => { + setup('Hello world'); + expect(screen.getByTestId('safe-markdown')).toHaveTextContent('Hello world'); +}); + +test('Should pass markdown source to SafeMarkdown', () => { + const markdown = [ + '# Chart overview', + '', + 'This chart shows **revenue** by region.', + '', + '- North', + '- South', + '', + '[Learn more](https://superset.apache.org)', + ].join('\n'); + + setup(markdown); + expect(screen.getByTestId('safe-markdown').textContent).toBe(markdown); +}); + +test('Should render long markdown description without crashing', () => { + const longMarkdown = `# Summary\n\n${'Long description paragraph. '.repeat(100)}`; + + setup(longMarkdown); + const content = screen.getByTestId('safe-markdown').textContent ?? ''; + expect(content).toContain('# Summary'); + expect(content.match(/Long description paragraph\./g)).toHaveLength(100); +}); + +test('Should render empty description without crashing', () => { + setup(''); + expect(screen.getByTestId('safe-markdown')).toBeEmptyDOMElement(); +}); diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceInfo.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceInfo.tsx new file mode 100644 index 000000000000..cd452280d0c9 --- /dev/null +++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceInfo.tsx @@ -0,0 +1,45 @@ +/** + * 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 { FC } from 'react'; +import { css, styled } from '@apache-superset/core/theme'; +import { SafeMarkdown } from '@superset-ui/core/components'; + +const SliceInfoContainer = styled.div` + ${({ theme }) => css` + max-width: 350px; + max-height: 400px; + overflow-y: auto; + overflow-x: auto; + font-size: ${theme.fontSize}px; + `} +`; + +interface SliceInfoProps { + slice: { + description: string; + }; +} + +const SliceInfo: FC = ({ slice }) => ( + + + +); + +export default SliceInfo; diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index ba8e7b27617d..f674d91ee79a 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -28,6 +28,7 @@ import { import { t } from '@apache-superset/core/translation'; import { getExtensionsRegistry, + handleKeyboardActivation, JsonObject, QueryData, VizType, @@ -40,7 +41,12 @@ import { } from '@apache-superset/core/theme'; import { useUiConfig } from 'src/components/UiConfigContext'; import { isEmbedded } from 'src/dashboard/util/isEmbedded'; -import { Tooltip, EditableTitle, Icons } from '@superset-ui/core/components'; +import { + Tooltip, + EditableTitle, + Icons, + Popover, +} from '@superset-ui/core/components'; import { useSelector } from 'react-redux'; import SliceHeaderControls from 'src/dashboard/components/SliceHeaderControls'; import { useIsMobile } from 'src/hooks/useIsMobile'; @@ -52,6 +58,7 @@ import { getSliceHeaderTooltip } from 'src/dashboard/util/getSliceHeaderTooltip' import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; import RowCountLabel from 'src/components/RowCountLabel'; import { Link } from 'react-router-dom'; +import SliceInfo from './SliceInfo'; const extensionsRegistry = getExtensionsRegistry(); @@ -210,6 +217,8 @@ const SliceHeader = forwardRef( state => state.charts[slice.slice_id].queriesResponse?.[1], ); + const [isDescriptionOpen, setIsDescriptionOpen] = useState(false); + const theme = useTheme(); const rowLimit = Number(formData.row_limit ?? 0); @@ -339,6 +348,27 @@ const SliceHeader = forwardRef( )} + {slice.description && !isExpanded && ( + } + placement="leftBottom" + open={isDescriptionOpen} + onOpenChange={setIsDescriptionOpen} + > + + setIsDescriptionOpen(open => !open), + )} + /> + + )} {!uiConfig.hideChartControls && ( )}