From fa1489e35c3a8055bdac079196e783ba283f591f Mon Sep 17 00:00:00 2001 From: Boyko Date: Tue, 26 Nov 2019 21:48:53 +0200 Subject: [PATCH] React UI: Improve graph legend hover performance (#6367) * improve hover performance Signed-off-by: blalov * increase graph tests coverage Signed-off-by: blalov * wrap plotSetAndDraw into requestAnimationFrame to achieve smooth hover effect Signed-off-by: Boyko Lalov * unit tests Signed-off-by: Boyko Lalov * add destroy plot method to types Signed-off-by: blalov * make chart undefined by default Signed-off-by: Boyko Lalov * make destroy plot test more meaningful Signed-off-by: Boyko Lalov * remove chart.destroy extra check Signed-off-by: Boyko Lalov --- web/ui/react-app/src/Graph.test.tsx | 399 ++++++++++++++++++++--- web/ui/react-app/src/Graph.tsx | 116 ++++--- web/ui/react-app/src/GraphTabContent.tsx | 28 ++ web/ui/react-app/src/Panel.test.tsx | 6 +- web/ui/react-app/src/Panel.tsx | 12 +- web/ui/react-app/src/types/index.d.ts | 4 + 6 files changed, 462 insertions(+), 103 deletions(-) create mode 100644 web/ui/react-app/src/GraphTabContent.tsx diff --git a/web/ui/react-app/src/Graph.test.tsx b/web/ui/react-app/src/Graph.test.tsx index ef410cf59..8f8166c83 100644 --- a/web/ui/react-app/src/Graph.test.tsx +++ b/web/ui/react-app/src/Graph.test.tsx @@ -1,49 +1,10 @@ import * as React from 'react'; -import { shallow } from 'enzyme'; +import $ from 'jquery'; +import { shallow, mount } from 'enzyme'; import Graph from './Graph'; -import { Alert } from 'reactstrap'; import ReactResizeDetector from 'react-resize-detector'; describe('Graph', () => { - it('renders an alert if data result type is different than "matrix"', () => { - const props: any = { - data: { resultType: 'invalid', result: [] }, - stacked: false, - queryParams: { - startTime: 1572100210000, - endTime: 1572100217898, - resolution: 10, - }, - color: 'danger', - children: `Query result is of wrong type '`, - }; - const graph = shallow(); - const alert = graph.find(Alert); - expect(alert.prop('color')).toEqual(props.color); - expect(alert.childAt(0).text()).toEqual(props.children); - }); - - it('renders an alert if data result empty', () => { - const props: any = { - data: { - resultType: 'matrix', - result: [], - }, - color: 'secondary', - children: 'Empty query result', - stacked: false, - queryParams: { - startTime: 1572100210000, - endTime: 1572100217898, - resolution: 10, - }, - }; - const graph = shallow(); - const alert = graph.find(Alert); - expect(alert.prop('color')).toEqual(props.color); - expect(alert.childAt(0).text()).toEqual(props.children); - }); - describe('data is returned', () => { const props: any = { queryParams: { @@ -100,9 +61,19 @@ describe('Graph', () => { expect(div).toHaveLength(1); expect(innerdiv).toHaveLength(1); }); + describe('Legend', () => { + it('renders a legend', () => { + const graph = shallow(); + expect(graph.find('.graph-legend .legend-item')).toHaveLength(1); + }); + }); + }); + describe('formatValue', () => { it('formats tick values correctly', () => { const graph = new Graph({ data: { result: [] }, queryParams: {} } as any); [ + { input: null, output: 'null' }, + { input: 0, output: '0.00' }, { input: 2e24, output: '2.00Y' }, { input: 2e23, output: '200.00Z' }, { input: 2e22, output: '20.00Z' }, @@ -159,10 +130,348 @@ describe('Graph', () => { expect(graph.formatValue(t.input)).toBe(t.output); }); }); - describe('Legend', () => { - it('renders a legend', () => { - const graph = shallow(); - expect(graph.find('.graph-legend .legend-item')).toHaveLength(1); + it('should throw error if no match', () => { + const graph = new Graph({ data: { result: [] }, queryParams: {} } as any); + try { + graph.formatValue(undefined as any); + } catch (error) { + expect(error.message).toEqual("couldn't format a value, this is a bug"); + } + }); + }); + describe('getColors', () => { + it('should generate proper colors', () => { + const graph = new Graph({ data: { result: [{}, {}, {}, {}, {}, {}, {}, {}, {}] }, queryParams: {} } as any); + expect( + graph + .getColors() + .map(c => c.toString()) + .join(',') + ).toEqual( + 'rgb(237,194,64),rgb(175,216,248),rgb(203,75,75),rgb(77,167,77),rgb(148,64,237),rgb(189,155,51),rgb(140,172,198),rgb(162,60,60),rgb(61,133,61)' + ); + }); + }); + describe('on component update', () => { + let graph: any; + beforeEach(() => { + jest.spyOn($, 'plot').mockImplementation(() => {}); + graph = mount( + + ); + }); + afterAll(() => { + jest.restoreAllMocks(); + }); + it('should trigger state update when new data is recieved', () => { + const spyState = jest.spyOn(Graph.prototype, 'setState'); + graph.setProps({ data: { result: [{ values: [{}], metric: {} }] } }); + expect(spyState).toHaveBeenCalledWith( + { + chartData: [ + { + color: 'rgb(237,194,64)', + data: [[1572128592000, 0]], + index: 0, + labels: {}, + }, + ], + selectedSeriesIndex: null, + }, + expect.anything() + ); + }); + it('should trigger state update when stacked prop is changed', () => { + const spyState = jest.spyOn(Graph.prototype, 'setState'); + graph.setProps({ stacked: true }); + expect(spyState).toHaveBeenCalledWith( + { + chartData: [ + { + color: 'rgb(237,194,64)', + data: [[1572128592000, 0]], + index: 0, + labels: {}, + }, + ], + selectedSeriesIndex: null, + }, + expect.anything() + ); + }); + }); + describe('on unmount', () => { + it('should call destroy plot', () => { + const wrapper = shallow( + + ); + const spyPlotDestroy = jest.spyOn(Graph.prototype, 'componentWillUnmount'); + wrapper.unmount(); + expect(spyPlotDestroy).toHaveBeenCalledTimes(1); + }); + }); + describe('parseValue', () => { + it('should parse number properly', () => { + expect(new Graph({ stacked: true, data: { result: [] }, queryParams: {} } as any).parseValue('12.3e')).toEqual(12.3); + }); + it('should return 0 if value is NaN and stacked prop is true', () => { + expect(new Graph({ stacked: true, data: { result: [] }, queryParams: {} } as any).parseValue('asd')).toEqual(0); + }); + it('should return null if value is NaN and stacked prop is false', () => { + expect(new Graph({ stacked: false, data: { result: [] }, queryParams: {} } as any).parseValue('asd')).toBeNull(); + }); + }); + + describe('plot', () => { + let spyFlot: any; + beforeEach(() => { + spyFlot = jest.spyOn($, 'plot').mockImplementation(() => {}); + }); + afterAll(() => { + jest.restoreAllMocks(); + }); + it('should not call jquery.plot if charRef not exist', () => { + const graph = shallow( + + ); + (graph.instance() as any).plot(); + expect(spyFlot).not.toBeCalled(); + }); + it('should call jquery.plot if charRef exist', () => { + const graph = mount( + + ); + (graph.instance() as any).plot(); + expect(spyFlot).toBeCalled(); + }); + it('should destroy plot', () => { + const spyPlotDestroy = jest.fn(); + jest.spyOn($, 'plot').mockReturnValue({ destroy: spyPlotDestroy } as any); + const graph = mount( + + ); + (graph.instance() as any).plot(); + (graph.instance() as any).destroyPlot(); + expect(spyPlotDestroy).toHaveBeenCalledTimes(1); + jest.restoreAllMocks(); + }); + }); + describe('plotSetAndDraw', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + it('should call spyPlotSetAndDraw on legend hover', () => { + jest.spyOn($, 'plot').mockReturnValue({ setData: jest.fn(), draw: jest.fn() } as any); + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb: any) => cb()); + const graph = mount( + + ); + (graph.instance() as any).plot(); // create chart + const spyPlotSetAndDraw = jest.spyOn(Graph.prototype, 'plotSetAndDraw'); + graph + .find('.legend-item') + .at(0) + .simulate('mouseover'); + expect(spyPlotSetAndDraw).toHaveBeenCalledTimes(1); + }); + it('should call spyPlotSetAndDraw with chartDate from state as default value', () => { + const spySetData = jest.fn(); + jest.spyOn($, 'plot').mockReturnValue({ setData: spySetData, draw: jest.fn() } as any); + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb: any) => cb()); + const graph: any = mount( + + ); + (graph.instance() as any).plot(); // create chart + graph.find('.graph-legend').simulate('mouseout'); + expect(spySetData).toHaveBeenCalledWith(graph.state().chartData); + }); + }); + describe('Plot options', () => { + it('should configer options properly if stacked prop is true', () => { + const wrapper = shallow( + + ); + expect((wrapper.instance() as any).getOptions()).toMatchObject({ + series: { + stack: true, + lines: { lineWidth: 1, steps: false, fill: true }, + shadowSize: 0, + }, + }); + }); + it('should configer options properly if stacked prop is false', () => { + const wrapper = shallow( + + ); + expect((wrapper.instance() as any).getOptions()).toMatchObject({ + series: { + stack: false, + lines: { lineWidth: 2, steps: false, fill: false }, + shadowSize: 0, + }, + }); + }); + it('should return proper tooltip html from options', () => { + const wrapper = shallow( + + ); + expect( + (wrapper.instance() as any) + .getOptions() + .tooltip.content('', 1572128592, 1572128592, { series: { labels: { foo: 1, bar: 2 }, color: '' } }) + ).toEqual(` +
Mon, 19 Jan 1970 04:42:08 GMT
+
+ + value: 1572128592 +
+
+
foo: 1
bar: 2
+
+ `); + }); + it('should render Plot with proper options', () => { + const wrapper = mount( + + ); + expect((wrapper.instance() as any).getOptions()).toEqual({ + grid: { + hoverable: true, + clickable: true, + autoHighlight: true, + mouseActiveRadius: 100, + }, + legend: { show: false }, + xaxis: { + mode: 'time', + showTicks: true, + showMinorTicks: true, + timeBase: 'milliseconds', + }, + yaxis: { tickFormatter: expect.anything() }, + crosshair: { mode: 'xy', color: '#bbb' }, + tooltip: { + show: true, + cssClass: 'graph-tooltip', + content: expect.anything(), + defaultTheme: false, + lines: true, + }, + series: { + stack: true, + lines: { lineWidth: 1, steps: false, fill: true }, + shadowSize: 0, + }, }); }); }); diff --git a/web/ui/react-app/src/Graph.tsx b/web/ui/react-app/src/Graph.tsx index 6b7b4d4d1..ce76cf2f4 100644 --- a/web/ui/react-app/src/Graph.tsx +++ b/web/ui/react-app/src/Graph.tsx @@ -1,11 +1,12 @@ import $ from 'jquery'; import React, { PureComponent } from 'react'; import ReactResizeDetector from 'react-resize-detector'; -import { Alert } from 'reactstrap'; import { escapeHTML } from './utils/html'; import SeriesName from './SeriesName'; import { Metric, QueryParams } from './types/types'; +import { isPresent } from './utils/func'; + require('flot'); require('flot/source/jquery.flot.crosshair'); require('flot/source/jquery.flot.legend'); @@ -22,25 +23,26 @@ interface GraphProps { queryParams: QueryParams | null; } -export interface GraphSeries { +interface GraphSeries { labels: { [key: string]: string }; color: string; - normalizedColor: string; data: (number | null)[][]; // [x,y][] index: number; } interface GraphState { selectedSeriesIndex: number | null; - hoveredSeriesIndex: number | null; + chartData: GraphSeries[]; } class Graph extends PureComponent { private chartRef = React.createRef(); + private $chart?: jquery.flot.plot; + private rafID = 0; state = { selectedSeriesIndex: null, - hoveredSeriesIndex: null, + chartData: this.getData(), }; formatValue = (y: number | null): string => { @@ -48,6 +50,7 @@ class Graph extends PureComponent { return 'null'; } const absY = Math.abs(y); + if (absY >= 1e24) { return (y / 1e24).toFixed(2) + 'Y'; } else if (absY >= 1e21) { @@ -176,18 +179,17 @@ class Graph extends PureComponent { getData(): GraphSeries[] { const colors = this.getColors(); - const { hoveredSeriesIndex } = this.state; const { stacked, queryParams } = this.props; const { startTime, endTime, resolution } = queryParams!; - return this.props.data.result.map((ts, index) => { + return this.props.data.result.map(({ values, metric }, index) => { // Insert nulls for all missing steps. const data = []; let pos = 0; for (let t = startTime; t <= endTime; t += resolution) { // Allow for floating point inaccuracy. - const currentValue = ts.values[pos]; - if (ts.values.length > pos && currentValue[0] < t + resolution / 100) { + const currentValue = values[pos]; + if (values.length > pos && currentValue[0] < t + resolution / 100) { data.push([currentValue[0] * 1000, this.parseValue(currentValue[1])]); pos++; } else { @@ -197,12 +199,10 @@ class Graph extends PureComponent { data.push([t * 1000, stacked ? 0 : null]); } } - const { r, g, b } = colors[index]; return { - labels: ts.metric !== null ? ts.metric : {}, - color: `rgba(${r}, ${g}, ${b}, ${hoveredSeriesIndex === null || hoveredSeriesIndex === index ? 1 : 0.3})`, - normalizedColor: `rgb(${r}, ${g}, ${b}`, + labels: metric !== null ? metric : {}, + color: colors[index].toString(), data, index, }; @@ -224,10 +224,9 @@ class Graph extends PureComponent { } componentDidUpdate(prevProps: GraphProps) { - if (prevProps.data !== this.props.data) { - this.setState({ selectedSeriesIndex: null }); + if (prevProps.data !== this.props.data || prevProps.stacked !== this.props.stacked) { + this.setState({ selectedSeriesIndex: null, chartData: this.getData() }, this.plot); } - this.plot(); } componentWillUnmount() { @@ -238,64 +237,83 @@ class Graph extends PureComponent { if (!this.chartRef.current) { return; } - const selectedData = this.getData()[this.state.selectedSeriesIndex!]; this.destroyPlot(); - $.plot($(this.chartRef.current), selectedData ? [selectedData] : this.getData(), this.getOptions()); + + this.$chart = $.plot($(this.chartRef.current), this.state.chartData, this.getOptions()); }; - destroyPlot() { - const chart = $(this.chartRef.current!).data('plot'); - if (chart !== undefined) { - chart.destroy(); + destroyPlot = () => { + if (isPresent(this.$chart)) { + this.$chart.destroy(); + } + }; + + plotSetAndDraw(data: GraphSeries[] = this.state.chartData) { + if (isPresent(this.$chart)) { + this.$chart.setData(data); + this.$chart.draw(); } } handleSeriesSelect = (index: number) => () => { - const { selectedSeriesIndex } = this.state; - this.setState({ selectedSeriesIndex: selectedSeriesIndex !== index ? index : null }); + const { selectedSeriesIndex, chartData } = this.state; + this.plotSetAndDraw( + selectedSeriesIndex === index ? chartData.map(this.toHoverColor(index)) : chartData.slice(index, index + 1) + ); + this.setState({ selectedSeriesIndex: selectedSeriesIndex === index ? null : index }); }; handleSeriesHover = (index: number) => () => { - this.setState({ hoveredSeriesIndex: index }); + if (this.rafID) { + cancelAnimationFrame(this.rafID); + } + this.rafID = requestAnimationFrame(() => { + this.plotSetAndDraw(this.state.chartData.map(this.toHoverColor(index))); + }); }; - handleLegendMouseOut = () => this.setState({ hoveredSeriesIndex: null }); + handleLegendMouseOut = () => { + cancelAnimationFrame(this.rafID); + this.plotSetAndDraw(); + }; + + getHoverColor = (color: string, opacity: number) => { + const { r, g, b } = $.color.parse(color); + if (!this.props.stacked) { + return `rgba(${r}, ${g}, ${b}, ${opacity})`; + } + /* + Unfortunetly flot doesn't take into consideration + the alpha value when adjusting the color on the stacked series. + TODO: find better way to set the opacity. + */ + const base = (1 - opacity) * 255; + return `rgb(${Math.round(base + opacity * r)},${Math.round(base + opacity * g)},${Math.round(base + opacity * b)})`; + }; + + toHoverColor = (index: number) => (series: GraphSeries, i: number) => ({ + ...series, + color: this.getHoverColor(series.color, i !== index ? 0.3 : 1), + }); render() { - if (this.props.data === null) { - return No data queried yet; - } - - if (this.props.data.resultType !== 'matrix') { - return ( - - Query result is of wrong type '{this.props.data.resultType}', should be 'matrix' (range vector). - - ); - } - - if (this.props.data.result.length === 0) { - return Empty query result; - } - - const { selectedSeriesIndex } = this.state; - const series = this.getData(); - const canUseHover = series.length > 1 && selectedSeriesIndex === null; + const { selectedSeriesIndex, chartData } = this.state; + const canUseHover = chartData.length > 1 && selectedSeriesIndex === null; return (
- {series.map(({ index, normalizedColor, labels }) => ( + {chartData.map(({ index, color, labels }) => (
1 ? this.handleSeriesSelect(index) : undefined} + style={{ opacity: selectedSeriesIndex === null || index === selectedSeriesIndex ? 1 : 0.5 }} + onClick={chartData.length > 1 ? this.handleSeriesSelect(index) : undefined} onMouseOver={canUseHover ? this.handleSeriesHover(index) : undefined} key={index} className="legend-item" > - +
))} diff --git a/web/ui/react-app/src/GraphTabContent.tsx b/web/ui/react-app/src/GraphTabContent.tsx new file mode 100644 index 000000000..b8f6d1fb7 --- /dev/null +++ b/web/ui/react-app/src/GraphTabContent.tsx @@ -0,0 +1,28 @@ +import React from 'react'; +import { Alert } from 'reactstrap'; +import Graph from './Graph'; +import { QueryParams } from './types/types'; +import { isPresent } from './utils/func'; + +export const GraphTabContent = ({ + data, + stacked, + lastQueryParams, +}: { + data: any; + stacked: boolean; + lastQueryParams: QueryParams | null; +}) => { + if (!isPresent(data)) { + return No data queried yet; + } + if (data.result.length === 0) { + return Empty query result; + } + if (data.resultType !== 'matrix') { + return ( + Query result is of wrong type '{data.resultType}', should be 'matrix' (range vector). + ); + } + return ; +}; diff --git a/web/ui/react-app/src/Panel.test.tsx b/web/ui/react-app/src/Panel.test.tsx index f44f65acb..8a8270ca8 100644 --- a/web/ui/react-app/src/Panel.test.tsx +++ b/web/ui/react-app/src/Panel.test.tsx @@ -3,10 +3,10 @@ import { mount, shallow } from 'enzyme'; import Panel, { PanelOptions, PanelType } from './Panel'; import ExpressionInput from './ExpressionInput'; import GraphControls from './GraphControls'; -import Graph from './Graph'; import { NavLink, TabPane } from 'reactstrap'; import TimeInput from './TimeInput'; import DataTable from './DataTable'; +import { GraphTabContent } from './GraphTabContent'; describe('Panel', () => { const props = { @@ -83,8 +83,8 @@ describe('Panel', () => { }; const graphPanel = mount(); const controls = graphPanel.find(GraphControls); - graphPanel.setState({ data: [] }); - const graph = graphPanel.find(Graph); + graphPanel.setState({ data: { resultType: 'matrix', result: [] } }); + const graph = graphPanel.find(GraphTabContent); expect(controls.prop('endTime')).toEqual(options.endTime); expect(controls.prop('range')).toEqual(options.range); expect(controls.prop('resolution')).toEqual(options.resolution); diff --git a/web/ui/react-app/src/Panel.tsx b/web/ui/react-app/src/Panel.tsx index 9422b59d9..df04af8d4 100644 --- a/web/ui/react-app/src/Panel.tsx +++ b/web/ui/react-app/src/Panel.tsx @@ -6,7 +6,7 @@ import moment from 'moment-timezone'; import ExpressionInput from './ExpressionInput'; import GraphControls from './GraphControls'; -import Graph from './Graph'; +import { GraphTabContent } from './GraphTabContent'; import DataTable from './DataTable'; import TimeInput from './TimeInput'; import QueryStatsView, { QueryStats } from './QueryStatsView'; @@ -287,11 +287,11 @@ class Panel extends Component { onChangeResolution={this.handleChangeResolution} onChangeStacking={this.handleChangeStacking} /> - {this.state.data ? ( - - ) : ( - No data queried yet - )} + )} diff --git a/web/ui/react-app/src/types/index.d.ts b/web/ui/react-app/src/types/index.d.ts index 41aea3f21..565069002 100644 --- a/web/ui/react-app/src/types/index.d.ts +++ b/web/ui/react-app/src/types/index.d.ts @@ -1,4 +1,8 @@ declare namespace jquery.flot { + // eslint-disable-next-line @typescript-eslint/class-name-casing + interface plot extends jquery.flot.plot { + destroy: () => void; + } // eslint-disable-next-line @typescript-eslint/class-name-casing interface plotOptions extends jquery.flot.plotOptions { tooltip: {