From 7e21863982aef268173d7d91cb4578502ef1cd6c Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Mon, 23 Nov 2020 19:07:02 +0100 Subject: [PATCH] GraphNG: Init refactorings and fixes (#29275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * When comparing field config, shallowly compare custom config * Refactoring plot init and data update (WIP) * GraphNG: Fixed points mode * Fixed min & max from frame config * Fixed axis left / right auto logic * Minor tweak to cursor color * Fixed time axis now that uPlot deals in milliseconds as well * fixed ts issue * Updated test * Fixed axis placement logic again * Added new unit test for axis placement logic * Removed unused props * Fixed zoom issue due to uPlot time resolution change * Add back millisecond time tick support * Comment out GraphNG test * Fixed being able to switch legend on/off * Updated unit tests * GraphNG: Fixed hiding axis * Frame comparison: allow skipping properties * Update y-axis ranges without reinitializing uPlot * update snap * GraphNG: Fixed axis label placement and spacing issues * update snaps Co-authored-by: Torkel Ödegaard --- .../src/dataframe/frameComparisons.test.ts | 143 ++++++++++++++++++ .../src/dataframe/frameComparisons.ts | 59 +++++++- .../src/components/GraphNG/GraphNG.test.tsx | 14 +- .../src/components/GraphNG/GraphNG.tsx | 46 +++--- .../grafana-ui/src/components/uPlot/Plot.scss | 9 ++ .../grafana-ui/src/components/uPlot/Plot.tsx | 127 +++++++++------- .../grafana-ui/src/components/uPlot/config.ts | 13 -- .../uPlot/config/UPlotAxisBuilder.ts | 46 ++++-- .../uPlot/config/UPlotConfigBuilder.test.ts | 64 +++++--- .../uPlot/config/UPlotConfigBuilder.ts | 25 ++- .../uPlot/config/UPlotScaleBuilder.ts | 7 +- .../uPlot/config/UPlotSeriesBuilder.ts | 26 ++-- .../grafana-ui/src/components/uPlot/hooks.ts | 19 ++- .../components/uPlot/plugins/ZoomPlugin.tsx | 2 +- .../grafana-ui/src/components/uPlot/types.ts | 8 +- .../grafana-ui/src/components/uPlot/utils.ts | 61 +------- public/app/plugins/panel/graph3/module.tsx | 1 - 17 files changed, 445 insertions(+), 225 deletions(-) diff --git a/packages/grafana-data/src/dataframe/frameComparisons.test.ts b/packages/grafana-data/src/dataframe/frameComparisons.test.ts index de320fca43c..e88a9d5707d 100644 --- a/packages/grafana-data/src/dataframe/frameComparisons.test.ts +++ b/packages/grafana-data/src/dataframe/frameComparisons.test.ts @@ -92,4 +92,147 @@ describe('test comparisons', () => { }) ).toBeFalsy(); }); + + it('should skip provided properties', () => { + expect( + compareDataFrameStructures( + { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + ...field1.config, + }, + }, + ], + }, + { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + ...field1.config, + unit: 'rpm', + }, + }, + ], + }, + ['unit'] + ) + ).toBeTruthy(); + }); + + describe('custom config comparison', () => { + it('handles custom config shallow equality', () => { + const a = { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + custom: { + a: 1, + b: 'test', + }, + }, + }, + ], + }; + + const b = { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + custom: { + a: 1, + b: 'test', + }, + }, + }, + ], + }; + + expect(compareDataFrameStructures(a, b)).toBeTruthy(); + }); + + it('handles custom config shallow inequality', () => { + const a = { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + custom: { + a: 1, + }, + }, + }, + ], + }; + + const b = { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + custom: { + a: 2, + }, + }, + }, + ], + }; + + expect(compareDataFrameStructures(a, b)).toBeFalsy(); + }); + + it('does not compare deeply', () => { + const a = { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + custom: { + a: { + b: 1, + }, + }, + }, + }, + ], + }; + + const b = { + ...frameB, + fields: [ + field0, + { + ...field1, + config: { + custom: { + a: { + b: 1, + }, + }, + }, + }, + ], + }; + + expect(compareDataFrameStructures(a, b)).toBeFalsy(); + }); + }); }); diff --git a/packages/grafana-data/src/dataframe/frameComparisons.ts b/packages/grafana-data/src/dataframe/frameComparisons.ts index fb5fabda563..47ef9ef5b30 100644 --- a/packages/grafana-data/src/dataframe/frameComparisons.ts +++ b/packages/grafana-data/src/dataframe/frameComparisons.ts @@ -14,13 +14,14 @@ import { DataFrame } from '../types/dataFrame'; * * @beta */ -export function compareDataFrameStructures(a: DataFrame, b: DataFrame): boolean { +export function compareDataFrameStructures(a: DataFrame, b: DataFrame, skipProperties?: string[]): boolean { if (a === b) { return true; } if (a?.fields?.length !== b?.fields?.length) { return false; } + for (let i = 0; i < a.fields.length; i++) { const fA = a.fields[i]; const fB = b.fields[i]; @@ -30,14 +31,34 @@ export function compareDataFrameStructures(a: DataFrame, b: DataFrame): boolean const cfgA = fA.config as any; const cfgB = fB.config as any; - const keys = Object.keys(cfgA); - if (keys.length !== Object.keys(cfgB).length) { + let aKeys = Object.keys(cfgA); + let bKeys = Object.keys(cfgB); + + if (skipProperties) { + aKeys = aKeys.filter(k => skipProperties.indexOf(k) < 0); + bKeys = aKeys.filter(k => skipProperties.indexOf(k) < 0); + } + if (aKeys.length !== bKeys.length) { return false; } - for (const key of keys) { + + for (const key of aKeys) { + if (skipProperties && skipProperties.indexOf(key) > -1) { + continue; + } + if (!cfgB.hasOwnProperty(key)) { return false; } + + if (key === 'custom') { + if (!shallowCompare(cfgA[key], cfgB[key])) { + return false; + } else { + continue; + } + } + if (cfgA[key] !== cfgB[key]) { return false; } @@ -65,3 +86,33 @@ export function compareArrayValues(a: T[], b: T[], cmp: (a: T, b: T) => boole } return true; } + +/** + * Checks if two objects are equal shallowly + * + * @beta + */ +export function shallowCompare(a: T, b: T, cmp?: (valA: any, valB: any) => boolean) { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (a === b) { + return true; + } + + if (aKeys.length !== bKeys.length) { + return false; + } + + for (let key of aKeys) { + if (cmp) { + //@ts-ignore + return cmp(a[key], b[key]); + } + //@ts-ignore + if (a[key] !== b[key]) { + return false; + } + } + + return true; +} diff --git a/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx b/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx index bd88405639a..a53d5b02a16 100644 --- a/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx +++ b/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx @@ -84,20 +84,12 @@ describe('GraphNG', () => { describe('config update', () => { it('should skip plot intialization for width and height equal 0', () => { const { data, timeRange } = mockData(); - const onPlotInitSpy = jest.fn(); - render( - + const { queryAllByTestId } = render( + ); - expect(onPlotInitSpy).not.toBeCalled(); + expect(queryAllByTestId('uplot-main-div')).toHaveLength(1); }); // it('reinitializes plot when number of series change', () => { diff --git a/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx b/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx index b2ece8e9b69..0fcd946958a 100755 --- a/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx +++ b/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx @@ -1,5 +1,6 @@ -import React, { useMemo, useRef } from 'react'; +import React, { useCallback, useMemo, useRef } from 'react'; import { + compareDataFrameStructures, DataFrame, FieldConfig, FieldType, @@ -11,12 +12,13 @@ import { import { mergeTimeSeriesData } from './utils'; import { UPlotChart } from '../uPlot/Plot'; import { PlotProps } from '../uPlot/types'; -import { AxisPlacement, getUPlotSideFromAxis, GraphFieldConfig, GraphMode, PointMode } from '../uPlot/config'; +import { AxisPlacement, GraphFieldConfig, GraphMode, PointMode } from '../uPlot/config'; import { useTheme } from '../../themes'; import { VizLayout } from '../VizLayout/VizLayout'; import { LegendDisplayMode, LegendItem, LegendOptions } from '../Legend/Legend'; import { GraphLegend } from '../Graph/GraphLegend'; import { UPlotConfigBuilder } from '../uPlot/config/UPlotConfigBuilder'; +import { useRevision } from '../uPlot/hooks'; const defaultFormatter = (v: any) => (v == null ? '-' : v.toFixed(1)); @@ -41,10 +43,7 @@ export const GraphNG: React.FC = ({ timeZone, ...plotProps }) => { - const theme = useTheme(); const alignedFrameWithGapTest = useMemo(() => mergeTimeSeriesData(data), [data]); - const legendItemsRef = useRef([]); - const hasLegend = legend && legend.displayMode !== LegendDisplayMode.Hidden; if (alignedFrameWithGapTest == null) { return ( @@ -54,7 +53,15 @@ export const GraphNG: React.FC = ({ ); } + const theme = useTheme(); + const legendItemsRef = useRef([]); + const hasLegend = useRef(legend && legend.displayMode !== LegendDisplayMode.Hidden); const alignedFrame = alignedFrameWithGapTest.frame; + const compareFrames = useCallback( + (a: DataFrame, b: DataFrame) => compareDataFrameStructures(a, b, ['min', 'max']), + [] + ); + const configRev = useRevision(alignedFrame, compareFrames); const configBuilder = useMemo(() => { const builder = new UPlotConfigBuilder(); @@ -76,15 +83,13 @@ export const GraphNG: React.FC = ({ builder.addAxis({ scaleKey: 'x', isTime: true, - side: getUPlotSideFromAxis(AxisPlacement.Bottom), + placement: AxisPlacement.Bottom, timeZone, theme, }); let seriesIdx = 0; const legendItems: LegendItem[] = []; - let hasLeftAxis = false; - let hasYAxis = false; for (let i = 0; i < alignedFrame.fields.length; i++) { const field = alignedFrame.fields[i]; @@ -97,23 +102,17 @@ export const GraphNG: React.FC = ({ const fmt = field.display ?? defaultFormatter; const scale = config.unit || '__fixed'; - const side = customConfig.axisPlacement ?? (hasLeftAxis ? AxisPlacement.Right : AxisPlacement.Left); + const isNewScale = !builder.hasScale(scale); - if (!builder.hasScale(scale) && customConfig.axisPlacement !== AxisPlacement.Hidden) { - if (side === AxisPlacement.Left) { - hasLeftAxis = true; - } - - builder.addScale({ scaleKey: scale }); + if (isNewScale && customConfig.axisPlacement !== AxisPlacement.Hidden) { + builder.addScale({ scaleKey: scale, min: field.config.min, max: field.config.max }); builder.addAxis({ scaleKey: scale, label: customConfig.axisLabel, - side: getUPlotSideFromAxis(side), - grid: !hasYAxis, + placement: customConfig.axisPlacement ?? AxisPlacement.Auto, formatValue: v => formattedValueToString(fmt(v)), theme, }); - hasYAxis = true; } // need to update field state here because we use a transform to merge framesP @@ -121,13 +120,14 @@ export const GraphNG: React.FC = ({ const colorMode = getFieldColorModeForField(field); const seriesColor = colorMode.getCalculator(field, theme)(0, 0); + const pointsMode = customConfig.mode === GraphMode.Points ? PointMode.Always : customConfig.points; builder.addSeries({ scaleKey: scale, line: (customConfig.mode ?? GraphMode.Line) === GraphMode.Line, lineColor: seriesColor, lineWidth: customConfig.lineWidth, - points: customConfig.points !== PointMode.Never, + points: pointsMode, pointSize: customConfig.pointRadius, pointColor: seriesColor, fill: customConfig.fillAlpha !== undefined, @@ -135,11 +135,13 @@ export const GraphNG: React.FC = ({ fillColor: seriesColor, }); - if (hasLegend) { + if (hasLegend.current) { + const axisPlacement = builder.getAxisPlacement(scale); + legendItems.push({ color: seriesColor, label: getFieldDisplayName(field, alignedFrame), - yAxis: side === AxisPlacement.Right ? 3 : 1, + yAxis: axisPlacement === AxisPlacement.Left ? 1 : 2, }); } @@ -148,7 +150,7 @@ export const GraphNG: React.FC = ({ legendItemsRef.current = legendItems; return builder; - }, [alignedFrameWithGapTest, hasLegend]); + }, [configRev]); let legendElement: React.ReactElement | undefined; diff --git a/packages/grafana-ui/src/components/uPlot/Plot.scss b/packages/grafana-ui/src/components/uPlot/Plot.scss index 50f282653b0..cce7ccc2bd1 100644 --- a/packages/grafana-ui/src/components/uPlot/Plot.scss +++ b/packages/grafana-ui/src/components/uPlot/Plot.scss @@ -8,3 +8,12 @@ .u-select { background: rgba(120, 120, 130, 0.2); } + +.u-cursor-x { + border-right: 1px dashed rgba(120, 120, 130, 0.5); +} + +.u-cursor-y { + width: 100%; + border-bottom: 1px dashed rgba(120, 120, 130, 0.5); +} diff --git a/packages/grafana-ui/src/components/uPlot/Plot.tsx b/packages/grafana-ui/src/components/uPlot/Plot.tsx index 48b9c0ef2f0..40749114d02 100755 --- a/packages/grafana-ui/src/components/uPlot/Plot.tsx +++ b/packages/grafana-ui/src/components/uPlot/Plot.tsx @@ -1,10 +1,13 @@ import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; -import { usePrevious } from 'react-use'; -import uPlot, { Options, AlignedData, AlignedDataWithGapTest } from 'uplot'; +import uPlot, { AlignedData, AlignedDataWithGapTest } from 'uplot'; import { buildPlotContext, PlotContext } from './context'; -import { pluginLog, shouldInitialisePlot } from './utils'; +import { pluginLog } from './utils'; import { usePlotConfig } from './hooks'; import { PlotProps } from './types'; +import { usePrevious } from 'react-use'; +import { DataFrame, FieldType } from '@grafana/data'; +import isNumber from 'lodash/isNumber'; +import { UPlotConfigBuilder } from './config/UPlotConfigBuilder'; // uPlot abstraction responsible for plot initialisation, setup and refresh // Receives a data frame that is x-axis aligned, as of https://github.com/leeoniya/uPlot/tree/master/docs#data-format @@ -13,11 +16,24 @@ export const UPlotChart: React.FC = props => { const canvasRef = useRef(null); const [plotInstance, setPlotInstance] = useState(); const plotData = useRef(); + const previousConfig = usePrevious(props.config); // uPlot config API const { currentConfig, registerPlugin } = usePlotConfig(props.width, props.height, props.timeZone, props.config); - const prevConfig = usePrevious(currentConfig); + const initializePlot = useCallback(() => { + if (!currentConfig || !plotData) { + return; + } + if (!canvasRef.current) { + throw new Error('Missing Canvas component as a child of the plot.'); + } + + pluginLog('UPlotChart: init uPlot', false, 'initialized with', plotData.current, currentConfig); + const instance = new uPlot(currentConfig, plotData.current, canvasRef.current); + + setPlotInstance(instance); + }, [setPlotInstance, currentConfig]); const getPlotInstance = useCallback(() => { if (!plotInstance) { @@ -27,22 +43,21 @@ export const UPlotChart: React.FC = props => { return plotInstance; }, [plotInstance]); - // Callback executed when there was no change in plot config - const updateData = useCallback(() => { - if (!plotInstance || !plotData.current) { - return; + useLayoutEffect(() => { + plotData.current = { + data: props.data.frame.fields.map(f => f.values.toArray()) as AlignedData, + isGap: props.data.isGap, + }; + + if (plotInstance && previousConfig === props.config) { + updateData(props.data.frame, props.config, plotInstance, plotData.current.data); } - pluginLog('uPlot core', false, 'updating plot data(throttled log!)', plotData.current); + }, [props.data, props.config]); - // If config hasn't changed just update uPlot's data - plotInstance.setData(plotData.current); + useLayoutEffect(() => { + initializePlot(); + }, [currentConfig]); - if (props.onDataUpdate) { - props.onDataUpdate(plotData.current.data!); - } - }, [plotInstance, props.onDataUpdate]); - - // Destroys previous plot instance when plot re-initialised useEffect(() => { const currentInstance = plotInstance; return () => { @@ -50,42 +65,6 @@ export const UPlotChart: React.FC = props => { }; }, [plotInstance]); - useLayoutEffect(() => { - plotData.current = { - data: props.data.frame.fields.map(f => f.values.toArray()) as AlignedData, - isGap: props.data.isGap, - }; - }, [props.data]); - - // Decides if plot should update data or re-initialise - useLayoutEffect(() => { - // Make sure everything is ready before proceeding - if (!currentConfig || !plotData.current) { - return; - } - - // Do nothing if there is data vs series config mismatch. This may happen when the data was updated and made this - // effect fire before the config update triggered the effect. - if (currentConfig.series.length !== plotData.current.data!.length) { - return; - } - - if (shouldInitialisePlot(prevConfig, currentConfig)) { - if (!canvasRef.current) { - throw new Error('Missing Canvas component as a child of the plot.'); - } - const instance = initPlot(plotData.current.data!, currentConfig, canvasRef.current); - - if (props.onPlotInit) { - props.onPlotInit(); - } - - setPlotInstance(instance); - } else { - updateData(); - } - }, [currentConfig, updateData, setPlotInstance, props.onPlotInit]); - // When size props changed update plot size synchronously useLayoutEffect(() => { if (plotInstance) { @@ -103,14 +82,46 @@ export const UPlotChart: React.FC = props => { return ( -
+
{props.children} ); }; -// Main function initialising uPlot. If final config is not settled it will do nothing -function initPlot(data: AlignedData, config: Options, ref: HTMLDivElement) { - pluginLog('uPlot core', false, 'initialized with', data, config); - return new uPlot(config, data, ref); +// Callback executed when there was no change in plot config +function updateData(frame: DataFrame, config: UPlotConfigBuilder, plotInstance?: uPlot, data?: AlignedData | null) { + if (!plotInstance || !data) { + return; + } + pluginLog('uPlot core', false, 'updating plot data(throttled log!)', data); + updateScales(frame, config, plotInstance); + // If config hasn't changed just update uPlot's data + plotInstance.setData(data); +} + +function updateScales(frame: DataFrame, config: UPlotConfigBuilder, plotInstance: uPlot) { + let yRange: [number, number] | undefined = undefined; + + for (let i = 0; i < frame.fields.length; i++) { + if (frame.fields[i].type !== FieldType.number) { + continue; + } + if (isNumber(frame.fields[i].config.min) && isNumber(frame.fields[i].config.max)) { + yRange = [frame.fields[i].config.min!, frame.fields[i].config.max!]; + break; + } + } + + const scalesConfig = config.getConfig().scales; + + if (scalesConfig && yRange) { + for (const scale in scalesConfig) { + if (!scalesConfig.hasOwnProperty(scale)) { + continue; + } + if (scale !== 'x') { + plotInstance.setScale(scale, { min: yRange[0], max: yRange[1] }); + } + } + } } diff --git a/packages/grafana-ui/src/components/uPlot/config.ts b/packages/grafana-ui/src/components/uPlot/config.ts index 977fff04e07..00ee5e2850c 100644 --- a/packages/grafana-ui/src/components/uPlot/config.ts +++ b/packages/grafana-ui/src/components/uPlot/config.ts @@ -9,19 +9,6 @@ export enum AxisPlacement { Hidden = 'hidden', } -export function getUPlotSideFromAxis(axis: AxisPlacement) { - switch (axis) { - case AxisPlacement.Top: - return 0; - case AxisPlacement.Right: - return 1; - case AxisPlacement.Bottom: - return 2; - case AxisPlacement.Left: - } - return 3; // default everythign to the left -} - export enum PointMode { Auto = 'auto', // will show points when the density is low or line is hidden Always = 'always', diff --git a/packages/grafana-ui/src/components/uPlot/config/UPlotAxisBuilder.ts b/packages/grafana-ui/src/components/uPlot/config/UPlotAxisBuilder.ts index d661a8c4390..8386284503f 100644 --- a/packages/grafana-ui/src/components/uPlot/config/UPlotAxisBuilder.ts +++ b/packages/grafana-ui/src/components/uPlot/config/UPlotAxisBuilder.ts @@ -2,15 +2,15 @@ import { dateTimeFormat, GrafanaTheme, systemDateFormats, TimeZone } from '@graf import uPlot, { Axis } from 'uplot'; import { PlotConfigBuilder } from '../types'; import { measureText } from '../../../utils/measureText'; +import { AxisPlacement } from '../config'; export interface AxisProps { scaleKey: string; theme: GrafanaTheme; label?: string; - stroke?: string; show?: boolean; size?: number; - side?: Axis.Side; + placement?: AxisPlacement; grid?: boolean; formatValue?: (v: any) => string; values?: any; @@ -24,7 +24,7 @@ export class UPlotAxisBuilder extends PlotConfigBuilder { scaleKey, label, show = true, - side = 3, + placement = AxisPlacement.Auto, grid = true, formatValue, values, @@ -32,16 +32,15 @@ export class UPlotAxisBuilder extends PlotConfigBuilder { timeZone, theme, } = this.props; - const stroke = this.props.stroke || theme.colors.text; const gridColor = theme.isDark ? theme.palette.gray25 : theme.palette.gray90; let config: Axis = { scale: scaleKey, - label, show, - stroke, - side, - font: '12px Roboto', + stroke: theme.colors.text, + side: getUPlotSideFromAxis(placement), + font: `12px 'Roboto'`, + labelFont: `12px 'Roboto'`, size: calculateAxisSize, grid: { show: grid, @@ -57,6 +56,11 @@ export class UPlotAxisBuilder extends PlotConfigBuilder { space: calculateSpace, }; + if (label !== undefined && label !== null && label.length > 0) { + config.label = label; + config.labelSize = 18; + } + if (values) { config.values = values; } else if (isTime) { @@ -102,20 +106,24 @@ function calculateAxisSize(self: uPlot, values: string[], axisIdx: number) { } } - return measureText(maxLength, 12).width - 8; + let axisWidth = measureText(maxLength, 12).width + 18; + return axisWidth; } /** Format time axis ticks */ function formatTime(self: uPlot, splits: number[], axisIdx: number, foundSpace: number, foundIncr: number): string[] { const timeZone = (self.axes[axisIdx] as any).timeZone; const scale = self.scales.x; - const range = (scale?.max ?? 0) - (scale?.min ?? 0); + const range = (scale?.max ?? 0) - (scale?.min ?? 0) / 1000; const oneDay = 86400; const oneYear = 31536000; + foundIncr = foundIncr / 1000; let format = systemDateFormats.interval.minute; - if (foundIncr <= 45) { + if (foundIncr < 1) { + format = systemDateFormats.interval.second.replace('ss', 'ss.SS'); + } else if (foundIncr <= 45) { format = systemDateFormats.interval.second; } else if (foundIncr <= 7200 || range <= oneDay) { format = systemDateFormats.interval.minute; @@ -127,5 +135,19 @@ function formatTime(self: uPlot, splits: number[], axisIdx: number, foundSpace: format = systemDateFormats.interval.month; } - return splits.map(v => dateTimeFormat(v * 1000, { format, timeZone })); + return splits.map(v => dateTimeFormat(v, { format, timeZone })); +} + +export function getUPlotSideFromAxis(axis: AxisPlacement) { + switch (axis) { + case AxisPlacement.Top: + return 0; + case AxisPlacement.Right: + return 1; + case AxisPlacement.Bottom: + return 2; + case AxisPlacement.Left: + } + + return 3; // default everythign to the left } diff --git a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.test.ts b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.test.ts index 782e1d9d9e9..a02d84d6a3f 100644 --- a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.test.ts +++ b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.test.ts @@ -3,6 +3,7 @@ import { UPlotConfigBuilder } from './UPlotConfigBuilder'; import { GrafanaTheme } from '@grafana/data'; import { expect } from '../../../../../../public/test/lib/common'; +import { AxisPlacement, PointMode } from '../config'; describe('UPlotConfigBuilder', () => { describe('scales config', () => { @@ -17,21 +18,23 @@ describe('UPlotConfigBuilder', () => { isTime: false, }); expect(builder.getConfig()).toMatchInlineSnapshot(` - Object { - "axes": Array [], - "scales": Object { - "scale-x": Object { - "time": true, + Object { + "axes": Array [], + "scales": Object { + "scale-x": Object { + "range": undefined, + "time": true, + }, + "scale-y": Object { + "range": undefined, + "time": false, + }, }, - "scale-y": Object { - "time": false, - }, - }, - "series": Array [ - Object {}, - ], - } - `); + "series": Array [ + Object {}, + ], + } + `); }); it('prevents duplicate scales', () => { @@ -55,14 +58,13 @@ describe('UPlotConfigBuilder', () => { scaleKey: 'scale-x', label: 'test label', timeZone: 'browser', - side: 2, + placement: AxisPlacement.Bottom, isTime: false, formatValue: () => 'test value', grid: false, show: true, size: 1, - stroke: '#ff0000', - theme: { isDark: true, palette: { gray25: '#ffffff' } } as GrafanaTheme, + theme: { isDark: true, palette: { gray25: '#ffffff' }, colors: { text: 'gray' } } as GrafanaTheme, values: [], }); @@ -70,19 +72,21 @@ describe('UPlotConfigBuilder', () => { Object { "axes": Array [ Object { - "font": "12px Roboto", + "font": "12px 'Roboto'", "grid": Object { "show": false, "stroke": "#ffffff", "width": 1, }, "label": "test label", + "labelFont": "12px 'Roboto'", + "labelSize": 18, "scale": "scale-x", "show": true, "side": 2, "size": [Function], "space": [Function], - "stroke": "#ff0000", + "stroke": "gray", "ticks": Object { "show": true, "stroke": "#ffffff", @@ -99,6 +103,24 @@ describe('UPlotConfigBuilder', () => { } `); }); + + it('Handles auto axis placement', () => { + const builder = new UPlotConfigBuilder(); + builder.addAxis({ + scaleKey: 'y1', + placement: AxisPlacement.Auto, + theme: { isDark: true, palette: { gray25: '#ffffff' } } as GrafanaTheme, + }); + builder.addAxis({ + scaleKey: 'y2', + placement: AxisPlacement.Auto, + theme: { isDark: true, palette: { gray25: '#ffffff' } } as GrafanaTheme, + }); + + expect(builder.getAxisPlacement('y1')).toBe(AxisPlacement.Left); + expect(builder.getAxisPlacement('y2')).toBe(AxisPlacement.Right); + }); + it('allows series configuration', () => { const builder = new UPlotConfigBuilder(); builder.addSeries({ @@ -106,7 +128,7 @@ describe('UPlotConfigBuilder', () => { fill: true, fillColor: '#ff0000', fillOpacity: 0.5, - points: true, + points: PointMode.Auto, pointSize: 5, pointColor: '#00ff00', line: true, @@ -123,7 +145,7 @@ describe('UPlotConfigBuilder', () => { Object { "fill": "rgba(255, 0, 0, 0.5)", "points": Object { - "show": true, + "fill": "#00ff00", "size": 5, "stroke": "#00ff00", }, diff --git a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts index 6c871b47888..e12232bfc5f 100644 --- a/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts +++ b/packages/grafana-ui/src/components/uPlot/config/UPlotConfigBuilder.ts @@ -2,15 +2,34 @@ import { PlotSeriesConfig } from '../types'; import { ScaleProps, UPlotScaleBuilder } from './UPlotScaleBuilder'; import { SeriesProps, UPlotSeriesBuilder } from './UPlotSeriesBuilder'; import { AxisProps, UPlotAxisBuilder } from './UPlotAxisBuilder'; +import { AxisPlacement } from '../config'; export class UPlotConfigBuilder { private series: UPlotSeriesBuilder[] = []; - private axes: UPlotAxisBuilder[] = []; + private axes: Record = {}; private scales: UPlotScaleBuilder[] = []; private registeredScales: string[] = []; + hasLeftAxis = false; + addAxis(props: AxisProps) { - this.axes.push(new UPlotAxisBuilder(props)); + props.placement = props.placement ?? AxisPlacement.Auto; + + // Handle auto placement logic + if (props.placement === AxisPlacement.Auto) { + props.placement = this.hasLeftAxis ? AxisPlacement.Right : AxisPlacement.Left; + } + + if (props.placement === AxisPlacement.Left) { + this.hasLeftAxis = true; + } + + this.axes[props.scaleKey] = new UPlotAxisBuilder(props); + } + + getAxisPlacement(scaleKey: string): AxisPlacement { + const axis = this.axes[scaleKey]; + return axis?.props.placement! ?? AxisPlacement.Left; } addSeries(props: SeriesProps) { @@ -28,7 +47,7 @@ export class UPlotConfigBuilder { getConfig() { const config: PlotSeriesConfig = { series: [{}] }; - config.axes = this.axes.map(a => a.getConfig()); + config.axes = Object.values(this.axes).map(a => a.getConfig()); config.series = [...config.series, ...this.series.map(s => s.getConfig())]; config.scales = this.scales.reduce((acc, s) => { return { ...acc, ...s.getConfig() }; diff --git a/packages/grafana-ui/src/components/uPlot/config/UPlotScaleBuilder.ts b/packages/grafana-ui/src/components/uPlot/config/UPlotScaleBuilder.ts index 2eaa2600446..796f7c432aa 100644 --- a/packages/grafana-ui/src/components/uPlot/config/UPlotScaleBuilder.ts +++ b/packages/grafana-ui/src/components/uPlot/config/UPlotScaleBuilder.ts @@ -1,17 +1,22 @@ +import isNumber from 'lodash/isNumber'; import { Scale } from 'uplot'; import { PlotConfigBuilder } from '../types'; export interface ScaleProps { scaleKey: string; isTime?: boolean; + min?: number | null; + max?: number | null; } export class UPlotScaleBuilder extends PlotConfigBuilder { getConfig() { - const { isTime, scaleKey } = this.props; + const { isTime, scaleKey, min, max } = this.props; + const range = isNumber(min) && isNumber(max) ? [min, max] : undefined; return { [scaleKey]: { time: !!isTime, + range, }, }; } diff --git a/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts b/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts index 10c7ef2d8da..09efd4ac57a 100755 --- a/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts +++ b/packages/grafana-ui/src/components/uPlot/config/UPlotSeriesBuilder.ts @@ -1,5 +1,6 @@ import tinycolor from 'tinycolor2'; import { Series } from 'uplot'; +import { PointMode } from '../config'; import { PlotConfigBuilder } from '../types'; export interface SeriesProps { @@ -7,7 +8,7 @@ export interface SeriesProps { line?: boolean; lineColor?: string; lineWidth?: number; - points?: boolean; + points?: PointMode; pointSize?: number; pointColor?: string; fill?: boolean; @@ -26,15 +27,20 @@ export class UPlotSeriesBuilder extends PlotConfigBuilder { } : {}; - const pointsConfig = points - ? { - points: { - show: true, - size: pointSize, - stroke: pointColor, - }, - } - : {}; + const pointsConfig: Partial = { + points: { + stroke: pointColor, + fill: pointColor, + size: pointSize, + }, + }; + + // we cannot set points.show property above (even to undefined) as that will clear uPlot's default auto behavior + if (points === PointMode.Never) { + pointsConfig.points!.show = false; + } else if (points === PointMode.Always) { + pointsConfig.points!.show = true; + } const areaConfig = fillOpacity !== undefined diff --git a/packages/grafana-ui/src/components/uPlot/hooks.ts b/packages/grafana-ui/src/components/uPlot/hooks.ts index 0568f5ffbfb..a4df275efbd 100644 --- a/packages/grafana-ui/src/components/uPlot/hooks.ts +++ b/packages/grafana-ui/src/components/uPlot/hooks.ts @@ -5,6 +5,7 @@ import uPlot, { Options } from 'uplot'; import { getTimeZoneInfo, TimeZone } from '@grafana/data'; import { usePlotPluginContext } from './context'; import { UPlotConfigBuilder } from './config/UPlotConfigBuilder'; +import usePrevious from 'react-use/lib/usePrevious'; export const usePlotPlugins = () => { /** @@ -104,6 +105,7 @@ export const DEFAULT_PLOT_CONFIG = { hooks: {}, }; +//pass plain confsig object,memoize! export const usePlotConfig = (width: number, height: number, timeZone: TimeZone, configBuilder: UPlotConfigBuilder) => { const { arePluginsReady, plugins, registerPlugin } = usePlotPlugins(); const [currentConfig, setCurrentConfig] = useState(); @@ -124,7 +126,6 @@ export const usePlotConfig = (width: number, height: number, timeZone: TimeZone, if (!arePluginsReady) { return; } - setCurrentConfig({ ...DEFAULT_PLOT_CONFIG, width, @@ -135,7 +136,7 @@ export const usePlotConfig = (width: number, height: number, timeZone: TimeZone, tzDate, ...configBuilder.getConfig(), }); - }, [arePluginsReady, plugins, width, height, configBuilder]); + }, [arePluginsReady, plugins, width, height, tzDate, configBuilder]); return { registerPlugin, @@ -171,3 +172,17 @@ export const useRefreshAfterGraphRendered = (pluginId: string) => { return renderToken; }; + +export function useRevision(dep: T, cmp: (prev: T, next: T) => boolean) { + const [rev, setRev] = useState(0); + const prevDep = usePrevious(dep); + + useEffect(() => { + const hasConfigChanged = prevDep ? !cmp(prevDep, dep) : true; + if (hasConfigChanged) { + setRev(r => r + 1); + } + }, [dep]); + + return rev; +} diff --git a/packages/grafana-ui/src/components/uPlot/plugins/ZoomPlugin.tsx b/packages/grafana-ui/src/components/uPlot/plugins/ZoomPlugin.tsx index 3771e1a6813..b477af506c3 100644 --- a/packages/grafana-ui/src/components/uPlot/plugins/ZoomPlugin.tsx +++ b/packages/grafana-ui/src/components/uPlot/plugins/ZoomPlugin.tsx @@ -17,7 +17,7 @@ export const ZoomPlugin: React.FC = ({ onZoom }) => { if (selection.bbox.width < MIN_ZOOM_DIST) { return; } - onZoom({ from: selection.min * 1000, to: selection.max * 1000 }); + onZoom({ from: selection.min, to: selection.max }); }} /> ); diff --git a/packages/grafana-ui/src/components/uPlot/types.ts b/packages/grafana-ui/src/components/uPlot/types.ts index a0224104184..57ea1c8a85c 100755 --- a/packages/grafana-ui/src/components/uPlot/types.ts +++ b/packages/grafana-ui/src/components/uPlot/types.ts @@ -1,5 +1,5 @@ import React from 'react'; -import uPlot, { Options, AlignedData, Series, Hooks } from 'uplot'; +import uPlot, { Options, Series, Hooks } from 'uplot'; import { DataFrame, TimeRange, TimeZone } from '@grafana/data'; import { UPlotConfigBuilder } from './config/UPlotConfigBuilder'; @@ -23,14 +23,10 @@ export interface PlotProps { height: number; config: UPlotConfigBuilder; children?: React.ReactElement[]; - /** Callback performed when uPlot data is updated */ - onDataUpdate?: (data: AlignedData) => {}; - /** Callback performed when uPlot is (re)initialized */ - onPlotInit?: () => {}; } export abstract class PlotConfigBuilder { - constructor(protected props: P) {} + constructor(public props: P) {} abstract getConfig(): T; } diff --git a/packages/grafana-ui/src/components/uPlot/utils.ts b/packages/grafana-ui/src/components/uPlot/utils.ts index 0f1c9ff8a81..4d5bccf5dd7 100755 --- a/packages/grafana-ui/src/components/uPlot/utils.ts +++ b/packages/grafana-ui/src/components/uPlot/utils.ts @@ -1,6 +1,4 @@ import throttle from 'lodash/throttle'; -import isEqual from 'lodash/isEqual'; -import omit from 'lodash/omit'; import { rangeUtil, RawTimeRange } from '@grafana/data'; import { Options } from 'uplot'; import { PlotPlugin, PlotProps } from './types'; @@ -38,7 +36,7 @@ export const buildPlotConfig = (props: PlotProps, plugins: Record { +export const isPlottingTime = (config: Options) => { let isTimeSeries = false; if (!config.scales) { @@ -56,63 +54,6 @@ const isPlottingTime = (config: Options) => { return isTimeSeries; }; -/** - * Based on two config objects indicates whether or not uPlot needs reinitialisation - * This COULD be done based on data frames, but keeping it this way for now as a simplification - */ -export const shouldInitialisePlot = (prevConfig?: Options, config?: Options) => { - if (!config && !prevConfig) { - return false; - } - - if (config) { - if (config.width === 0 || config.height === 0) { - return false; - } - if (!prevConfig) { - return true; - } - } - - if (isPlottingTime(config!) && prevConfig!.tzDate !== config!.tzDate) { - return true; - } - - // reinitialise when number of series, scales or axes changes - if ( - prevConfig!.series?.length !== config!.series?.length || - prevConfig!.axes?.length !== config!.axes?.length || - prevConfig!.scales?.length !== config!.scales?.length - ) { - return true; - } - - let idx = 0; - - // reinitialise when any of the series config changes - if (config!.series && prevConfig!.series) { - for (const series of config!.series) { - if (!isEqual(series, prevConfig!.series[idx])) { - return true; - } - idx++; - } - } - - if (config!.axes && prevConfig!.axes) { - idx = 0; - for (const axis of config!.axes) { - // Comparing axes config, skipping values property as it changes across config builds - probably need to be more clever - if (!isEqual(omit(axis, 'values', 'size'), omit(prevConfig!.axes[idx], 'values', 'size'))) { - return true; - } - idx++; - } - } - - return false; -}; - // Dev helpers export const throttledLog = throttle((...t: any[]) => { console.log(...t); diff --git a/public/app/plugins/panel/graph3/module.tsx b/public/app/plugins/panel/graph3/module.tsx index 85093b110f6..32a4dc8bc9b 100644 --- a/public/app/plugins/panel/graph3/module.tsx +++ b/public/app/plugins/panel/graph3/module.tsx @@ -67,7 +67,6 @@ export const plugin = new PanelPlugin(GraphPanel) .addRadio({ path: 'points', name: 'Points', - description: 'NOTE: auto vs always are currently the same', defaultValue: graphFieldOptions.points[0].value, settings: { options: graphFieldOptions.points,