You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@superset.apache.org by GitBox <gi...@apache.org> on 2018/04/10 20:37:37 UTC

[GitHub] mistercrunch closed pull request #4756: Improve xAxis ticks, thinner bottom margin

mistercrunch closed pull request #4756: Improve xAxis ticks, thinner bottom margin
URL: https://github.com/apache/incubator-superset/pull/4756
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx b/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
index 66e60c9b2b..fed7c28bbe 100644
--- a/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
+++ b/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import CopyToClipboard from '../../components/CopyToClipboard';
-import { storeQuery } from '../../../utils/common';
+import { storeQuery } from '../../utils/common';
 import { t } from '../../locales';
 
 const propTypes = {
diff --git a/superset/assets/javascripts/SqlLab/components/QueryTable.jsx b/superset/assets/javascripts/SqlLab/components/QueryTable.jsx
index 01d490277e..30d158b718 100644
--- a/superset/assets/javascripts/SqlLab/components/QueryTable.jsx
+++ b/superset/assets/javascripts/SqlLab/components/QueryTable.jsx
@@ -10,7 +10,7 @@ import ResultSet from './ResultSet';
 import ModalTrigger from '../../components/ModalTrigger';
 import HighlightedSql from './HighlightedSql';
 import { fDuration } from '../../modules/dates';
-import { storeQuery } from '../../../utils/common';
+import { storeQuery } from '../../utils/common';
 import QueryStateLabel from './QueryStateLabel';
 import { t } from '../../locales';
 
diff --git a/superset/assets/javascripts/chart/Chart.jsx b/superset/assets/javascripts/chart/Chart.jsx
index 4b4410640a..fb9884bfbb 100644
--- a/superset/assets/javascripts/chart/Chart.jsx
+++ b/superset/assets/javascripts/chart/Chart.jsx
@@ -127,9 +127,7 @@ class Chart extends React.PureComponent {
   }
 
   clearError() {
-    this.setState({
-      errorMsg: null,
-    });
+    this.setState({ errorMsg: null });
   }
 
   width() {
@@ -151,6 +149,10 @@ class Chart extends React.PureComponent {
     return d3format(format, number);
   }
 
+  error(e) {
+    this.props.actions.chartRenderingFailed(e, this.props.chartKey);
+  }
+
   render_template(s) {
     const context = {
       width: this.width(),
@@ -199,7 +201,6 @@ class Chart extends React.PureComponent {
       });
       this.props.actions.chartRenderingSucceeded(this.props.chartKey);
     } catch (e) {
-      console.error(e);  // eslint-disable-line
       this.props.actions.chartRenderingFailed(e, this.props.chartKey);
     }
   }
diff --git a/superset/assets/javascripts/explore/components/SaveModal.jsx b/superset/assets/javascripts/explore/components/SaveModal.jsx
index 4aa7670466..90bf12bc99 100644
--- a/superset/assets/javascripts/explore/components/SaveModal.jsx
+++ b/superset/assets/javascripts/explore/components/SaveModal.jsx
@@ -6,7 +6,7 @@ import { connect } from 'react-redux';
 import { Modal, Alert, Button, Radio } from 'react-bootstrap';
 import Select from 'react-select';
 import { t } from '../../locales';
-import { supersetURL } from '../../../utils/common';
+import { supersetURL } from '../../utils/common';
 
 const propTypes = {
   can_overwrite: PropTypes.bool,
diff --git a/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx b/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx
index aa278b7a46..a0c0078ad4 100644
--- a/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx
+++ b/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx
@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { Popover, OverlayTrigger } from 'react-bootstrap';
 import CopyToClipboard from './../../components/CopyToClipboard';
-import { getShortUrl } from '../../../utils/common';
+import { getShortUrl } from '../../utils/common';
 import { getExploreLongUrl } from '../exploreUtils';
 import { t } from '../../locales';
 
diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx
index 5bd825a633..2cc70f11ad 100644
--- a/superset/assets/javascripts/explore/stores/controls.jsx
+++ b/superset/assets/javascripts/explore/stores/controls.jsx
@@ -724,6 +724,16 @@ export const controls = {
     description: t('Bottom margin, in pixels, allowing for more room for axis labels'),
   },
 
+  x_ticks_layout: {
+    type: 'SelectControl',
+    label: t('X Tick Layout'),
+    choices: formatSelectOptions(['auto', 'flat', '45°', 'staggered']),
+    default: 'auto',
+    clearable: false,
+    renderTrigger: true,
+    description: t('The way the ticks are laid out on the X axis'),
+  },
+
   left_margin: {
     type: 'SelectControl',
     freeForm: true,
diff --git a/superset/assets/javascripts/explore/stores/visTypes.js b/superset/assets/javascripts/explore/stores/visTypes.js
index 8f52020ed8..d1807aeb9d 100644
--- a/superset/assets/javascripts/explore/stores/visTypes.js
+++ b/superset/assets/javascripts/explore/stores/visTypes.js
@@ -118,9 +118,16 @@ export const visTypes = {
           ['color_scheme'],
           ['show_legend', 'show_bar_value'],
           ['bar_stacked', 'order_bars'],
-          ['y_axis_format', 'bottom_margin'],
-          ['x_axis_label', 'y_axis_label'],
-          ['reduce_x_ticks', 'show_controls'],
+          ['y_axis_format', 'y_axis_label'],
+          ['show_controls', null],
+        ],
+      },
+      {
+        label: t('X Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'reduce_x_ticks'],
         ],
       },
     ],
@@ -182,7 +189,8 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['x_axis_label', 'bottom_margin'],
-          ['x_axis_showminmax', 'x_axis_format'],
+          ['x_ticks_layout', 'x_axis_format'],
+          ['x_axis_showminmax', null],
         ],
       },
       {
@@ -312,13 +320,21 @@ export const visTypes = {
         ],
       },
       {
-        label: t('Axes'),
+        label: t('X Axis'),
         expanded: true,
         controlSetRows: [
-          ['x_axis_format', 'y_axis_format'],
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'x_axis_format'],
           ['x_axis_showminmax', 'reduce_x_ticks'],
-          ['x_axis_label', 'y_axis_label'],
-          ['y_axis_bounds', 'y_log_scale'],
+        ],
+      },
+      {
+        label: t('Y Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['y_axis_label', 'left_margin'],
+          ['y_axis_showminmax', 'y_log_scale'],
+          ['y_axis_format', 'y_axis_bounds'],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -342,7 +358,24 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['color_scheme'],
-          ['x_axis_format', 'y_axis_format'],
+        ],
+      },
+      {
+        label: t('X Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'x_axis_format'],
+          ['x_axis_showminmax', null],
+        ],
+      },
+      {
+        label: t('Y Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['y_axis_label', 'left_margin'],
+          ['y_axis_showminmax', 'y_log_scale'],
+          ['y_axis_format', 'y_axis_bounds'],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -724,10 +757,18 @@ export const visTypes = {
         ],
       },
       {
-        label: t('Axes'),
+        label: t('X Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'x_axis_format'],
+          ['x_axis_showminmax', null],
+        ],
+      },
+      {
+        label: t('Y Axis'),
         expanded: true,
         controlSetRows: [
-          ['x_axis_format', 'x_axis_showminmax'],
           ['y_axis_format', 'y_axis_bounds'],
           ['y_log_scale', null],
         ],
@@ -979,7 +1020,9 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['series', 'entity'],
-          ['size', 'limit'],
+          ['x', 'y'],
+          ['size', 'max_bubble_size'],
+          ['limit', null],
         ],
       },
       {
@@ -990,18 +1033,12 @@ export const visTypes = {
           ['show_legend', null],
         ],
       },
-      {
-        label: t('Bubbles'),
-        controlSetRows: [
-          ['size', 'max_bubble_size'],
-        ],
-      },
       {
         label: t('X Axis'),
         expanded: true,
         controlSetRows: [
           ['x_axis_label', 'left_margin'],
-          ['x', 'x_axis_format'],
+          ['x_axis_format', 'x_ticks_layout'],
           ['x_log_scale', 'x_axis_showminmax'],
         ],
       },
@@ -1010,7 +1047,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['y_axis_label', 'bottom_margin'],
-          ['y', 'y_axis_format'],
+          ['y_axis_format', null],
           ['y_log_scale', 'y_axis_showminmax'],
         ],
       },
diff --git a/superset/assets/utils/common.js b/superset/assets/javascripts/utils/common.js
similarity index 92%
rename from superset/assets/utils/common.js
rename to superset/assets/javascripts/utils/common.js
index c7be5ddbb4..f2c3bd24ce 100644
--- a/superset/assets/utils/common.js
+++ b/superset/assets/javascripts/utils/common.js
@@ -94,3 +94,12 @@ export function supersetURL(rootUrl, getParams = {}) {
   }
   return url.href;
 }
+
+export function isTruthy(obj) {
+  if (typeof obj === 'boolean') {
+    return obj;
+  } else if (typeof obj === 'string') {
+    return ['yes', 'y', 'true', 't', '1'].indexOf(obj.toLowerCase()) >= 0;
+  }
+  return !!obj;
+}
diff --git a/superset/assets/utils/reducerUtils.js b/superset/assets/javascripts/utils/reducerUtils.js
similarity index 100%
rename from superset/assets/utils/reducerUtils.js
rename to superset/assets/javascripts/utils/reducerUtils.js
diff --git a/superset/assets/spec/javascripts/explore/utils_spec.jsx b/superset/assets/spec/javascripts/explore/utils_spec.jsx
index fe18f39073..9f855d2f39 100644
--- a/superset/assets/spec/javascripts/explore/utils_spec.jsx
+++ b/superset/assets/spec/javascripts/explore/utils_spec.jsx
@@ -3,7 +3,7 @@ import { expect } from 'chai';
 import URI from 'urijs';
 import { getExploreUrlAndPayload, getExploreLongUrl } from '../../../javascripts/explore/exploreUtils';
 
-describe('utils', () => {
+describe('exploreUtils', () => {
   const location = window.location;
   const formData = {
     datasource: '1__table',
diff --git a/superset/assets/spec/javascripts/utils/common_spec.jsx b/superset/assets/spec/javascripts/utils/common_spec.jsx
new file mode 100644
index 0000000000..a3490d476e
--- /dev/null
+++ b/superset/assets/spec/javascripts/utils/common_spec.jsx
@@ -0,0 +1,43 @@
+import { it, describe } from 'mocha';
+import { expect } from 'chai';
+import { isTruthy } from '../../../javascripts/utils/common';
+
+describe('utils/common', () => {
+  describe('isTruthy', () => {
+    it('evals false-looking strings properly', () => {
+      expect(isTruthy('f')).to.equal(false);
+      expect(isTruthy('false')).to.equal(false);
+      expect(isTruthy('no')).to.equal(false);
+      expect(isTruthy('n')).to.equal(false);
+      expect(isTruthy('F')).to.equal(false);
+      expect(isTruthy('False')).to.equal(false);
+      expect(isTruthy('NO')).to.equal(false);
+      expect(isTruthy('N')).to.equal(false);
+    });
+    it('evals true-looking strings properly', () => {
+      expect(isTruthy('t')).to.equal(true);
+      expect(isTruthy('true')).to.equal(true);
+      expect(isTruthy('yes')).to.equal(true);
+      expect(isTruthy('y')).to.equal(true);
+      expect(isTruthy('Y')).to.equal(true);
+      expect(isTruthy('True')).to.equal(true);
+      expect(isTruthy('Yes')).to.equal(true);
+      expect(isTruthy('YES')).to.equal(true);
+    });
+    it('evals bools properly', () => {
+      expect(isTruthy(false)).to.equal(false);
+      expect(isTruthy(true)).to.equal(true);
+    });
+    it('evals ints properly', () => {
+      expect(isTruthy(0)).to.equal(false);
+      expect(isTruthy(1)).to.equal(true);
+    });
+    it('evals constants properly', () => {
+      expect(isTruthy(null)).to.equal(false);
+      expect(isTruthy(undefined)).to.equal(false);
+    });
+    it('string auto is false', () => {
+      expect(isTruthy('false')).to.equal(false);
+    });
+  });
+});
diff --git a/superset/assets/visualizations/mapbox.jsx b/superset/assets/visualizations/mapbox.jsx
index baea634285..0c01d6b19e 100644
--- a/superset/assets/visualizations/mapbox.jsx
+++ b/superset/assets/visualizations/mapbox.jsx
@@ -17,7 +17,7 @@ import {
   DEFAULT_LONGITUDE,
   DEFAULT_LATITUDE,
   DEFAULT_ZOOM,
-} from '../utils/common';
+} from '../javascripts/utils/common';
 import './mapbox.css';
 
 const NOOP = () => {};
diff --git a/superset/assets/visualizations/nvd3_vis.js b/superset/assets/visualizations/nvd3_vis.js
index fb439ec720..8ee68fe2a5 100644
--- a/superset/assets/visualizations/nvd3_vis.js
+++ b/superset/assets/visualizations/nvd3_vis.js
@@ -12,6 +12,8 @@ import AnnotationTypes, {
   applyNativeColumns,
 } from '../javascripts/modules/AnnotationTypes';
 import { customizeToolTip, d3TimeFormatPreset, d3FormatPreset, tryNumify } from '../javascripts/modules/utils';
+import { isTruthy } from '../javascripts/utils/common';
+import { t } from '../javascripts/locales';
 
 // CSS
 import '../node_modules/nvd3/build/nv.d3.min.css';
@@ -59,14 +61,14 @@ const addTotalBarValues = function (svg, chart, data, stacked, axisFormat) {
         const yPos = parseFloat(rectObj.attr('y'));
         const xPos = parseFloat(rectObj.attr('x'));
         const rectWidth = parseFloat(rectObj.attr('width'));
-        const t = groupLabels.append('text')
+        const textEls = groupLabels.append('text')
           .attr('x', xPos) // rough position first, fine tune later
           .attr('y', yPos - 5)
           .text(format(stacked ? totalStackedValues[index] : d.y))
           .attr('transform', transformAttr)
           .attr('class', 'bar-chart-label');
-        const labelWidth = t.node().getBBox().width;
-        t.attr('x', xPos + rectWidth / 2 - labelWidth / 2); // fine tune
+        const labelWidth = textEls.node().getBBox().width;
+        textEls.attr('x', xPos + rectWidth / 2 - labelWidth / 2); // fine tune
       }
     });
 };
@@ -79,8 +81,8 @@ function getMaxLabelSize(container, axisClass) {
   // axis class = .nv-y2  // second y axis on dual line chart
   // axis class = .nv-x  // x axis on time series line chart
   const labelEls = container.find(`.${axisClass} text`).not('.nv-axislabel');
-  const labelDimensions = labelEls.map(i => labelEls[i].getComputedTextLength());
-  return Math.max(...labelDimensions);
+  const labelDimensions = labelEls.map(i => labelEls[i].getComputedTextLength() * 0.75);
+  return Math.ceil(Math.max(...labelDimensions));
 }
 
 /* eslint-disable camelcase */
@@ -116,22 +118,6 @@ function nvd3Vis(slice, payload) {
   slice.container.html('');
   slice.clearError();
 
-
-  // Calculates the longest label size for stretching bottom margin
-  function calculateStretchMargins(payloadData) {
-    let stretchMargin = 0;
-    const pixelsPerCharX = 4.5; // approx, depends on font size
-    let maxLabelSize = 10; // accommodate for shorter labels
-    payloadData.data.forEach((d) => {
-      const axisLabels = d.values;
-      for (let i = 0; i < axisLabels.length; i++) {
-        maxLabelSize = Math.max(axisLabels[i].x.toString().length, maxLabelSize);
-      }
-    });
-    stretchMargin = Math.ceil(pixelsPerCharX * maxLabelSize);
-    return stretchMargin;
-  }
-
   let width = slice.width();
   const fd = slice.formData;
 
@@ -160,16 +146,43 @@ function nvd3Vis(slice, payload) {
       svg = d3.select(slice.selector).append('svg');
     }
     let height = slice.height();
+    const isTimeSeries = [
+      'line', 'dual_line', 'area', 'compare', 'bar', 'time_pivot'].indexOf(vizType) >= 0;
+
+    // Handling xAxis ticks settings
+    let xLabelRotation = 0;
+    let staggerLabels = false;
+    if (fd.x_ticks_layout === 'auto') {
+      if (['column', 'dist_bar'].indexOf(vizType) >= 0) {
+        xLabelRotation = 45;
+      } else if (isTimeSeries) {
+        staggerLabels = true;
+      }
+    } else if (fd.x_ticks_layout === 'staggered') {
+      staggerLabels = true;
+    } else if (fd.x_ticks_layout === '45°') {
+      if (isTruthy(fd.show_brush)) {
+        const error = t('You cannot use 45° tick layout along with the time range filter');
+        slice.error(error);
+        return null;
+      }
+      xLabelRotation = 45;
+    }
+    const showBrush = (
+      isTruthy(fd.show_brush) ||
+      (fd.show_brush === 'auto' && height >= minHeightForBrush && fd.x_ticks_layout !== '45°')
+    );
+
     switch (vizType) {
       case 'line':
-        if (
-          fd.show_brush === true ||
-          fd.show_brush === 'yes' ||
-          (fd.show_brush === 'auto' && height >= minHeightForBrush)
-        ) {
+        if (showBrush) {
           chart = nv.models.lineWithFocusChart();
+          if (staggerLabels) {
+            // Give a bit more room to focus area if X axis ticks are staggered
+            chart.focus.margin({ bottom: 40 });
+            chart.focusHeight(80);
+          }
           chart.focus.xScale(d3.time.scale.utc());
-          chart.x2Axis.staggerLabels(false);
         } else {
           chart = nv.models.lineChart();
         }
@@ -177,14 +190,12 @@ function nvd3Vis(slice, payload) {
         // chart.interactiveLayer.tooltip.headerFormatter(function(){return '';});
         chart.xScale(d3.time.scale.utc());
         chart.interpolate(fd.line_interpolation);
-        chart.xAxis.staggerLabels(false);
         break;
 
       case 'time_pivot':
         chart = nv.models.lineChart();
         chart.xScale(d3.time.scale.utc());
         chart.interpolate(fd.line_interpolation);
-        chart.xAxis.staggerLabels(false);
         break;
 
       case 'dual_line':
@@ -202,8 +213,7 @@ function nvd3Vis(slice, payload) {
         }
         chart.width(width);
         chart.xAxis
-        .showMaxMin(false)
-        .staggerLabels(true);
+        .showMaxMin(false);
 
         stacked = fd.bar_stacked;
         chart.stacked(stacked);
@@ -219,7 +229,6 @@ function nvd3Vis(slice, payload) {
         chart = nv.models.multiBarChart()
         .showControls(fd.show_controls)
         .reduceXTicks(reduceXTicks)
-        .rotateLabels(45)
         .groupSpacing(0.1); // Distance between each group of bars.
 
         chart.xAxis.showMaxMin(false);
@@ -271,17 +280,14 @@ function nvd3Vis(slice, payload) {
 
       case 'column':
         chart = nv.models.multiBarChart()
-        .reduceXTicks(false)
-        .rotateLabels(45);
+        .reduceXTicks(false);
         break;
 
       case 'compare':
         chart = nv.models.cumulativeLineChart();
         chart.xScale(d3.time.scale.utc());
         chart.useInteractiveGuideline(true);
-        chart.xAxis
-        .showMaxMin(false)
-        .staggerLabels(true);
+        chart.xAxis.showMaxMin(false);
         break;
 
       case 'bubble':
@@ -311,14 +317,12 @@ function nvd3Vis(slice, payload) {
         chart.showControls(fd.show_controls);
         chart.style(fd.stacked_style);
         chart.xScale(d3.time.scale.utc());
-        chart.xAxis.staggerLabels(true);
         break;
 
       case 'box_plot':
         colorKey = 'label';
         chart = nv.models.boxPlotChart();
         chart.x(d => d.label);
-        chart.staggerLabels(true);
         chart.maxBoxWidth(75); // prevent boxes from being incredibly wide
         break;
 
@@ -330,6 +334,19 @@ function nvd3Vis(slice, payload) {
         throw new Error('Unrecognized visualization for nvd3' + vizType);
     }
 
+    if (chart.xAxis && chart.xAxis.staggerLabels) {
+      chart.xAxis.staggerLabels(staggerLabels);
+    }
+    if (chart.xAxis && chart.xAxis.rotateLabels) {
+      chart.xAxis.rotateLabels(xLabelRotation);
+    }
+    if (chart.x2Axis && chart.x2Axis.staggerLabels) {
+      chart.x2Axis.staggerLabels(staggerLabels);
+    }
+    if (chart.x2Axis && chart.x2Axis.rotateLabels) {
+      chart.x2Axis.rotateLabels(xLabelRotation);
+    }
+
     if ('showLegend' in chart && typeof fd.show_legend !== 'undefined') {
       if (width < BREAKPOINTS.small && vizType !== 'pie') {
         chart.showLegend(false);
@@ -342,9 +359,6 @@ function nvd3Vis(slice, payload) {
       height = Math.min(height, 50);
     }
 
-    chart.height(height);
-    slice.container.css('height', height + 'px');
-
     if (chart.forceY &&
         fd.y_axis_bounds &&
         (fd.y_axis_bounds[0] !== null || fd.y_axis_bounds[1] !== null)) {
@@ -356,12 +370,6 @@ function nvd3Vis(slice, payload) {
     if (fd.x_log_scale) {
       chart.xScale(d3.scale.log());
     }
-    const isTimeSeries = [
-      'line', 'dual_line', 'area', 'compare', 'bar', 'time_pivot'].indexOf(vizType) >= 0;
-    // if x axis format is a date format, rotate label 90 degrees
-    if (isTimeSeries) {
-      chart.xAxis.rotateLabels(45);
-    }
 
     let xAxisFormatter = d3FormatPreset(fd.x_axis_format);
     if (isTimeSeries) {
@@ -369,7 +377,6 @@ function nvd3Vis(slice, payload) {
     }
     if (chart.x2Axis && chart.x2Axis.tickFormat) {
       chart.x2Axis.tickFormat(xAxisFormatter);
-      height += 30;
     }
     const isXAxisString = ['dist_bar', 'box_plot'].indexOf(vizType) >= 0;
     if (!isXAxisString && chart.xAxis && chart.xAxis.tickFormat) {
@@ -392,6 +399,7 @@ function nvd3Vis(slice, payload) {
       }
     }
     setAxisShowMaxMin(chart.xAxis, fd.x_axis_showminmax);
+    setAxisShowMaxMin(chart.x2Axis, fd.x_axis_showminmax);
     setAxisShowMaxMin(chart.yAxis, fd.y_axis_showminmax);
     setAxisShowMaxMin(chart.y2Axis, fd.y_axis_showminmax);
 
@@ -436,18 +444,6 @@ function nvd3Vis(slice, payload) {
       }
     }
 
-
-    if (fd.bottom_margin === 'auto') {
-      if (vizType === 'dist_bar') {
-        const stretchMargin = calculateStretchMargins(payload);
-        chart.margin({ bottom: stretchMargin });
-      } else {
-        chart.margin({ bottom: 50 });
-      }
-    } else {
-      chart.margin({ bottom: fd.bottom_margin });
-    }
-
     if (vizType === 'dual_line') {
       const yAxisFormatter1 = d3.format(fd.y_axis_format);
       const yAxisFormatter2 = d3.format(fd.y_axis_2_format);
@@ -456,6 +452,9 @@ function nvd3Vis(slice, payload) {
       customizeToolTip(chart, xAxisFormatter, [yAxisFormatter1, yAxisFormatter2]);
       chart.showLegend(width > BREAKPOINTS.small);
     }
+    chart.height(height);
+    slice.container.css('height', height + 'px');
+
     svg
     .datum(data)
     .transition().duration(500)
@@ -472,8 +471,9 @@ function nvd3Vis(slice, payload) {
     if (chart.yAxis !== undefined || chart.yAxis2 !== undefined) {
       // Hack to adjust y axis left margin to accommodate long numbers
       const containerWidth = slice.container.width();
-      const marginPad = Math.min(isExplore ? containerWidth * 0.01 : containerWidth * 0.03,
-        maxMarginPad);
+      const marginPad = Math.ceil(
+        Math.min(isExplore ? containerWidth * 0.01 : containerWidth * 0.03, maxMarginPad),
+      );
       const maxYAxisLabelWidth = chart.yAxis2 ? getMaxLabelSize(slice.container, 'nv-y1')
                                               : getMaxLabelSize(slice.container, 'nv-y');
       const maxXAxisLabelHeight = getMaxLabelSize(slice.container, 'nv-x');
@@ -486,36 +486,35 @@ function nvd3Vis(slice, payload) {
       // - measure the width or height of the labels
       // ---- (x axis labels are rotated 45 degrees so we use height),
       // - adjust margins based on these measures and render again
-      if (isTimeSeries && vizType !== 'bar') {
-        const chartMargins = {
-          bottom: maxXAxisLabelHeight + marginPad,
-          right: maxXAxisLabelHeight + marginPad,
-        };
-
-        if (vizType === 'dual_line') {
-          const maxYAxis2LabelWidth = getMaxLabelSize(slice.container, 'nv-y2');
-          // use y axis width if it's wider than axis width/height
-          if (maxYAxis2LabelWidth > maxXAxisLabelHeight) {
-            chartMargins.right = maxYAxis2LabelWidth + marginPad;
-          }
-        }
-        // apply margins
-        chart.margin(chartMargins);
+      const margins = chart.margin();
+      margins.bottom = 20;
+      if (fd.x_axis_showminmax) {
+        // If x bounds are shown, we need a right margin
+        margins.right = Math.max(20, maxXAxisLabelHeight / 2) + marginPad;
+      }
+      if (xLabelRotation === 45) {
+        margins.bottom = maxXAxisLabelHeight + marginPad;
+        margins.right = maxXAxisLabelHeight + marginPad;
+      } else if (staggerLabels) {
+        margins.bottom = 30;
       }
 
-      if (fd.x_axis_label && fd.x_axis_label !== '' && chart.xAxis) {
-        chart.margin({ bottom: maxXAxisLabelHeight + marginPad + 25 });
+      if (vizType === 'dual_line') {
+        const maxYAxis2LabelWidth = getMaxLabelSize(slice.container, 'nv-y2');
+        // use y axis width if it's wider than axis width/height
+        if (maxYAxis2LabelWidth > maxXAxisLabelHeight) {
+          margins.right = maxYAxis2LabelWidth + marginPad;
+        }
       }
       if (fd.bottom_margin && fd.bottom_margin !== 'auto') {
-        chart.margin().bottom = fd.bottom_margin;
+        margins.bottom = parseInt(fd.bottom_margin, 10);
       }
       if (fd.left_margin && fd.left_margin !== 'auto') {
-        chart.margin().left = fd.left_margin;
+        margins.left = fd.left_margin;
       }
 
-      // Axis labels
-      const margins = chart.margin();
       if (fd.x_axis_label && fd.x_axis_label !== '' && chart.xAxis) {
+        margins.bottom += 25;
         let distance = 0;
         if (margins.bottom && !isNaN(margins.bottom)) {
           distance = margins.bottom - 45;


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services