mirror of
https://github.com/apache/superset.git
synced 2026-04-17 15:15:20 +00:00
fix(echarts): restore dashed line style for time comparison series (#37135)
This commit is contained in:
@@ -28,11 +28,16 @@ import { hasTimeOffset } from './timeOffset';
|
||||
export const isDerivedSeries = (
|
||||
series: JsonObject,
|
||||
formData: QueryFormData,
|
||||
seriesName?: string,
|
||||
): boolean => {
|
||||
const comparisonType = formData.comparison_type;
|
||||
if (comparisonType !== ComparisonType.Values) {
|
||||
return false;
|
||||
}
|
||||
const timeCompare: string[] = ensureIsArray(formData?.time_compare);
|
||||
return hasTimeOffset(series, timeCompare);
|
||||
// Check if series matches time offset patterns or exact match (single metric case)
|
||||
return (
|
||||
hasTimeOffset(series, timeCompare) ||
|
||||
(seriesName !== undefined && timeCompare.includes(seriesName))
|
||||
);
|
||||
};
|
||||
|
||||
@@ -97,3 +97,24 @@ test('should be false if series name invalid', () => {
|
||||
};
|
||||
expect(isDerivedSeries(series, formDataWithActualTypes)).toEqual(false);
|
||||
});
|
||||
|
||||
test('should be true for exact match when seriesName parameter is provided', () => {
|
||||
const exactMatchSeries = {
|
||||
id: '1 week ago',
|
||||
name: '1 week ago',
|
||||
data: [100],
|
||||
};
|
||||
const formDataWithTimeCompare = {
|
||||
...formData,
|
||||
comparison_type: ComparisonType.Values,
|
||||
time_compare: ['1 week ago'],
|
||||
};
|
||||
// Without seriesName parameter, exact match is not detected via hasTimeOffset
|
||||
expect(isDerivedSeries(exactMatchSeries, formDataWithTimeCompare)).toEqual(
|
||||
false,
|
||||
);
|
||||
// With seriesName parameter, exact match is detected
|
||||
expect(
|
||||
isDerivedSeries(exactMatchSeries, formDataWithTimeCompare, '1 week ago'),
|
||||
).toEqual(true);
|
||||
});
|
||||
|
||||
@@ -45,6 +45,7 @@ import { GenericDataType } from '@apache-superset/core/api/core';
|
||||
import {
|
||||
extractExtraMetrics,
|
||||
getOriginalSeries,
|
||||
getTimeOffset,
|
||||
isDerivedSeries,
|
||||
} from '@superset-ui/chart-controls';
|
||||
import type { EChartsCoreOption } from 'echarts/core';
|
||||
@@ -309,7 +310,7 @@ export default function transformProps(
|
||||
const array = ensureIsArray(chartProps.rawFormData?.time_compare);
|
||||
const inverted = invert(verboseMap);
|
||||
|
||||
let patternIncrement = 0;
|
||||
const offsetLineWidths: { [key: string]: number } = {};
|
||||
|
||||
// For horizontal bar charts, calculate min/max from data to avoid cutting off labels
|
||||
const shouldCalculateDataBounds =
|
||||
@@ -320,17 +321,33 @@ export default function transformProps(
|
||||
let dataMin: number | undefined;
|
||||
|
||||
rawSeries.forEach(entry => {
|
||||
const derivedSeries = isDerivedSeries(entry, chartProps.rawFormData);
|
||||
const lineStyle: LineStyleOption = {};
|
||||
if (derivedSeries) {
|
||||
patternIncrement += 1;
|
||||
// use a combination of dash and dot for the line style
|
||||
lineStyle.type = [(patternIncrement % 5) + 1, (patternIncrement % 3) + 1];
|
||||
lineStyle.opacity = OpacityEnum.DerivedSeries;
|
||||
}
|
||||
|
||||
const entryName = String(entry.name || '');
|
||||
const seriesName = inverted[entryName] || entryName;
|
||||
// isDerivedSeries checks for time comparison series patterns:
|
||||
// - "metric__1 day ago" pattern (via hasTimeOffset)
|
||||
// - "1 day ago, groupby" pattern (via hasTimeOffset)
|
||||
// - exact match "1 day ago" (via seriesName parameter)
|
||||
const derivedSeries = isDerivedSeries(
|
||||
entry,
|
||||
chartProps.rawFormData,
|
||||
seriesName,
|
||||
);
|
||||
const lineStyle: LineStyleOption = {};
|
||||
if (derivedSeries) {
|
||||
// Get the time offset for this series to assign different dash patterns
|
||||
const offset = getTimeOffset(entry, array) || seriesName;
|
||||
if (!offsetLineWidths[offset]) {
|
||||
offsetLineWidths[offset] = Object.keys(offsetLineWidths).length + 1;
|
||||
}
|
||||
// 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.opacity = OpacityEnum.DerivedSeries;
|
||||
}
|
||||
|
||||
// Calculate min/max from data for horizontal bar charts
|
||||
if (shouldCalculateDataBounds && entry.data && Array.isArray(entry.data)) {
|
||||
@@ -349,14 +366,17 @@ export default function transformProps(
|
||||
|
||||
let colorScaleKey = getOriginalSeries(seriesName, array);
|
||||
|
||||
// If this series name exactly matches a time compare value, it's a time-shifted series
|
||||
// and we need to find the corresponding original series for color matching
|
||||
if (array && array.includes(seriesName)) {
|
||||
// Find the original series (first non-time-compare series)
|
||||
const originalSeries = rawSeries.find(s => {
|
||||
const sName = inverted[String(s.name || '')] || String(s.name || '');
|
||||
return !array.includes(sName);
|
||||
});
|
||||
// If series name exactly matches a time offset (single metric case),
|
||||
// find the original series for color matching
|
||||
if (derivedSeries && array.includes(seriesName)) {
|
||||
const originalSeries = rawSeries.find(
|
||||
s =>
|
||||
!isDerivedSeries(
|
||||
s,
|
||||
chartProps.rawFormData,
|
||||
inverted[String(s.name || '')] || String(s.name || ''),
|
||||
),
|
||||
);
|
||||
if (originalSeries) {
|
||||
const originalSeriesName =
|
||||
inverted[String(originalSeries.name || '')] ||
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
AnnotationStyle,
|
||||
AnnotationType,
|
||||
ChartProps,
|
||||
ComparisonType,
|
||||
EventAnnotationLayer,
|
||||
FormulaAnnotationLayer,
|
||||
IntervalAnnotationLayer,
|
||||
@@ -740,6 +741,170 @@ describe('legend sorting', () => {
|
||||
});
|
||||
});
|
||||
|
||||
const timeCompareFormData: SqlaFormData = {
|
||||
colorScheme: 'bnbColors',
|
||||
datasource: '3__table',
|
||||
granularity_sqla: 'ds',
|
||||
metric: 'sum__num',
|
||||
viz_type: 'my_viz',
|
||||
};
|
||||
|
||||
const timeCompareChartPropsConfig = {
|
||||
formData: timeCompareFormData,
|
||||
width: 800,
|
||||
height: 600,
|
||||
theme: supersetTheme,
|
||||
};
|
||||
|
||||
test('should apply dashed line style to time comparison series with single metric', () => {
|
||||
const queriesDataWithTimeCompare = [
|
||||
{
|
||||
data: [
|
||||
{ sum__num: 100, '1 week ago': 80, __timestamp: 599616000000 },
|
||||
{ sum__num: 150, '1 week ago': 120, __timestamp: 599916000000 },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
const chartProps = new ChartProps({
|
||||
...timeCompareChartPropsConfig,
|
||||
formData: {
|
||||
...timeCompareFormData,
|
||||
time_compare: ['1 week ago'],
|
||||
comparison_type: ComparisonType.Values,
|
||||
},
|
||||
queriesData: queriesDataWithTimeCompare,
|
||||
});
|
||||
|
||||
const transformed = transformProps(
|
||||
chartProps as unknown as EchartsTimeseriesChartProps,
|
||||
);
|
||||
const series = transformed.echartOptions.series as any[];
|
||||
|
||||
const mainSeries = series.find(s => s.name === 'sum__num');
|
||||
const comparisonSeries = series.find(s => s.name === '1 week ago');
|
||||
|
||||
expect(mainSeries).toBeDefined();
|
||||
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(comparisonSeries.lineStyle?.type[0]).toBeGreaterThanOrEqual(4);
|
||||
expect(comparisonSeries.lineStyle?.type[1]).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
|
||||
test('should apply dashed line style to time comparison series with metric__offset pattern', () => {
|
||||
const queriesDataWithTimeCompare = [
|
||||
{
|
||||
data: [
|
||||
{
|
||||
sum__num: 100,
|
||||
'sum__num__1 week ago': 80,
|
||||
__timestamp: 599616000000,
|
||||
},
|
||||
{
|
||||
sum__num: 150,
|
||||
'sum__num__1 week ago': 120,
|
||||
__timestamp: 599916000000,
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
const chartProps = new ChartProps({
|
||||
...timeCompareChartPropsConfig,
|
||||
formData: {
|
||||
...timeCompareFormData,
|
||||
time_compare: ['1 week ago'],
|
||||
comparison_type: ComparisonType.Values,
|
||||
},
|
||||
queriesData: queriesDataWithTimeCompare,
|
||||
});
|
||||
|
||||
const transformed = transformProps(
|
||||
chartProps as unknown as EchartsTimeseriesChartProps,
|
||||
);
|
||||
const series = transformed.echartOptions.series as any[];
|
||||
|
||||
const mainSeries = series.find(s => s.name === 'sum__num');
|
||||
const comparisonSeries = series.find(s => s.name === 'sum__num__1 week ago');
|
||||
|
||||
expect(mainSeries).toBeDefined();
|
||||
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(comparisonSeries.lineStyle?.type[0]).toBeGreaterThanOrEqual(4);
|
||||
expect(comparisonSeries.lineStyle?.type[1]).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
|
||||
test('should apply connectNulls to time comparison series', () => {
|
||||
const queriesDataWithNulls = [
|
||||
{
|
||||
data: [
|
||||
{ sum__num: 100, '1 week ago': null, __timestamp: 599616000000 },
|
||||
{ sum__num: 150, '1 week ago': 120, __timestamp: 599916000000 },
|
||||
{ sum__num: 200, '1 week ago': null, __timestamp: 600216000000 },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
const chartProps = new ChartProps({
|
||||
...timeCompareChartPropsConfig,
|
||||
formData: {
|
||||
...timeCompareFormData,
|
||||
time_compare: ['1 week ago'],
|
||||
comparison_type: ComparisonType.Values,
|
||||
},
|
||||
queriesData: queriesDataWithNulls,
|
||||
});
|
||||
|
||||
const transformed = transformProps(
|
||||
chartProps as unknown as EchartsTimeseriesChartProps,
|
||||
);
|
||||
const series = transformed.echartOptions.series as any[];
|
||||
|
||||
const comparisonSeries = series.find(s => s.name === '1 week ago');
|
||||
|
||||
expect(comparisonSeries).toBeDefined();
|
||||
expect(comparisonSeries.connectNulls).toBe(true);
|
||||
});
|
||||
|
||||
test('should not apply dashed line style for non-Values comparison types', () => {
|
||||
const queriesDataWithTimeCompare = [
|
||||
{
|
||||
data: [
|
||||
{ sum__num: 100, '1 week ago': 80, __timestamp: 599616000000 },
|
||||
{ sum__num: 150, '1 week ago': 120, __timestamp: 599916000000 },
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
const chartProps = new ChartProps({
|
||||
...timeCompareChartPropsConfig,
|
||||
formData: {
|
||||
...timeCompareFormData,
|
||||
time_compare: ['1 week ago'],
|
||||
comparison_type: ComparisonType.Difference,
|
||||
},
|
||||
queriesData: queriesDataWithTimeCompare,
|
||||
});
|
||||
|
||||
const transformed = transformProps(
|
||||
chartProps as unknown as EchartsTimeseriesChartProps,
|
||||
);
|
||||
const series = transformed.echartOptions.series as any[];
|
||||
|
||||
const comparisonSeries = series.find(s => s.name === '1 week ago');
|
||||
|
||||
expect(comparisonSeries).toBeDefined();
|
||||
// Non-Values comparison types don't get dashed styling (isDerivedSeries returns false)
|
||||
expect(Array.isArray(comparisonSeries.lineStyle?.type)).toBe(false);
|
||||
expect(comparisonSeries.connectNulls).toBeFalsy();
|
||||
});
|
||||
|
||||
test('EchartsTimeseries AUTO mode should detect single currency and format with $ for USD', () => {
|
||||
const chartProps = new ChartProps<SqlaFormData>({
|
||||
...chartPropsConfig,
|
||||
|
||||
Reference in New Issue
Block a user