Make sure input controls can have their min and max bounds set manually when they are initially blank (#16947) (#17041)

* Fix one bug with input controls

* Fixed the bug, without tests.

Unfortunately adding a jest test for this would require a bit of
refactoring which I think would fit better in a separate PR, and might
not be worthwhile right now…. so just fixing it to get it in.

* remove left over console log

src/core_plugins/input_control_vis/public/components/vis/__snapshots__/range_control.test.js.snap

@@ -26,6 +26,7 @@ exports[`renders RangeControl 1`] = `
   label="range control"
 >
   <EuiFormRow
+    data-test-subj="rangeControlFormRow"
     error={Array []}
     fullWidth={false}
     hasEmptyLabelSpace={false}
@@ -45,6 +46,7 @@ exports[`renders RangeControl 1`] = `
       >
         <input
           className="euiFieldNumber"
+          data-test-subj="rangeControlMinInputValue"
           disabled={false}
           id="mock-range-control_min"
           max={100}
@@ -99,6 +101,7 @@ exports[`renders RangeControl 1`] = `
       >
         <input
           className="euiFieldNumber"
+          data-test-subj="rangeControlMaxInputValue"
           disabled={false}
           id="mock-range-control_max"
           max={100}

src/core_plugins/input_control_vis/public/components/vis/range_control.js

@@ -14,10 +14,9 @@ const toState = (props) => {
   const state = {
     sliderValue: props.control.value,
     minValue: '',
-    isMinValid: true,
     maxValue: '',
-    isMaxValid: true,
-    errorMsgs: [],
+    isRangeValid: true,
+    errorMessage: '',
   };
   if (props.control.hasValue()) {
     state.minValue = props.control.value.min;
@@ -35,94 +34,60 @@ export class RangeControl extends Component {
 
   componentWillReceiveProps = (nextProps) => {
     this.setState(toState(nextProps));
-  }
+  };
 
   handleOnChange = (value) => {
     this.setState({
       sliderValue: value,
       minValue: value.min,
-      isMinValid: true,
+      isRangeValid: true,
       maxValue: value.max,
-      isMaxValid: true,
-      errorMsgs: [],
+      errorMessage: '',
     });
-  }
+  };
 
   handleOnChangeComplete = (value) => {
     this.props.stageFilter(this.props.controlIndex, value);
-  }
+  };
 
   handleMinChange = (evt) => {
-    const min = parseFloat(evt.target.value);
-    const max = this.state.maxValue;
-    if (isNaN(min)) {
-      if (max === '') {
-        this.setState({
-          minValue: '',
-          isMinValid: true,
-          maxValue: '',
-          isMaxValid: true,
-          errorMsgs: [],
-        });
-        return;
-      }
-      this.setState({
-        minValue: '',
-        isMinValid: false,
-        errorMsgs: ['both min and max must be set'],
-      });
-      return;
-
-    } else if (min > max) {
-      this.setState({
-        minValue: min,
-        isMinValid: false,
-        errorMsgs: ['min must be less than max'],
-      });
-      return;
-    }
-
-    this.handleOnChangeComplete({
-      min: min,
-      max: max
-    });
-  }
+    this.handleChange(parseFloat(evt.target.value), this.state.maxValue);
+  };
 
   handleMaxChange = (evt) => {
-    const min = this.state.minValue;
-    const max = parseFloat(evt.target.value);
-    if (isNaN(max)) {
-      if (min === '') {
-        this.setState({
-          minValue: '',
-          isMinValid: true,
-          maxValue: '',
-          isMaxValid: true,
-          errorMsgs: [],
-        });
-        return;
-      }
-      this.setState({
-        maxValue: '',
-        isMaxValid: false,
-        errorMsgs: ['both min and max must be set'],
-      });
-      return;
+    this.handleChange(this.state.minValue, parseFloat(evt.target.value));
+  };
 
-    } else if (max < min) {
-      this.setState({
-        maxValue: max,
-        isMaxValid: false,
-        errorMsgs: ['max must be greater than min'],
-      });
-      return;
+  handleChange = (min, max) => {
+    min = isNaN(min) ? '' : min;
+    max = isNaN(max) ? '' : max;
+
+    const isMinValid = min !== '';
+    const isMaxValid = max !== '';
+    let isRangeValid = true;
+    let errorMessage = '';
+
+    if ((!isMinValid && isMaxValid) || (isMinValid && !isMaxValid)) {
+      isRangeValid = false;
+      errorMessage = 'both min and max must be set';
     }
 
-    this.handleOnChangeComplete({
-      min: min,
-      max: max
+    if (isMinValid && isMaxValid && max < min) {
+      isRangeValid = false;
+      errorMessage = 'max must be greater or equal to min';
+    }
+
+    this.setState({
+      minValue: min,
+      maxValue: max,
+      isRangeValid,
+      errorMessage,
     });
-  }
+
+    if (isRangeValid && isMaxValid && isMinValid) {
+      this.handleOnChangeComplete({ min, max });
+    }
+  };
 
   formatLabel = (value) => {
     let formatedValue = value;
@@ -131,13 +96,14 @@ export class RangeControl extends Component {
       formatedValue = value.toFixed(decimalPlaces);
     }
     return formatedValue;
-  }
+  };
 
   renderControl() {
     return (
       <EuiFormRow
-        isInvalid={!this.state.isMinValid || !this.state.isMaxValid}
-        error={this.state.errorMsgs}
+        isInvalid={!this.state.isRangeValid}
+        error={this.state.errorMessage ? [this.state.errorMessage] : []}
+        data-test-subj="rangeControlFormRow"
       >
         <EuiFlexGroup>
           <EuiFlexItem grow={false}>
@@ -146,6 +112,7 @@ export class RangeControl extends Component {
               disabled={!this.props.control.isEnabled()}
               name="min"
               type="number"
+              data-test-subj="rangeControlMinInputValue"
               className="euiFieldNumber"
               value={this.state.minValue}
               min={this.props.control.min}
@@ -174,6 +141,7 @@ export class RangeControl extends Component {
               name="max"
               type="number"
               className="euiFieldNumber"
+              data-test-subj="rangeControlMaxInputValue"
               value={this.state.maxValue}
               min={this.props.control.min}
               max={this.props.control.max}

src/core_plugins/input_control_vis/public/components/vis/range_control.test.js

@@ -1,6 +1,6 @@
 import React from 'react';
-import sinon from 'sinon';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
+import { findTestSubject } from '@elastic/eui/lib/test';
 
 import {
   RangeControl,
@@ -22,17 +22,69 @@ const control = {
     return false;
   }
 };
-let stageFilter;
-
-beforeEach(() => {
-  stageFilter = sinon.spy();
-});
 
 test('renders RangeControl', () => {
   const component = shallow(<RangeControl
     control={control}
     controlIndex={0}
-    stageFilter={stageFilter}
+    stageFilter={() => {}}
   />);
   expect(component).toMatchSnapshot(); // eslint-disable-line
 });
+
+describe('min and max input values', () => {
+  const component = mount(<RangeControl
+    control={control}
+    controlIndex={0}
+    stageFilter={() => {}}
+  />);
+
+  const BOTH_MIN_AND_MAX_MUST_BE_SET_ERROR = 'both min and max must be set';
+
+  const getMinInput = () => findTestSubject(component, 'rangeControlMinInputValue');
+  const getMaxInput = () => findTestSubject(component, 'rangeControlMaxInputValue');
+  const getRangeRow = () => findTestSubject(component, 'rangeControlFormRow');
+
+  test('are initially blank', () => {
+    expect(getMinInput().props().value).toBe('');
+    expect(getMaxInput().props().value).toBe('');
+  });
+
+  test('min can be set manually', () => {
+    getMinInput().simulate('change', { target: { value: 3 } });
+    expect(getMinInput().props().value).toBe(3);
+  });
+
+  test('when only min is specified an error is shown', () => {
+    expect(getRangeRow().text().indexOf(BOTH_MIN_AND_MAX_MUST_BE_SET_ERROR)).toBeGreaterThan(-1);
+  });
+
+  test('max can be set manually', () => {
+    getMaxInput().simulate('change', { target: { value: 6 } });
+    expect(getMaxInput().props().value).toBe(6);
+  });
+
+  test('when both min and max are set there is no error', () => {
+    expect(getRangeRow().text().indexOf(BOTH_MIN_AND_MAX_MUST_BE_SET_ERROR)).toBe(-1);
+  });
+
+  test('0 is a valid minimum value', () => {
+    getMinInput().simulate('change', { target: { value: 0 } });
+    expect(getMinInput().props().value).toBe(0);
+    expect(getRangeRow().text().indexOf(BOTH_MIN_AND_MAX_MUST_BE_SET_ERROR)).toBe(-1);
+  });
+
+  test('min can be deleted and there will be an error shown', () => {
+    getMinInput().simulate('change', { target: { value: '' } });
+    expect(getMinInput().props().value).toBe('');
+    expect(getRangeRow().text().indexOf(BOTH_MIN_AND_MAX_MUST_BE_SET_ERROR)).toBeGreaterThan(-1);
+  });
+
+  test('both max and min can be deleted and there will not be an error shown', () => {
+    getMaxInput().simulate('change', { target: { value: '' } });
+    expect(getMaxInput().props().value).toBe('');
+    expect(getRangeRow().text().indexOf(BOTH_MIN_AND_MAX_MUST_BE_SET_ERROR)).toBe(-1);
+  });
+});
+
+