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 e53070ceb81..237950626fb 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -124,6 +124,60 @@ import { import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser'; import { mergeCustomEChartOptions } from '../utils/mergeCustomEChartOptions'; +const visibleDashPatterns: ([number, number] | 'dashed' | 'dotted')[] = [ + 'dashed', + 'dotted', + [6, 15], // narrow dashed + [2, 10], // wide dotted + [20, 3], // wide dashed +]; +const visibleSymbols = [ + 'rect', + 'triangle', + 'diamond', + 'roundRect', + 'pin', +] as const; + +function getSymbolMarker(symbol: string, color: string) { + const size = 10; + switch (symbol) { + case 'circle': + return ``; + case 'rect': + return ``; + case 'roundRect': + return ``; + case 'triangle': + return ``; + case 'diamond': + return ``; + case 'pin': + return ``; + default: + return ``; + } +} + export default function transformProps( chartProps: EchartsTimeseriesChartProps, ): TimeseriesChartTransformedProps { @@ -346,7 +400,8 @@ export default function transformProps( ); const lineStyle: LineStyleOption = {}; - if (derivedSeries) { + let lineSymbol; + if (derivedSeries && timeShiftColor) { // Get the time offset for this series to assign different dash patterns const offset = getTimeOffset(entry, array) || seriesName; if (!offsetLineWidths[offset]) { @@ -355,11 +410,11 @@ export default function transformProps( // Use visible dash patterns that vary by offset index // Pattern: [dash length, gap length] - scaled to be clearly visible const patternIndex = offsetLineWidths[offset]; - lineStyle.type = [ - (patternIndex % 5) + 4, // dash: 4-8px (visible) - (patternIndex % 3) + 3, // gap: 3-5px (visible) - ]; + lineStyle.type = + visibleDashPatterns[patternIndex % visibleDashPatterns.length]; + lineStyle.opacity = OpacityEnum.DerivedSeries; + lineSymbol = visibleSymbols[patternIndex % visibleSymbols.length]; } // Calculate min/max from data for horizontal bar charts @@ -443,6 +498,7 @@ export default function transformProps( sliceId, isHorizontal, lineStyle, + lineSymbol, timeCompare: array, timeShiftColor, theme, @@ -934,10 +990,16 @@ export default function transformProps( if (value.observation === 0 && stack) { return; } + const seriesForKey = series.find(s => s.name === key); + const symbolForSeries = (seriesForKey as any)?.symbol || 'circle'; + const marker = value.color + ? getSymbolMarker(symbolForSeries, value.color) + : value.marker; const row = formatForecastTooltipSeries({ ...value, seriesName: key, formatter, + marker, }); const annotationRow = annotationLayers.some( 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 799fce3003c..2a5f937075b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts @@ -221,6 +221,7 @@ export function transformSeries( seriesKey?: OptionName; sliceId?: number; isHorizontal?: boolean; + lineSymbol?: string; lineStyle?: LineStyleOption; queryIndex?: number; timeCompare?: string[]; @@ -359,8 +360,10 @@ export function transformSeries( // Use filled circles in dark mode to avoid the white fill issue with hollow circles // Use emptyCircle explicitly in light mode - const symbol = - plotType === 'line' ? (isDarkMode ? 'circle' : 'emptyCircle') : undefined; + let symbol; + if (plotType === 'line') { + symbol = opts.lineSymbol || (isDarkMode ? 'circle' : 'emptyCircle'); + } return { ...series, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts index 4dffdcf59ba..2656bf0a678 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts @@ -92,6 +92,7 @@ export type ForecastValue = { forecastTrend?: number; forecastLower?: number; forecastUpper?: number; + color?: string; }; export type LegendFormData = { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts index 8364f4ed220..47cf7d63440 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts @@ -60,7 +60,7 @@ export const extractForecastValuesFromTooltipParams = ( ): Record => { const values: Record = {}; params.forEach(param => { - const { marker, seriesId, value } = param; + const { marker, seriesId, value, color } = param; const context = extractForecastSeriesContext(seriesId); const numericValue = isHorizontal ? value[0] : value[1]; if (typeof numericValue === 'number') { @@ -69,6 +69,7 @@ export const extractForecastValuesFromTooltipParams = ( marker: marker || '', }; const forecastValues = values[context.name]; + forecastValues.color = color; if (context.type === ForecastSeriesEnum.Observation) forecastValues.observation = numericValue; if (context.type === ForecastSeriesEnum.ForecastTrend) diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts index 9e147a4f245..11114815338 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts @@ -955,6 +955,7 @@ test('should apply dashed line style to time comparison series with single metri formData: { ...timeCompareFormData, time_compare: ['1 week ago'], + timeShiftColor: true, comparison_type: ComparisonType.Values, }, queriesData: queriesDataWithTimeCompare, @@ -974,18 +975,9 @@ test('should apply dashed line style to time comparison series with single metri expect(comparisonSeries).toBeDefined(); // Main series should not have a dash pattern array expect(Array.isArray(mainSeries?.lineStyle?.type)).toBe(false); - // Comparison series should have a visible dash pattern array [dash, gap] - expect(Array.isArray(comparisonSeries?.lineStyle?.type)).toBe(true); - expect( - Array.isArray(comparisonSeries?.lineStyle?.type) - ? comparisonSeries.lineStyle.type[0] - : undefined, - ).toBeGreaterThanOrEqual(4); - expect( - Array.isArray(comparisonSeries?.lineStyle?.type) - ? comparisonSeries.lineStyle.type[1] - : undefined, - ).toBeGreaterThanOrEqual(3); + expect(mainSeries?.lineStyle?.type).not.toBe('dotted'); + // Comparison series should have a visible dash pattern + expect(comparisonSeries?.lineStyle?.type).toBe('dotted'); }); test('should apply dashed line style to time comparison series with metric__offset pattern', () => { @@ -1008,6 +1000,7 @@ test('should apply dashed line style to time comparison series with metric__offs formData: { ...timeCompareFormData, time_compare: ['1 week ago'], + timeShiftColor: true, comparison_type: ComparisonType.Values, }, queriesData: queriesDataWithTimeCompare, @@ -1029,18 +1022,8 @@ test('should apply dashed line style to time comparison series with metric__offs expect(comparisonSeries).toBeDefined(); // Main series should not have a dash pattern array expect(Array.isArray(mainSeries?.lineStyle?.type)).toBe(false); - // Comparison series should have a visible dash pattern array [dash, gap] - expect(Array.isArray(comparisonSeries?.lineStyle?.type)).toBe(true); - expect( - Array.isArray(comparisonSeries?.lineStyle?.type) - ? comparisonSeries.lineStyle.type[0] - : undefined, - ).toBeGreaterThanOrEqual(4); - expect( - Array.isArray(comparisonSeries?.lineStyle?.type) - ? comparisonSeries.lineStyle.type[1] - : undefined, - ).toBeGreaterThanOrEqual(3); + // Comparison series should have a visible dash pattern + expect(comparisonSeries?.lineStyle?.type).toBe('dotted'); }); test('should apply connectNulls to time comparison series', () => { @@ -1352,3 +1335,52 @@ test('should not apply axis bounds calculation when seriesType is not Bar for ho // Should not have explicit max set when seriesType is not Bar expect(xAxisRaw.max).toBeUndefined(); }); + +test('should assign distinct dash patterns for multiple time offsets consistently', () => { + const queriesDataWithMultipleOffsets = [ + createTestQueryData([ + { + sum__num: 100, + '1 year ago': 80, + '2 years ago': 60, + __timestamp: 599616000000, + }, + { + sum__num: 150, + '1 year ago': 120, + '2 years ago': 90, + __timestamp: 599916000000, + }, + ]), + ]; + + const chartProps = createTestChartProps({ + formData: { + ...timeCompareFormData, + time_compare: ['1 year ago', '2 years ago'], + comparison_type: ComparisonType.Values, + timeShiftColor: true, + }, + queriesData: queriesDataWithMultipleOffsets, + }); + + const transformed = transformProps(chartProps); + const series = (transformed.echartOptions.series as SeriesOption[]) || []; + + const series1 = series.find(s => s.name === '1 year ago') as any; + const series2 = series.find(s => s.name === '2 years ago') as any; + + expect(series1).toBeDefined(); + expect(series2).toBeDefined(); + + const pattern1 = series1.lineStyle?.type; + const symbol1 = series1.symbol; + const pattern2 = series2.lineStyle?.type; + const symbol2 = series2.symbol; + + // must be different patterns + expect(pattern1).not.toEqual(pattern2); + + // must be different patterns + expect(symbol1).not.toEqual(symbol2); +});