From fa5fad5c7e45ee1a8ee89a9e1b0e98d79ed7575a Mon Sep 17 00:00:00 2001
From: Alexander Zobnin <alexanderzobnin@gmail.com>
Date: Tue, 23 Oct 2018 14:08:30 +0300
Subject: [PATCH] graph legend: review fixes

---
 .../app/plugins/panel/graph/Legend/Legend.tsx | 33 ++++++++++++-----
 .../panel/graph/Legend/LegendSeriesItem.tsx   | 36 +++++--------------
 public/sass/components/_panel_graph.scss      | 11 ------
 3 files changed, 32 insertions(+), 48 deletions(-)

diff --git a/public/app/plugins/panel/graph/Legend/Legend.tsx b/public/app/plugins/panel/graph/Legend/Legend.tsx
index e0ea048af92..b46de63db04 100644
--- a/public/app/plugins/panel/graph/Legend/Legend.tsx
+++ b/public/app/plugins/panel/graph/Legend/Legend.tsx
@@ -58,7 +58,7 @@ export type LegendComponentProps = LegendProps &
   LegendComponentEventHandlers;
 
 interface LegendState {
-  hiddenSeries: any;
+  hiddenSeries: { [seriesAlias: string]: boolean };
 }
 
 export class GraphLegend extends React.PureComponent<GraphLegendProps, LegendState> {
@@ -127,7 +127,7 @@ export class GraphLegend extends React.PureComponent<GraphLegendProps, LegendSta
     }
 
     // check if every other series is hidden
-    const alreadyExclusive = _.every(this.props.seriesList, value => {
+    const alreadyExclusive = this.props.seriesList.every(value => {
       if (value.alias === series.alias) {
         return true;
       }
@@ -137,12 +137,12 @@ export class GraphLegend extends React.PureComponent<GraphLegendProps, LegendSta
 
     if (alreadyExclusive) {
       // remove all hidden series
-      _.each(this.props.seriesList, value => {
+      this.props.seriesList.forEach(value => {
         delete hiddenSeries[value.alias];
       });
     } else {
       // hide all but this serie
-      _.each(this.props.seriesList, value => {
+      this.props.seriesList.forEach(value => {
         if (value.alias === series.alias) {
           return;
         }
@@ -155,13 +155,26 @@ export class GraphLegend extends React.PureComponent<GraphLegendProps, LegendSta
   }
 
   render() {
-    const { optionalClass, rightSide, sideWidth, sort, sortDesc, hideEmpty, hideZero } = this.props;
-    const { values, min, max, avg, current, total } = this.props;
+    const {
+      optionalClass,
+      rightSide,
+      sideWidth,
+      sort,
+      sortDesc,
+      hideEmpty,
+      hideZero,
+      values,
+      min,
+      max,
+      avg,
+      current,
+      total,
+    } = this.props;
     const seriesValuesProps = { values, min, max, avg, current, total };
     const hiddenSeries = this.state.hiddenSeries;
     const seriesHideProps = { hideEmpty, hideZero };
     const sortProps = { sort, sortDesc };
-    const seriesList = _.filter(this.sortLegend(), series => !series.hideFromLegend(seriesHideProps));
+    const seriesList = this.sortLegend().filter(series => !series.hideFromLegend(seriesHideProps));
     const legendClass = `${this.props.alignAsTable ? 'graph-legend-table' : ''} ${optionalClass}`;
 
     // Set min-width if side style and there is a value, otherwise remove the CSS property
@@ -198,7 +211,9 @@ class LegendSeriesList extends React.PureComponent<LegendComponentProps> {
     const seriesValuesProps = { values, min, max, avg, current, total };
     return seriesList.map((series, i) => (
       <LegendItem
-        key={i}
+        // This trick required because TimeSeries.id is not unique (it's just TimeSeries.alias).
+        // In future would be good to make id unique across the series list.
+        key={`${series.id}-${i}`}
         series={series}
         hidden={hiddenSeries[series.alias]}
         {...seriesValuesProps}
@@ -253,7 +268,7 @@ class LegendTable extends React.PureComponent<Partial<LegendComponentProps>> {
           </tr>
           {seriesList.map((series, i) => (
             <LegendItem
-              key={i}
+              key={`${series.id}-${i}`}
               asTable={true}
               series={series}
               hidden={hiddenSeries[series.alias]}
diff --git a/public/app/plugins/panel/graph/Legend/LegendSeriesItem.tsx b/public/app/plugins/panel/graph/Legend/LegendSeriesItem.tsx
index c235f132251..ecf7d477edf 100644
--- a/public/app/plugins/panel/graph/Legend/LegendSeriesItem.tsx
+++ b/public/app/plugins/panel/graph/Legend/LegendSeriesItem.tsx
@@ -1,4 +1,5 @@
 import React from 'react';
+import classNames from 'classnames';
 import { TimeSeries } from 'app/core/core';
 import { SeriesColorPicker } from 'app/core/components/colorpicker/SeriesColorPicker';
 
@@ -61,18 +62,6 @@ export class LegendItem extends React.PureComponent<LegendItemProps, LegendItemS
     this.forceUpdate();
   };
 
-  getOptionSeriesCSSClasses() {
-    const { series, hidden } = this.props;
-    const classes = [];
-    if (series.yaxis === 2) {
-      classes.push('graph-legend-series--right-y');
-    }
-    if (hidden) {
-      classes.push('graph-legend-series-hidden');
-    }
-    return classes.join(' ');
-  }
-
   renderLegendValues() {
     const { series, asTable } = this.props;
     const legendValueItems = [];
@@ -88,8 +77,11 @@ export class LegendItem extends React.PureComponent<LegendItemProps, LegendItemS
   }
 
   render() {
-    const { series, values, asTable } = this.props;
-    const seriesOptionClasses = this.getOptionSeriesCSSClasses();
+    const { series, values, asTable, hidden } = this.props;
+    const seriesOptionClasses = classNames({
+      'graph-legend-series-hidden': hidden,
+      'graph-legend-series--right-y': series.yaxis === 2,
+    });
     const valueItems = values ? this.renderLegendValues() : [];
     const seriesLabel = (
       <LegendSeriesLabel
@@ -173,25 +165,13 @@ class LegendSeriesIcon extends React.PureComponent<LegendSeriesIconProps, Legend
     onToggleAxis: () => {},
   };
 
-  constructor(props) {
-    super(props);
-    this.state = {
-      color: this.props.color,
-    };
-  }
-
-  onColorChange = color => {
-    this.setState({ color: color });
-    this.props.onColorChange(color);
-  };
-
   render() {
     return (
       <SeriesColorPicker
         optionalClass="graph-legend-icon"
         yaxis={this.props.yaxis}
-        color={this.state.color}
-        onColorChange={this.onColorChange}
+        color={this.props.color}
+        onColorChange={this.props.onColorChange}
         onToggleAxis={this.props.onToggleAxis}
       >
         <SeriesIcon color={this.props.color} />
diff --git a/public/sass/components/_panel_graph.scss b/public/sass/components/_panel_graph.scss
index c8ffd2985d8..e9e0c756f46 100644
--- a/public/sass/components/_panel_graph.scss
+++ b/public/sass/components/_panel_graph.scss
@@ -61,13 +61,6 @@
   position: relative;
 }
 
-// @TODO: delete unused class
-.graph-legend-scroll {
-  position: relative;
-  overflow: auto !important;
-  padding: 1px;
-}
-
 .graph-legend-icon {
   position: relative;
   padding-right: 4px;
@@ -133,10 +126,6 @@
     .graph-legend-table {
       display: table;
       width: auto;
-
-      .graph-legend-scroll {
-        display: table;
-      }
     }
   }
 }