From 651e52fd105029267bd6a833bf3744b282100488 Mon Sep 17 00:00:00 2001 From: Drew Slobodnjak <60050885+drew08t@users.noreply.github.com> Date: Tue, 9 Sep 2025 15:09:44 -0700 Subject: [PATCH] TimeSeries: Improve time compare default styling (#110575) * TimeSeries: Use exported time comparison function * Add alignTimeRangeCompareData to grafana/data * Simplify tooltip time text formatting * Bump scenes version * Add tests for alignTimeRangeCompareData * TimeSeries: Improve time compare default styling * Update Time Comparison panel option menu * Add backwards compatibility for older scenes * Update shouldAlignTimeCompare for typical query * Fix styling when multiple RefId matches * Update default dash to be smaller * Fix tooltip for older versions of scenes --- .../grafana-data/src/dataframe/utils.test.ts | 22 ++--- packages/grafana-data/src/dataframe/utils.ts | 34 ++++++-- .../src/options/builder/timeCompare.tsx | 4 +- .../panel/timeseries/TimeSeriesPanel.tsx | 3 +- .../panel/timeseries/TimeSeriesTooltip.tsx | 1 - public/app/plugins/panel/timeseries/utils.ts | 87 ++++++++++++++++--- 6 files changed, 111 insertions(+), 40 deletions(-) diff --git a/packages/grafana-data/src/dataframe/utils.test.ts b/packages/grafana-data/src/dataframe/utils.test.ts index a1b957f363c..18da26194ff 100644 --- a/packages/grafana-data/src/dataframe/utils.test.ts +++ b/packages/grafana-data/src/dataframe/utils.test.ts @@ -1,3 +1,4 @@ +import { createTheme } from '../themes/createTheme'; import { FieldType } from '../types/dataFrame'; import { TimeRange } from '../types/time'; @@ -118,7 +119,7 @@ describe('alignTimeRangeCompareData', () => { ], }); - alignTimeRangeCompareData(frame, ONE_DAY_MS); + alignTimeRangeCompareData(frame, ONE_DAY_MS, createTheme()); expect(frame.fields[0].values).toEqual([ONE_DAY_MS + 1000, ONE_DAY_MS + 2000, ONE_DAY_MS + 3000]); expect(frame.fields[1].values).toEqual([10, 20, 30]); // non-time fields unchanged @@ -132,13 +133,13 @@ describe('alignTimeRangeCompareData', () => { ], }); - alignTimeRangeCompareData(frame, -ONE_WEEK_MS); + alignTimeRangeCompareData(frame, -ONE_WEEK_MS, createTheme()); // When diff is negative, function does v - diff, so v - (-ONE_WEEK_MS) = v + ONE_WEEK_MS expect(frame.fields[0].values).toEqual([ONE_WEEK_MS + 1000, ONE_WEEK_MS + 2000, ONE_WEEK_MS + 3000]); }); - it('should apply default gray color and timeCompare config', () => { + it('should apply timeCompare config', () => { const frame = toDataFrame({ fields: [ { name: 'time', type: FieldType.time, values: [1000, 2000] }, @@ -146,10 +147,9 @@ describe('alignTimeRangeCompareData', () => { ], }); - alignTimeRangeCompareData(frame, ONE_DAY_MS); + alignTimeRangeCompareData(frame, ONE_DAY_MS, createTheme()); frame.fields.forEach((field) => { - expect(field.config.color?.fixedColor).toBe('gray'); expect(field.config.custom?.timeCompare).toEqual({ diffMs: ONE_DAY_MS, isTimeShiftQuery: true, @@ -157,16 +157,6 @@ describe('alignTimeRangeCompareData', () => { }); }); - it('should apply custom color when provided', () => { - const frame = toDataFrame({ - fields: [{ name: 'value', type: FieldType.number, values: [10, 20] }], - }); - - alignTimeRangeCompareData(frame, ONE_DAY_MS, 'red'); - - expect(frame.fields[0].config.color?.fixedColor).toBe('red'); - }); - it('should preserve existing config when merging', () => { const frame = toDataFrame({ fields: [ @@ -182,7 +172,7 @@ describe('alignTimeRangeCompareData', () => { ], }); - alignTimeRangeCompareData(frame, ONE_WEEK_MS); + alignTimeRangeCompareData(frame, ONE_WEEK_MS, createTheme()); expect(frame.fields[0].config.displayName).toBe('My Display Name'); expect(frame.fields[0].config.custom?.existingProperty).toBe('existingValue'); diff --git a/packages/grafana-data/src/dataframe/utils.ts b/packages/grafana-data/src/dataframe/utils.ts index c9114e7e647..e4d065f559b 100644 --- a/packages/grafana-data/src/dataframe/utils.ts +++ b/packages/grafana-data/src/dataframe/utils.ts @@ -1,3 +1,5 @@ +import { getFieldSeriesColor } from '../field/fieldColor'; +import { GrafanaTheme2 } from '../themes/types'; import { DataFrame, Field, FieldType } from '../types/dataFrame'; import { TimeRange } from '../types/time'; @@ -129,9 +131,9 @@ export function addRow(dataFrame: DataFrame, row: Record | unkn * Aligns time range comparison data by adjusting timestamps and applying compare-specific styling * @param series - The DataFrame containing the comparison data * @param diff - The time difference in milliseconds to align the timestamps - * @param compareColor - Optional color to use for the comparison series (defaults to 'gray') + * @param theme - The Grafana theme for color calculations */ -export function alignTimeRangeCompareData(series: DataFrame, diff: number, compareColor = 'gray') { +export function alignTimeRangeCompareData(series: DataFrame, diff: number, theme: GrafanaTheme2) { series.fields.forEach((field: Field) => { // Align compare series time stamps with reference series if (field.type === FieldType.time) { @@ -142,10 +144,6 @@ export function alignTimeRangeCompareData(series: DataFrame, diff: number, compa field.config = { ...(field.config ?? {}), - color: { - mode: 'fixed', - fixedColor: compareColor, - }, custom: { ...(field.config?.custom ?? {}), timeCompare: { @@ -154,6 +152,30 @@ export function alignTimeRangeCompareData(series: DataFrame, diff: number, compa }, }, }; + + // Apply visual styling for comparison series + if (field.type === FieldType.number || field.type === FieldType.boolean || field.type === FieldType.enum) { + const seriesColor = getFieldSeriesColor(field, theme).color; + const lineStyle = field.config.custom?.lineStyle; + const isSolid = !lineStyle?.fill || lineStyle.fill === 'solid'; + + if (isSolid) { + field.config.custom = { + ...(field.config.custom ?? {}), + lineStyle: { + fill: 'dash', + dash: [5, 5], + }, + }; + } else { + // For already dashed/dotted lines, reduce opacity + const tinycolor = require('tinycolor2'); + field.config.color = { + mode: 'fixed', + fixedColor: tinycolor(seriesColor).setAlpha(0.5).toString(), + }; + } + } }); } diff --git a/packages/grafana-ui/src/options/builder/timeCompare.tsx b/packages/grafana-ui/src/options/builder/timeCompare.tsx index de19457d80f..222856ff9f4 100644 --- a/packages/grafana-ui/src/options/builder/timeCompare.tsx +++ b/packages/grafana-ui/src/options/builder/timeCompare.tsx @@ -11,9 +11,9 @@ export function addTimeCompareOption { let frames = prepareGraphableFields(data.series, config.theme2, timeRange); - if (frames != null) { let compareDiffMs: number[] = [0]; @@ -76,7 +75,7 @@ export const TimeSeriesPanel = ({ const needsAlignment = shouldAlignTimeCompare(frame, frames, timeRange); if (needsAlignment) { - alignTimeRangeCompareData(frame, diffMs, config.theme2.colors.text.disabled); + alignTimeRangeCompareData(frame, diffMs, config.theme2); } } }); diff --git a/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx b/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx index 8c7125e6149..9ef7e387051 100644 --- a/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx +++ b/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx @@ -64,7 +64,6 @@ export const TimeSeriesTooltip = ({ compareDiffMs, }: TimeSeriesTooltipProps) => { const xField = series.fields[0]; - let xVal = xField.values[dataIdxs[0]!]; if (compareDiffMs != null && xField.type === FieldType.time) { diff --git a/public/app/plugins/panel/timeseries/utils.ts b/public/app/plugins/panel/timeseries/utils.ts index 8b974ac38fa..2975ff8a219 100644 --- a/public/app/plugins/panel/timeseries/utils.ts +++ b/public/app/plugins/panel/timeseries/utils.ts @@ -242,20 +242,81 @@ const matchEnumColorToSeriesColor = (frames: DataFrame[], theme: GrafanaTheme2) export const setClassicPaletteIdxs = (frames: DataFrame[], theme: GrafanaTheme2, skipFieldIdx?: number) => { let seriesIndex = 0; - frames.forEach((frame) => { - frame.fields.forEach((field, fieldIdx) => { - if ( - fieldIdx !== skipFieldIdx && - (field.type === FieldType.number || field.type === FieldType.boolean || field.type === FieldType.enum) - ) { - field.state = { - ...field.state, - seriesIndex: seriesIndex++, // TODO: skip this for fields with custom renderers (e.g. Candlestick)? - }; - field.display = getDisplayProcessor({ field, theme }); + + const updateFieldDisplay = (field: Field, idx: number) => { + field.state = { ...field.state, seriesIndex: idx }; + field.display = getDisplayProcessor({ field, theme }); + }; + + const shouldProcessField = (field: Field, fieldIdx: number) => { + return ( + fieldIdx !== skipFieldIdx && + (field.type === FieldType.number || field.type === FieldType.boolean || field.type === FieldType.enum) + ); + }; + + // Pre-pass to group main frames by refId + const mainFramesByRefId = new Map(); + for (const frame of frames) { + if (!frame.meta?.timeCompare?.isTimeShiftQuery && frame.refId) { + if (!mainFramesByRefId.has(frame.refId)) { + mainFramesByRefId.set(frame.refId, []); } - }); - }); + mainFramesByRefId.get(frame.refId)!.push(frame); + } + } + + // Counter for comparison indices per baseRefId + const compareIndicesByRefId = new Map(); + + for (const frame of frames) { + const isCompareFrame = frame.meta?.timeCompare?.isTimeShiftQuery; + + if (isCompareFrame) { + const baseRefId = frame.refId?.replace('-compare', ''); + + if (baseRefId) { + // Get and increment the comparison index + let compareIndex = compareIndicesByRefId.get(baseRefId) ?? 0; + compareIndicesByRefId.set(baseRefId, compareIndex + 1); + + // Get the matching main frame using the index + const mainFrames = mainFramesByRefId.get(baseRefId); + const mainFrame = mainFrames?.[compareIndex]; + + if (mainFrame && mainFrame.fields.length === frame.fields.length) { + // Match series indices with main frame + frame.fields.forEach((field, fieldIdx) => { + if (shouldProcessField(field, fieldIdx)) { + const mainField = mainFrame.fields[fieldIdx]; + updateFieldDisplay(field, mainField.state?.seriesIndex ?? seriesIndex++); + } + }); + } else { + // Fallback + frame.fields.forEach((field, fieldIdx) => { + if (shouldProcessField(field, fieldIdx)) { + updateFieldDisplay(field, seriesIndex++); + } + }); + } + } else { + // Fallback when no baseRefId + frame.fields.forEach((field, fieldIdx) => { + if (shouldProcessField(field, fieldIdx)) { + updateFieldDisplay(field, seriesIndex++); + } + }); + } + } else { + // Main frames + frame.fields.forEach((field, fieldIdx) => { + if (shouldProcessField(field, fieldIdx)) { + updateFieldDisplay(field, seriesIndex++); + } + }); + } + } }; export function getTimezones(timezones: string[] | undefined, defaultTimezone: string): string[] {