diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx index 59cfba2319a..89a0269dce6 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx @@ -309,3 +309,93 @@ test('falls back to window resize listener when ResizeObserver is unavailable', addEventListenerSpy.mockRestore(); removeEventListenerSpy.mockRestore(); }); + +// Test for issue #25334: Bar chart cross-filter without dimensions +test('emits cross-filter on X-axis value when no dimensions and categorical X-axis', async () => { + const setDataMaskMock = jest.fn(); + + const propsWithCategoricalXAxis: TimeseriesChartTransformedProps = { + ...defaultProps, + emitCrossFilters: true, + setDataMask: setDataMaskMock, + groupby: [], // No dimensions + xAxis: { + label: 'category_column', + type: AxisType.Category, // Categorical X-axis + }, + }; + + render(); + + // Get the click handler from the mock + const lastCall = mockEchart.mock.calls.at(-1); + expect(lastCall).toBeDefined(); + const [props] = lastCall as [EchartsProps]; + expect(props.eventHandlers).toBeDefined(); + expect(props.eventHandlers?.click).toBeDefined(); + + // Simulate a click event with X-axis data + const clickHandler = props.eventHandlers?.click; + if (clickHandler) { + clickHandler({ + seriesName: 'Sales', // This is the metric name + data: ['Product A', 100], // X-axis value is 'Product A' + name: 'Product A', + dataIndex: 0, + }); + + // Wait for the timer (TIMER_DURATION = 300ms) + await waitFor( + () => { + expect(setDataMaskMock).toHaveBeenCalled(); + }, + { timeout: 500 }, + ); + + // Verify the cross-filter uses the X-axis column and value, not the metric + const dataMaskCall = setDataMaskMock.mock.calls[0][0]; + expect(dataMaskCall.extraFormData.filters).toEqual([ + { + col: 'category_column', // X-axis column + op: 'IN', + val: ['Product A'], // X-axis value, not 'Sales' (metric) + }, + ]); + } +}); + +test('does not emit cross-filter when no dimensions and time-based X-axis', async () => { + const setDataMaskMock = jest.fn(); + + const propsWithTimeXAxis: TimeseriesChartTransformedProps = { + ...defaultProps, + emitCrossFilters: true, + setDataMask: setDataMaskMock, + groupby: [], // No dimensions + xAxis: { + label: '__timestamp', + type: AxisType.Time, // Time-based X-axis (not categorical) + }, + }; + + render(); + + const lastCall = mockEchart.mock.calls.at(-1); + expect(lastCall).toBeDefined(); + const [props] = lastCall as [EchartsProps]; + + // Simulate a click event + const clickHandler = props.eventHandlers?.click; + if (clickHandler) { + clickHandler({ + seriesName: 'Sales', + data: [1609459200000, 100], // Timestamp + name: '2021-01-01', + dataIndex: 0, + }); + + // Wait a bit and verify setDataMask was NOT called + await new Promise(resolve => setTimeout(resolve, 400)); + expect(setDataMaskMock).not.toHaveBeenCalled(); + } +}); 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 b14b7fb6ba6..bd261ba0a2d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx @@ -154,6 +154,43 @@ export default function EchartsTimeseries({ [groupby, labelMap, selectedValues], ); + // Cross-filter using X-axis value when no dimensions are set (issue #25334) + const getXAxisCrossFilterDataMask = useCallback( + (xAxisValue: string | number) => { + const stringValue = String(xAxisValue); + const selected: string[] = Object.values(selectedValues); + let values: string[]; + if (selected.includes(stringValue)) { + values = selected.filter(v => v !== stringValue); + } else { + values = [stringValue]; + } + return { + dataMask: { + extraFormData: { + filters: + values.length === 0 + ? [] + : [ + { + col: xAxis.label, + op: 'IN' as const, + val: values, + }, + ], + }, + filterState: { + label: values.length ? values : undefined, + value: values.length ? values : null, + selectedValues: values.length ? values : null, + }, + }, + isCurrentValueSelected: selected.includes(stringValue), + }; + }, + [selectedValues, xAxis.label], + ); + const handleChange = useCallback( (value: string) => { if (!emitCrossFilters) { @@ -164,9 +201,25 @@ export default function EchartsTimeseries({ [emitCrossFilters, setDataMask, getCrossFilterDataMask], ); + // Handle cross-filter using X-axis value when no dimensions (issue #25334) + const handleXAxisChange = useCallback( + (xAxisValue: string | number) => { + if (!emitCrossFilters) { + return; + } + setDataMask(getXAxisCrossFilterDataMask(xAxisValue).dataMask); + }, + [emitCrossFilters, setDataMask, getXAxisCrossFilterDataMask], + ); + + // Determine if X-axis can be used for cross-filtering (categorical axis without dimensions) + const canCrossFilterByXAxis = + !hasDimensions && xAxis.type === AxisType.Category; + const eventHandlers: EventHandlers = { click: props => { - if (!hasDimensions) { + // Allow cross-filter by dimensions OR by categorical X-axis (issue #25334) + if (!hasDimensions && !canCrossFilterByXAxis) { return; } if (clickTimer.current) { @@ -174,8 +227,14 @@ export default function EchartsTimeseries({ } // Ensure that double-click events do not trigger single click event. So we put it in the timer. clickTimer.current = setTimeout(() => { - const { seriesName: name } = props; - handleChange(name); + if (hasDimensions) { + // Cross-filter by dimension (original behavior) + const { seriesName: name } = props; + handleChange(name); + } else if (canCrossFilterByXAxis && props.data?.[0] != null) { + // Cross-filter by X-axis value when no dimensions (issue #25334) + handleXAxisChange(props.data[0]); + } }, TIMER_DURATION); }, mouseout: () => { @@ -252,12 +311,18 @@ export default function EchartsTimeseries({ }); }); + // Provide cross-filter for dimensions OR categorical X-axis (issue #25334) + let crossFilter; + if (hasDimensions) { + crossFilter = getCrossFilterDataMask(seriesName); + } else if (canCrossFilterByXAxis && data?.[0] != null) { + crossFilter = getXAxisCrossFilterDataMask(data[0]); + } + onContextMenu(pointerEvent.clientX, pointerEvent.clientY, { drillToDetail: drillToDetailFilters, drillBy: { filters: drillByFilters, groupbyFieldName: 'groupby' }, - crossFilter: hasDimensions - ? getCrossFilterDataMask(seriesName) - : undefined, + crossFilter, }); } }, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts index 6ebc30fc226..7e818051c83 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -419,6 +419,7 @@ export default function transformProps( timeCompare: array, timeShiftColor, theme, + hasDimensions: (groupBy?.length ?? 0) > 0, }, ); if (transformedSeries) { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts index 33e8fc446ea..34dfcb34c45 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts @@ -196,6 +196,7 @@ export function transformSeries( timeCompare?: string[]; timeShiftColor?: boolean; theme?: SupersetTheme; + hasDimensions?: boolean; }, ): SeriesOption | undefined { const { name, data } = series; @@ -237,8 +238,12 @@ export function transformSeries( const isConfidenceBand = forecastSeries.type === ForecastSeriesEnum.ForecastLower || forecastSeries.type === ForecastSeriesEnum.ForecastUpper; + // When cross-filtering by X-axis (no dimensions), selectedValues contains + // X-axis values rather than series names, so skip series-level dimming. const isFiltered = - filterState?.selectedValues && !filterState?.selectedValues.includes(name); + opts.hasDimensions !== false && + filterState?.selectedValues && + !filterState?.selectedValues.includes(name); const opacity = isFiltered ? OpacityEnum.SemiTransparent : opts.lineStyle?.opacity || OpacityEnum.NonTransparent; diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts index 143bdf43898..bb2e25ee914 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts @@ -89,6 +89,34 @@ describe('transformSeries', () => { expect((result as any).itemStyle.borderType).toBeUndefined(); expect((result as any).itemStyle.borderColor).toBeUndefined(); }); + + it('should dim series when selectedValues does not include series name (dimension-based filtering)', () => { + const opts = { + filterState: { selectedValues: ['other-series'] }, + hasDimensions: true, + seriesType: EchartsTimeseriesSeriesType.Bar, + timeShiftColor: false, + }; + + const result = transformSeries(series, mockColorScale, 'test-key', opts); + + // OpacityEnum.SemiTransparent = 0.3 + expect((result as any).itemStyle.opacity).toBe(0.3); + }); + + it('should not dim series when hasDimensions is false (X-axis cross-filtering)', () => { + const opts = { + filterState: { selectedValues: ['Product A'] }, + hasDimensions: false, + seriesType: EchartsTimeseriesSeriesType.Bar, + timeShiftColor: false, + }; + + const result = transformSeries(series, mockColorScale, 'test-key', opts); + + // OpacityEnum.NonTransparent = 1 (not dimmed) + expect((result as any).itemStyle.opacity).toBe(1); + }); }); describe('transformNegativeLabelsPosition', () => {