[explore] adding y_axis_bounds to force Y axis bounds (#2878)

* [explore] adding y_axis_bounds to force Y axis

* Handling comments

superset/assets/javascripts/explore/components/Control.jsx

@@ -8,6 +8,7 @@ import SelectControl from './controls/SelectControl';
 import TextAreaControl from './controls/TextAreaControl';
 import TextControl from './controls/TextControl';
 import VizTypeControl from './controls/VizTypeControl';
+import BoundsControl from './controls/BoundsControl';
 
 const controlMap = {
   CheckboxControl,
@@ -17,6 +18,7 @@ const controlMap = {
   TextAreaControl,
   TextControl,
   VizTypeControl,
+  BoundsControl,
 };
 const controlTypes = Object.keys(controlMap);
 

superset/assets/javascripts/explore/components/ControlPanelsContainer.jsx

@@ -19,7 +19,6 @@ const propTypes = {
   controls: PropTypes.object.isRequired,
   form_data: PropTypes.object.isRequired,
   isDatasourceMetaLoading: PropTypes.bool.isRequired,
-  y_axis_zero: PropTypes.any,
 };
 
 class ControlPanelsContainer extends React.Component {
@@ -65,14 +64,15 @@ class ControlPanelsContainer extends React.Component {
                 <ControlRow
                   key={`controlsetrow-${i}`}
                   controls={controlSets.map(controlName => (
-                    <Control
+                    controlName &&
-                      name={controlName}
+                      <Control
-                      key={`control-${controlName}`}
+                        name={controlName}
-                      value={this.props.form_data[controlName]}
+                        key={`control-${controlName}`}
-                      validationErrors={this.props.controls[controlName].validationErrors}
+                        value={this.props.form_data[controlName]}
-                      actions={this.props.actions}
+                        validationErrors={this.props.controls[controlName].validationErrors}
-                      {...this.getControlData(controlName)}
+                        actions={this.props.actions}
-                    />
+                        {...this.getControlData(controlName)}
+                      />
                   ))}
                 />
               ))}

superset/assets/javascripts/explore/components/controls/BoundsControl.jsx

@@ -0,0 +1,95 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { Col, Row, FormGroup, FormControl } from 'react-bootstrap';
+import ControlHeader from '../ControlHeader';
+
+const propTypes = {
+  name: PropTypes.string.isRequired,
+  label: PropTypes.string,
+  description: PropTypes.string,
+  onChange: PropTypes.func,
+  value: PropTypes.array,
+};
+
+const defaultProps = {
+  label: null,
+  description: null,
+  onChange: () => {},
+  value: [null, null],
+};
+
+export default class BoundsControl extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      minMax: [
+        props.value[0] === null ? '' : props.value[0],
+        props.value[1] === null ? '' : props.value[1],
+      ],
+    };
+    this.onChange = this.onChange.bind(this);
+    this.onMinChange = this.onMinChange.bind(this);
+    this.onMaxChange = this.onMaxChange.bind(this);
+  }
+  onMinChange(event) {
+    this.setState({
+      minMax: [
+        event.target.value,
+        this.state.minMax[1],
+      ],
+    }, this.onChange);
+  }
+  onMaxChange(event) {
+    this.setState({
+      minMax: [
+        this.state.minMax[0],
+        event.target.value,
+      ],
+    }, this.onChange);
+  }
+  onChange() {
+    const mm = this.state.minMax;
+    const errors = [];
+    if (mm[0] && isNaN(mm[0])) {
+      errors.push('`Min` value should be numeric or empty');
+    }
+    if (mm[1] && isNaN(mm[1])) {
+      errors.push('`Max` value should be numeric or empty');
+    }
+    if (errors.length === 0) {
+      this.props.onChange([parseFloat(mm[0]), parseFloat(mm[1])], errors);
+    } else {
+      this.props.onChange([null, null], errors);
+    }
+  }
+  render() {
+    return (
+      <div>
+        <ControlHeader {...this.props} />
+        <FormGroup bsSize="small">
+          <Row>
+            <Col xs={6}>
+              <FormControl
+                type="text"
+                placeholder="Min"
+                onChange={this.onMinChange}
+                value={this.state.minMax[0]}
+              />
+            </Col>
+            <Col xs={6}>
+              <FormControl
+                type="text"
+                placeholder="Max"
+                onChange={this.onMaxChange}
+                value={this.state.minMax[1]}
+              />
+            </Col>
+          </Row>
+        </FormGroup>
+      </div>
+    );
+  }
+}
+
+BoundsControl.propTypes = propTypes;
+BoundsControl.defaultProps = defaultProps;

superset/assets/javascripts/explore/components/controls/VizTypeControl.jsx

@@ -63,7 +63,7 @@ export default class VizTypeControl extends React.PureComponent {
     const rows = [];
     for (let i = 0; i <= filteredVizTypes.length; i += imgPerRow) {
       rows.push(
-        <Row>
+        <Row key={`row-${i}`}>
           {filteredVizTypes.slice(i, i + imgPerRow).map(vt => (
             <Col md={3} key={`grid-col-${vt}`}>
               {this.renderVizType(vt)}

superset/assets/javascripts/explore/stores/controls.jsx

@@ -65,7 +65,17 @@ export const controls = {
     }),
     description: 'One or many metrics to display',
   },
-
+  y_axis_bounds: {
+    type: 'BoundsControl',
+    label: 'Y Axis Bounds',
+    default: [null, null],
+    description: (
+      'Bounds for the Y axis. When left empty, the bounds are ' +
+      'dynamically defined based on the min/max of the data. Note that ' +
+      "this feature will only expand the axis range. It won't " +
+      "narrow the data's extent."
+    ),
+  },
   order_by_cols: {
     type: 'SelectControl',
     multi: true,
@@ -751,7 +761,7 @@ export const controls = {
   x_axis_format: {
     type: 'SelectControl',
     freeForm: true,
-    label: 'X axis format',
+    label: 'X Axis Format',
     renderTrigger: true,
     default: 'smart_date',
     choices: TIME_STAMP_OPTIONS,
@@ -761,7 +771,7 @@ export const controls = {
   y_axis_format: {
     type: 'SelectControl',
     freeForm: true,
-    label: 'Y axis format',
+    label: 'Y Axis Format',
     renderTrigger: true,
     default: '.3s',
     choices: D3_TIME_FORMAT_OPTIONS,
@@ -771,7 +781,7 @@ export const controls = {
   y_axis_2_format: {
     type: 'SelectControl',
     freeForm: true,
-    label: 'Right axis format',
+    label: 'Right Axis Format',
     default: '.3s',
     choices: D3_TIME_FORMAT_OPTIONS,
     description: D3_FORMAT_DOCS,
@@ -937,14 +947,6 @@ export const controls = {
     'point in time',
   },
 
-  y_axis_zero: {
-    type: 'CheckboxControl',
-    label: 'Y Axis Zero',
-    default: false,
-    renderTrigger: true,
-    description: 'Force the Y axis to start at 0 instead of the minimum value',
-  },
-
   y_log_scale: {
     type: 'CheckboxControl',
     label: 'Y Log Scale',

superset/assets/javascripts/explore/stores/store.js

@@ -19,6 +19,16 @@ export function getControlNames(vizType, datasourceType) {
   return controlNames;
 }
 
+function handleDeprecatedControls(formData) {
+  // Reacffectation / handling of deprecated controls
+  /* eslint-disable no-param-reassign */
+
+  // y_axis_zero was a boolean forcing 0 to be part of the Y Axis
+  if (formData.y_axis_zero) {
+    formData.y_axis_bounds = [0, null];
+  }
+}
+
 export function getControlsState(state, form_data) {
   /*
   * Gets a new controls object to put in the state. The controls object
@@ -32,6 +42,8 @@ export function getControlsState(state, form_data) {
   const formData = Object.assign({}, form_data);
   const vizType = formData.viz_type || 'table';
 
+  handleDeprecatedControls(formData);
+
   const controlNames = getControlNames(vizType, state.datasource.type);
 
   const viz = visTypes[vizType];

superset/assets/javascripts/explore/stores/visTypes.js

@@ -128,12 +128,17 @@ const visTypes = {
         label: 'Chart Options',
         controlSetRows: [
           ['show_brush', 'show_legend'],
-          ['rich_tooltip', 'y_axis_zero'],
+          ['rich_tooltip', null],
-          ['y_log_scale', 'contribution'],
           ['show_markers', 'x_axis_showminmax'],
-          ['line_interpolation'],
+          ['line_interpolation', 'contribution'],
-          ['x_axis_format', 'y_axis_format'],
+        ],
-          ['x_axis_label', 'y_axis_label'],
+      },
+      {
+        label: 'Axes',
+        controlSetRows: [
+          ['x_axis_label', 'x_axis_format'],
+          ['y_axis_label', 'y_axis_bounds'],
+          ['y_axis_format', 'y_log_scale'],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -185,13 +190,18 @@ const visTypes = {
         label: 'Chart Options',
         controlSetRows: [
           ['show_brush', 'show_legend', 'show_bar_value'],
-          ['rich_tooltip', 'y_axis_zero'],
+          ['rich_tooltip', 'contribution'],
-          ['y_log_scale', 'contribution'],
-          ['x_axis_format', 'y_axis_format'],
           ['line_interpolation', 'bar_stacked'],
-          ['x_axis_showminmax', 'bottom_margin'],
+          ['bottom_margin', 'show_controls'],
+        ],
+      },
+      {
+        label: 'Axes',
+        controlSetRows: [
+          ['x_axis_format', 'y_axis_format'],
+          ['x_axis_showminmax', 'reduce_x_ticks'],
           ['x_axis_label', 'y_axis_label'],
-          ['reduce_x_ticks', 'show_controls'],
+          ['y_axis_bounds', 'y_log_scale'],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -222,11 +232,17 @@ const visTypes = {
         label: 'Chart Options',
         controlSetRows: [
           ['show_brush', 'show_legend'],
-          ['rich_tooltip', 'y_axis_zero'],
-          ['y_log_scale', 'contribution'],
-          ['x_axis_format', 'y_axis_format'],
-          ['x_axis_showminmax', 'show_controls'],
           ['line_interpolation', 'stacked_style'],
+          ['rich_tooltip', 'contribution'],
+          ['show_controls', null],
+        ],
+      },
+      {
+        label: 'Axes',
+        controlSetRows: [
+          ['x_axis_format', 'x_axis_showminmax'],
+          ['y_axis_format', 'y_axis_bounds'],
+          ['y_log_scale', null],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -401,10 +417,9 @@ const visTypes = {
       {
         label: 'Chart Options',
         controlSetRows: [
-          ['x_log_scale', 'y_log_scale'],
+          ['show_legend', 'max_bubble_size'],
-          ['show_legend'],
-          ['max_bubble_size'],
           ['x_axis_label', 'y_axis_label'],
+          ['x_log_scale', 'y_log_scale'],
         ],
       },
     ],

superset/assets/spec/javascripts/explorev2/components/BoundsControl_spec.jsx

@@ -0,0 +1,38 @@
+/* eslint-disable no-unused-expressions */
+import React from 'react';
+import { FormControl } from 'react-bootstrap';
+import sinon from 'sinon';
+import { expect } from 'chai';
+import { describe, it, beforeEach } from 'mocha';
+import { mount } from 'enzyme';
+
+import BoundsControl from '../../../../javascripts/explore/components/controls/BoundsControl';
+
+const defaultProps = {
+  name: 'y_axis_bounds',
+  label: 'Bounds of the y axis',
+  onChange: sinon.spy(),
+};
+
+describe('BoundsControl', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    wrapper = mount(<BoundsControl {...defaultProps} />);
+  });
+
+  it('renders two FormControls', () => {
+    expect(wrapper.find(FormControl)).to.have.lengthOf(2);
+  });
+
+  it('errors on non-numeric', () => {
+    wrapper.find(FormControl).first().simulate('change', { target: { value: 's' } });
+    expect(defaultProps.onChange.calledWith([null, null])).to.be.true;
+    expect(defaultProps.onChange.getCall(0).args[1][0]).to.contain('value should be numeric');
+  });
+  it('casts to numeric', () => {
+    wrapper.find(FormControl).first().simulate('change', { target: { value: '1' } });
+    wrapper.find(FormControl).last().simulate('change', { target: { value: '5' } });
+    expect(defaultProps.onChange.calledWith([1, 5])).to.be.true;
+  });
+});

superset/assets/visualizations/nvd3_vis.js

@@ -306,9 +306,12 @@ function nvd3Vis(slice, payload) {
     if ((vizType === 'line' || vizType === 'area') && fd.rich_tooltip) {
       chart.useInteractiveGuideline(true);
     }
-    if (fd.y_axis_zero) {
+    if (chart.forceY &&
-      chart.forceY([0]);
+        fd.y_axis_bounds &&
-    } else if (fd.y_log_scale) {
+        (fd.y_axis_bounds[0] !== null || fd.y_axis_bounds[1] !== null)) {
+      chart.forceY(fd.y_axis_bounds);
+    }
+    if (fd.y_log_scale) {
       chart.yScale(d3.scale.log());
     }
     if (fd.x_log_scale) {