Browse Source

fix(graph): legend height calculation now takes into account series hidden from legend, fixes #4245

Torkel Ödegaard 9 years ago
parent
commit
4b08f3909d

+ 17 - 0
public/app/core/time_series2.ts

@@ -180,4 +180,21 @@ export default class TimeSeries {
     }
     return false;
   }
+
+  hideFromLegend(options) {
+    if (options.hideEmpty && this.allIsNull) {
+      return true;
+    }
+    // ignore series excluded via override
+    if (!this.legend) {
+      return true;
+    }
+
+    // ignore zero series
+    if (options.hideZero && this.allIsZero) {
+      return true;
+    }
+
+    return false;
+  }
 }

+ 9 - 4
public/app/plugins/panel/graph/graph.js

@@ -69,8 +69,13 @@ function (angular, $, moment, _, kbn, GraphTooltip) {
           if (!panel.legend.show || panel.legend.rightSide) {
             return 0;
           }
+
           if (panel.legend.alignAsTable) {
-            var total = 30 + (25 * data.length);
+            var legendSeries = _.filter(data, function(series) {
+              return series.hideFromLegend(panel.legend) === false;
+            });
+            console.log(legendSeries.length);
+            var total = 23 + (22 * legendSeries.length);
             return Math.min(total, Math.floor(panelHeight/2));
           } else {
             return 26;
@@ -85,13 +90,14 @@ function (angular, $, moment, _, kbn, GraphTooltip) {
             }
 
             graphHeight -= 5; // padding
-            graphHeight -= panel.title ? 24 : 9; // subtract panel title bar
+            graphHeight -= panel.title ? 25 : 5; // subtract panel title bar
             graphHeight = graphHeight - getLegendHeight(graphHeight); // subtract one line legend
 
             elem.css('height', graphHeight + 'px');
 
             return true;
           } catch(e) { // IE throws errors sometimes
+            console.log(e);
             return false;
           }
         }
@@ -107,7 +113,7 @@ function (angular, $, moment, _, kbn, GraphTooltip) {
 
           if (!setElementHeight()) { return true; }
 
-          if (_.isString(data)) {
+          if(_.isString(data)) {
             render_panel_as_graphite_png(data);
             return true;
           }
@@ -226,7 +232,6 @@ function (angular, $, moment, _, kbn, GraphTooltip) {
 
           for (var i = 0; i < data.length; i++) {
             var series = data[i];
-            series.applySeriesOverrides(panel.seriesOverrides);
             series.data = series.getFlotPairs(series.nullPointMode || panel.nullPointMode, panel.y_formats);
 
             // if hidden remove points and disable stack

+ 2 - 13
public/app/plugins/panel/graph/legend.js

@@ -145,16 +145,7 @@ function (angular, _, $) {
           for (i = 0; i < seriesList.length; i++) {
             var series = seriesList[i];
 
-            // ignore empty series
-            if (panel.legend.hideEmpty && series.allIsNull) {
-              continue;
-            }
-            // ignore series excluded via override
-            if (!series.legend) {
-              continue;
-            }
-            // ignore zero series
-            if (panel.legend.hideZero && series.allIsZero) {
+            if (series.hideFromLegend(panel.legend)) {
               continue;
             }
 
@@ -166,9 +157,7 @@ function (angular, _, $) {
             html += '<i class="fa fa-minus pointer" style="color:' + series.color + '"></i>';
             html += '</div>';
 
-            html += '<div class="graph-legend-alias">';
-            html += '<a>' + _.escape(series.label) + '</a>';
-            html += '</div>';
+            html += '<a class="graph-legend-alias pointer">' + _.escape(series.label) + '</a>';
 
             if (panel.legend.values) {
               var avg = series.formatValue(series.stats.avg);

+ 1 - 0
public/app/plugins/panel/graph/module.ts

@@ -205,6 +205,7 @@ class GraphCtrl extends MetricsPanelCtrl {
       this.panel.tooltip.msResolution = this.panel.tooltip.msResolution || series.isMsResolutionNeeded();
     }
 
+    series.applySeriesOverrides(this.panel.seriesOverrides);
     return series;
   }
 

+ 8 - 8
public/app/plugins/panel/graph/specs/graph_specs.ts

@@ -5,8 +5,8 @@ import {describe, beforeEach, it, sinon, expect, angularMocks} from '../../../..
 import '../module';
 import angular from 'angular';
 import $ from 'jquery';
-import helpers from '../../../../../test/specs/helpers';
-import TimeSeries from '../../../../core/time_series2';
+import helpers from 'test/specs/helpers';
+import TimeSeries from 'app/core/time_series2';
 import moment from 'moment';
 
 describe('grafanaGraph', function() {
@@ -182,11 +182,10 @@ describe('grafanaGraph', function() {
     ctx.setup(function(ctrl, data) {
       ctrl.panel.lines = true;
       ctrl.panel.fill = 5;
-      ctrl.panel.seriesOverrides = [
-        { alias: 'test', fill: 0, points: true }
-      ];
-
+      data[0].zindex = 10;
       data[1].alias = 'test';
+      data[1].lines = {fill: 0.001};
+      data[1].points = {show: true};
     });
 
     it('should match second series and fill zero, and enable points', function() {
@@ -197,8 +196,9 @@ describe('grafanaGraph', function() {
   });
 
   graphScenario('should order series order according to zindex', function(ctx) {
-    ctx.setup(function(ctrl) {
-      ctrl.panel.seriesOverrides = [{ alias: 'series1', zindex: 2 }];
+    ctx.setup(function(ctrl, data) {
+      data[1].zindex = 1;
+      data[0].zindex = 10;
     });
 
     it('should move zindex 2 last', function() {

+ 4 - 2
public/sass/components/_panel_graph.scss

@@ -39,6 +39,7 @@
 .graph-legend-icon,
 .graph-legend-alias,
 .graph-legend-value {
+  cursor: pointer;
   float: left;
   white-space: nowrap;
   font-size: 85%;
@@ -79,7 +80,7 @@
 
 .graph-legend-table {
   width: 100%;
-  margin: 0;
+  margin-top: 4px;
 
   .graph-legend-series {
     display: table-row;
@@ -174,7 +175,8 @@
 }
 
 .graph-legend-series-hidden {
-  a {
+  .graph-legend-value,
+  .graph-legend-alias {
     color: $link-color-disabled;
   }
 }