Graph multi series select (#6391)

* adding graph series multi select support

Signed-off-by: Boyko Lalov <boiskila@gmail.com>
Signed-off-by: blalov <boiskila@gmail.com>

* skip inital plot draw from ResizeDetector

Signed-off-by: Boyko Lalov <boiskila@gmail.com>
Signed-off-by: blalov <boiskila@gmail.com>

* fix unit tests

Signed-off-by: blalov <boiskila@gmail.com>

* extract Legend as component

Signed-off-by: blalov <boiskila@gmail.com>

* create legend state interface

Signed-off-by: blalov <boiskila@gmail.com>

* fix click selection issue

Signed-off-by: blalov <boiskila@gmail.com>

* fix single series select bug

Signed-off-by: Boyko Lalov <boiskila@gmail.com>

* review changes

Signed-off-by: blalov <boiskila@gmail.com>

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;

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(<Graph {...props} />);
-        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(
         <Graph
           {...({
@@ -85,12 +91,13 @@ describe('Graph', () => {
           } 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(
         <Graph
           {...({
             stacked: true,
@@ -141,21 +145,17 @@ describe('Graph', () => {
           } 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
           {...({
@@ -170,9 +170,12 @@ describe('Graph', () => {
         />
       );
       (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
           {...({
@@ -187,11 +190,11 @@ describe('Graph', () => {
         />
       );
       (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
           {...({
@@ -207,17 +210,12 @@ describe('Graph', () => {
       );
       (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
           {...({
@@ -232,7 +230,7 @@ describe('Graph', () => {
         />
       );
       (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
           {...({
@@ -258,7 +257,8 @@ describe('Graph', () => {
       );
       (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();
     });
   });
 });

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<GraphProps, GraphState> {
   private chartRef = React.createRef<HTMLDivElement>();
   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<GraphProps, GraphState> {
     }
   }
 
-  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<GraphProps, GraphState> {
     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 (
       <div className="graph">
-        <ReactResizeDetector handleWidth onResize={this.plot} />
+        <ReactResizeDetector handleWidth onResize={this.handleResize} skipOnMount />
         <div className="graph-chart" ref={this.chartRef} />
-        <div className="graph-legend" onMouseOut={canUseHover ? this.handleLegendMouseOut : undefined}>
-          {chartData.map(({ index, color, labels }) => (
-            <div
-              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"
-            >
-              <span className="legend-swatch" style={{ backgroundColor: color }}></span>
-              <SeriesName labels={labels} format />
-            </div>
-          ))}
-        </div>
+        <Legend
+          shouldReset={this.selectedSeriesIndexes.length === 0}
+          chartData={chartData}
+          onHover={this.handleSeriesHover}
+          onLegendMouseOut={this.handleLegendMouseOut}
+          onSeriesToggle={this.handleSeriesSelect}
+        />
       </div>
     );
   }

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<HTMLDivElement>) => void;
+  onSeriesToggle: (selected: number[], index: number) => void;
+  onHover: (index: number) => (ev: SyntheticEvent<HTMLDivElement>) => void;
+}
+
+interface LegendState {
+  selectedIndexes: number[];
+}
+
+export class Legend extends PureComponent<LegendProps, LegendState> {
+  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<number[]>((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 (
+      <div className="graph-legend" onMouseOut={canUseHover ? onLegendMouseOut : undefined}>
+        {chartData.map(({ index, color, labels }) => (
+          <div
+            style={{ opacity: selectedIndexes.length === 0 || selectedIndexes.includes(index) ? 1 : 0.5 }}
+            onClick={chartData.length > 1 ? this.handleSeriesSelect(index) : undefined}
+            onMouseOver={canUseHover ? onHover(index) : undefined}
+            key={index}
+            className="legend-item"
+          >
+            <span className="legend-swatch" style={{ backgroundColor: color }}></span>
+            <SeriesName labels={labels} format />
+          </div>
+        ))}
+        {chartData.length > 1 && (
+          <div className="pl-1 mt-1 text-muted" style={{ fontSize: 13 }}>
+            Click: select series, CTRL + click: toggle multiple series
+          </div>
+        )}
+      </div>
+    );
+  }
+}