From 997041ede3fd4376f349545d739496878a0624b3 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Tue, 10 Apr 2018 13:37:34 -0700 Subject: [PATCH] Improve xAxis ticks, thinner bottom margin (#4756) * Improve xAxis ticks, thinner bottom margin * Moving utils folder * Add isTruthy --- .../SqlLab/components/CopyQueryTabUrl.jsx | 2 +- .../SqlLab/components/QueryTable.jsx | 2 +- superset/assets/javascripts/chart/Chart.jsx | 9 +- .../explore/components/SaveModal.jsx | 2 +- .../explore/components/URLShortLinkButton.jsx | 2 +- .../javascripts/explore/stores/controls.jsx | 10 ++ .../javascripts/explore/stores/visTypes.js | 77 +++++--- .../assets/{ => javascripts}/utils/common.js | 9 + .../{ => javascripts}/utils/reducerUtils.js | 0 .../spec/javascripts/explore/utils_spec.jsx | 2 +- .../spec/javascripts/utils/common_spec.jsx | 43 +++++ superset/assets/visualizations/mapbox.jsx | 2 +- superset/assets/visualizations/nvd3_vis.js | 167 +++++++++--------- 13 files changed, 213 insertions(+), 114 deletions(-) rename superset/assets/{ => javascripts}/utils/common.js (92%) rename superset/assets/{ => javascripts}/utils/reducerUtils.js (100%) create mode 100644 superset/assets/spec/javascripts/utils/common_spec.jsx diff --git a/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx b/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx index 66e60c9b2b4f7..fed7c28bbe9a7 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 01d490277ea9b..30d158b71896e 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 4b4410640abe1..fb9884bfbb146 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 4aa767046670c..90bf12bc9902a 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 aa278b7a465ce..a0c0078ad426c 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 7e82e07e28933..d2efd53fe9266 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 8f52020ed872b..d1807aeb9db85 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 c7be5ddbb4760..f2c3bd24ceff7 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 fe18f3907335b..9f855d2f398bf 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 0000000000000..a3490d476e0d4 --- /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 baea634285a56..0c01d6b19eadf 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 5887d795855c6..eb5b70187e702 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)); } export function formatLabel(input, verboseMap = {}) { @@ -117,22 +119,6 @@ export default 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; @@ -161,16 +147,43 @@ export default 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(); } @@ -178,14 +191,12 @@ export default 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': @@ -203,8 +214,7 @@ export default function nvd3Vis(slice, payload) { } chart.width(width); chart.xAxis - .showMaxMin(false) - .staggerLabels(true); + .showMaxMin(false); stacked = fd.bar_stacked; chart.stacked(stacked); @@ -220,7 +230,6 @@ export default 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); @@ -272,17 +281,14 @@ export default 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': @@ -312,14 +318,12 @@ export default 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; @@ -331,6 +335,19 @@ export default 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); @@ -343,9 +360,6 @@ export default 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)) { @@ -357,12 +371,6 @@ export default 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) { @@ -370,7 +378,6 @@ export default 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) { @@ -398,6 +405,7 @@ export default 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); @@ -442,18 +450,6 @@ export default 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); @@ -462,6 +458,9 @@ export default 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) @@ -478,8 +477,9 @@ export default 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'); @@ -492,36 +492,35 @@ export default 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;