diff --git a/web/ui/react-app/src/App.css b/web/ui/react-app/src/App.css index e9bf97adb7..303ee0faea 100644 --- a/web/ui/react-app/src/App.css +++ b/web/ui/react-app/src/App.css @@ -139,10 +139,10 @@ div.time-input { font-size: 0.75em; padding: 10px 5px; display: inline-block; - cursor: pointer; } .legend-item { + cursor: pointer; display: flex; align-items: center; padding: 0 5px; diff --git a/web/ui/react-app/src/graph/Graph.test.tsx b/web/ui/react-app/src/graph/Graph.test.tsx index daf985b7d6..e9874b477c 100644 --- a/web/ui/react-app/src/graph/Graph.test.tsx +++ b/web/ui/react-app/src/graph/Graph.test.tsx @@ -3,8 +3,12 @@ import $ from 'jquery'; import { shallow, mount } from 'enzyme'; import Graph from './Graph'; import ReactResizeDetector from 'react-resize-detector'; +import { Legend } from './Legend'; describe('Graph', () => { + beforeAll(() => { + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb: any) => cb()); + }); describe('data is returned', () => { const props: any = { queryParams: { @@ -64,14 +68,16 @@ describe('Graph', () => { describe('Legend', () => { it('renders a legend', () => { const graph = shallow(); - expect(graph.find('.graph-legend .legend-item')).toHaveLength(1); + expect(graph.find(Legend)).toHaveLength(1); }); }); }); describe('on component update', () => { let graph: any; + let spyState: any; + let mockPlot: any; beforeEach(() => { - jest.spyOn($, 'plot').mockImplementation(() => ({} as any)); + mockPlot = jest.spyOn($, 'plot').mockReturnValue({ setData: jest.fn(), draw: jest.fn(), destroy: jest.fn() } as any); graph = mount( { } as any)} /> ); + spyState = jest.spyOn(graph.instance(), 'setState'); }); - afterAll(() => { - jest.restoreAllMocks(); + afterEach(() => { + spyState.mockReset(); + mockPlot.mockReset(); }); 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( { @@ -102,25 +109,22 @@ describe('Graph', () => { 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 }); + graph.setProps({ stacked: false }); expect(spyState).toHaveBeenCalledWith( { chartData: [ { color: 'rgb(237,194,64)', - data: [[1572128592000, 0]], + data: [[1572128592000, null]], index: 0, labels: {}, }, ], - selectedSeriesIndex: null, }, expect.anything() ); @@ -128,7 +132,7 @@ describe('Graph', () => { }); describe('on unmount', () => { it('should call destroy plot', () => { - const wrapper = shallow( + const graph = mount( { } as any)} /> ); - const spyPlotDestroy = jest.spyOn(Graph.prototype, 'componentWillUnmount'); - wrapper.unmount(); + const spyPlotDestroy = jest.spyOn(graph.instance(), 'componentWillUnmount'); + graph.unmount(); expect(spyPlotDestroy).toHaveBeenCalledTimes(1); + spyPlotDestroy.mockReset(); }); }); describe('plot', () => { - let spyFlot: any; - beforeEach(() => { - spyFlot = jest.spyOn($, 'plot').mockImplementation(() => ({} as any)); - }); - afterAll(() => { - jest.restoreAllMocks(); - }); it('should not call jquery.plot if chartRef not exist', () => { + const mockSetData = jest.fn(); + jest.spyOn($, 'plot').mockReturnValue({ setData: mockSetData, draw: jest.fn(), destroy: jest.fn() } as any); const graph = shallow( { /> ); (graph.instance() as any).plot(); - expect(spyFlot).not.toBeCalled(); + expect(mockSetData).not.toBeCalled(); }); it('should call jquery.plot if chartRef exist', () => { + const mockPlot = jest + .spyOn($, 'plot') + .mockReturnValue({ setData: jest.fn(), draw: jest.fn(), destroy: jest.fn() } as any); const graph = mount( { /> ); (graph.instance() as any).plot(); - expect(spyFlot).toBeCalled(); + expect(mockPlot).toBeCalled(); }); it('should destroy plot', () => { - const spyPlotDestroy = jest.fn(); - jest.spyOn($, 'plot').mockReturnValue({ destroy: spyPlotDestroy } as any); + const mockDestroy = jest.fn(); + jest.spyOn($, 'plot').mockReturnValue({ setData: jest.fn(), draw: jest.fn(), destroy: mockDestroy } as any); const graph = mount( { ); (graph.instance() as any).plot(); (graph.instance() as any).destroyPlot(); - expect(spyPlotDestroy).toHaveBeenCalledTimes(1); - jest.restoreAllMocks(); + expect(mockDestroy).toHaveBeenCalledTimes(2); }); }); 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()); + jest.spyOn($, 'plot').mockReturnValue({ setData: jest.fn(), draw: jest.fn(), destroy: jest.fn() } as any); const graph = mount( { /> ); (graph.instance() as any).plot(); // create chart - const spyPlotSetAndDraw = jest.spyOn(Graph.prototype, 'plotSetAndDraw'); + const spyPlotSetAndDraw = jest.spyOn(graph.instance() as any, 'plotSetAndDraw'); graph .find('.legend-item') .at(0) @@ -240,9 +238,10 @@ describe('Graph', () => { 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 mockSetData = jest.fn(); + const spyPlot = jest + .spyOn($, 'plot') + .mockReturnValue({ setData: mockSetData, draw: jest.fn(), destroy: jest.fn() } as any); const graph: any = mount( { ); (graph.instance() as any).plot(); // create chart graph.find('.graph-legend').simulate('mouseout'); - expect(spySetData).toHaveBeenCalledWith(graph.state().chartData); + expect(mockSetData).toHaveBeenCalledWith(graph.state().chartData); + spyPlot.mockReset(); }); }); }); diff --git a/web/ui/react-app/src/graph/Graph.tsx b/web/ui/react-app/src/graph/Graph.tsx index 1ba196e7c2..4fe3caf258 100644 --- a/web/ui/react-app/src/graph/Graph.tsx +++ b/web/ui/react-app/src/graph/Graph.tsx @@ -2,7 +2,7 @@ import $ from 'jquery'; import React, { PureComponent } from 'react'; import ReactResizeDetector from 'react-resize-detector'; -import SeriesName from '../SeriesName'; +import { Legend } from './Legend'; import { Metric, QueryParams } from '../types/types'; import { isPresent } from '../utils/func'; import { normalizeData, getOptions, toHoverColor } from './GraphHelpers'; @@ -31,7 +31,6 @@ export interface GraphSeries { } interface GraphState { - selectedSeriesIndex: number | null; chartData: GraphSeries[]; } @@ -39,30 +38,43 @@ class Graph extends PureComponent { private chartRef = React.createRef(); private $chart?: jquery.flot.plot; private rafID = 0; + private selectedSeriesIndexes: number[] = []; state = { - selectedSeriesIndex: null, chartData: normalizeData(this.props), }; componentDidUpdate(prevProps: GraphProps) { const { data, stacked } = this.props; - if (prevProps.data !== data || prevProps.stacked !== stacked) { - this.setState({ selectedSeriesIndex: null, chartData: normalizeData(this.props) }, this.plot); + if (prevProps.data !== data) { + this.selectedSeriesIndexes = []; + this.setState({ chartData: normalizeData(this.props) }, this.plot); + } else if (prevProps.stacked !== stacked) { + this.setState({ chartData: normalizeData(this.props) }, () => { + if (this.selectedSeriesIndexes.length === 0) { + this.plot(); + } else { + this.plot(this.state.chartData.filter((_, i) => this.selectedSeriesIndexes.includes(i))); + } + }); } } + componentDidMount() { + this.plot(); + } + componentWillUnmount() { this.destroyPlot(); } - plot = () => { + plot = (data: GraphSeries[] = this.state.chartData) => { if (!this.chartRef.current) { return; } this.destroyPlot(); - this.$chart = $.plot($(this.chartRef.current), this.state.chartData, getOptions(this.props.stacked)); + this.$chart = $.plot($(this.chartRef.current), data, getOptions(this.props.stacked)); }; destroyPlot = () => { @@ -78,14 +90,14 @@ class Graph extends PureComponent { } } - handleSeriesSelect = (index: number) => () => { - const { selectedSeriesIndex, chartData } = this.state; - this.plotSetAndDraw( - selectedSeriesIndex === index - ? chartData.map(toHoverColor(index, this.props.stacked)) - : chartData.slice(index, index + 1) + handleSeriesSelect = (selected: number[], selectedIndex: number) => { + const { chartData } = this.state; + this.plot( + this.selectedSeriesIndexes.length === 1 && this.selectedSeriesIndexes.includes(selectedIndex) + ? chartData.map(toHoverColor(selectedIndex, this.props.stacked)) + : chartData.filter((_, i) => selected.includes(i)) // draw only selected ); - this.setState({ selectedSeriesIndex: selectedSeriesIndex === index ? null : index }); + this.selectedSeriesIndexes = selected; }; handleSeriesHover = (index: number) => () => { @@ -102,28 +114,25 @@ class Graph extends PureComponent { this.plotSetAndDraw(); }; - render() { - const { selectedSeriesIndex, chartData } = this.state; - const canUseHover = chartData.length > 1 && selectedSeriesIndex === null; + handleResize = () => { + if (isPresent(this.$chart)) { + this.plot(this.$chart.getData() as GraphSeries[]); + } + }; + render() { + const { chartData } = this.state; return (
- +
-
- {chartData.map(({ index, color, labels }) => ( -
1 ? this.handleSeriesSelect(index) : undefined} - onMouseOver={canUseHover ? this.handleSeriesHover(index) : undefined} - key={index} - className="legend-item" - > - - -
- ))} -
+
); } diff --git a/web/ui/react-app/src/graph/Legend.tsx b/web/ui/react-app/src/graph/Legend.tsx new file mode 100644 index 0000000000..894f59b280 --- /dev/null +++ b/web/ui/react-app/src/graph/Legend.tsx @@ -0,0 +1,77 @@ +import React, { PureComponent, SyntheticEvent } from 'react'; +import SeriesName from '../SeriesName'; +import { GraphSeries } from './Graph'; + +interface LegendProps { + chartData: GraphSeries[]; + shouldReset: boolean; + onLegendMouseOut: (ev: SyntheticEvent) => void; + onSeriesToggle: (selected: number[], index: number) => void; + onHover: (index: number) => (ev: SyntheticEvent) => void; +} + +interface LegendState { + selectedIndexes: number[]; +} + +export class Legend extends PureComponent { + state = { + selectedIndexes: [] as number[], + }; + componentDidUpdate(prevProps: LegendProps) { + if (this.props.shouldReset && prevProps.shouldReset !== this.props.shouldReset) { + this.setState({ selectedIndexes: [] }); + } + } + handleSeriesSelect = (index: number) => (ev: any) => { + // TODO: add proper event type + const { selectedIndexes } = this.state; + + let selected = [index]; + if (ev.ctrlKey) { + const { chartData } = this.props; + if (selectedIndexes.includes(index)) { + selected = selectedIndexes.filter(idx => idx !== index); + } else { + selected = + // Flip the logic - In case none is selected ctrl + click should deselect clicked series. + selectedIndexes.length === 0 + ? chartData.reduce((acc, _, i) => (i === index ? acc : [...acc, i]), []) + : [...selectedIndexes, index]; // Select multiple. + } + } else if (selectedIndexes.length === 1 && selectedIndexes.includes(index)) { + selected = []; + } + + this.setState({ selectedIndexes: selected }); + this.props.onSeriesToggle(selected, index); + }; + + render() { + const { chartData, onLegendMouseOut, onHover } = this.props; + const { selectedIndexes } = this.state; + const canUseHover = chartData.length > 1 && selectedIndexes.length === 0; + + return ( +
+ {chartData.map(({ index, color, labels }) => ( +
1 ? this.handleSeriesSelect(index) : undefined} + onMouseOver={canUseHover ? onHover(index) : undefined} + key={index} + className="legend-item" + > + + +
+ ))} + {chartData.length > 1 && ( +
+ Click: select series, CTRL + click: toggle multiple series +
+ )} +
+ ); + } +}