fix(Slider): check validity of number input after onChange (#9132)

* feat(slider): check input validity after changes * fix: slider invalid padding * fix: story example for testing * fix: invalid prop state sync * fix: update derivedStateFromProps * chore: update tests * fix: derivedStateFromProps return null * fix: set validity as true for drag and key evt * fix: update tests Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
carbon-design-system · Sep 13, 2021 · 641d0ec · 641d0ec
1 parent ee69d34
commit 641d0ec
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 22 deletions.
diff --git a/packages/components/src/components/slider/_slider.scss b/packages/components/src/components/slider/_slider.scss
@@ -128,6 +128,10 @@
     }
   }
 
+  .#{$prefix}--slider-text-input.#{$prefix}--text-input--invalid {
+    padding-right: 1rem;
+  }
+
   .#{$prefix}--slider__thumb:focus ~ .#{$prefix}--slider__filled-track {
     background-color: $interactive-04;
   }

diff --git a/packages/react/src/components/Slider/Slider-story.js b/packages/react/src/components/Slider/Slider-story.js
@@ -58,7 +58,7 @@ export const Default = () => (
   <Slider
     labelText="Slider Label"
     value={50}
-    min={0}
+    min={30}
     max={100}
     step={1}
     stepMultiplier={10}

diff --git a/packages/react/src/components/Slider/Slider-test.js b/packages/react/src/components/Slider/Slider-test.js
@@ -128,6 +128,8 @@ describe('Slider', () => {
   describe('key/mouse event processing', () => {
     const handleChange = jest.fn();
     const handleRelease = jest.fn();
+    const handleBlur = jest.fn();
+
     const wrapper = mount(
       <Slider
         id="slider"
@@ -139,6 +141,7 @@ describe('Slider', () => {
         stepMultiplier={5}
         onChange={handleChange}
         onRelease={handleRelease}
+        onBlur={handleBlur}
       />
     );
 
@@ -224,24 +227,36 @@ describe('Slider', () => {
       });
     });
 
-    it('does not set incorrect state when typing a invalid value in input field', () => {
+    it('allows user to set invalid value when typing in input field', () => {
       const evt = {
         target: {
           value: '999',
-          checkValidity: () => false,
         },
       };
 
       wrapper.instance().onChange(evt);
-      expect(wrapper.state().value).toEqual(100);
-      expect(handleChange).toHaveBeenLastCalledWith({ value: 100 });
+      expect(wrapper.state().value).toEqual(999);
+      expect(handleChange).toHaveBeenLastCalledWith({ value: 999 });
+    });
+
+    it('checks for validity onBlur', () => {
+      const checkValidity = jest.fn();
+
+      const evt = {
+        target: {
+          value: '',
+          checkValidity: checkValidity,
+        },
+      };
+
+      wrapper.instance().onBlur(evt);
+      expect(checkValidity).toHaveBeenCalled();
     });
 
     it('sets correct state when typing a valid value in input field', () => {
       const evt = {
         target: {
           value: '12',
-          checkValidity: () => true,
         },
       };
 
@@ -251,6 +266,51 @@ describe('Slider', () => {
     });
   });
 
+  describe('syncs invalid state and props', () => {
+    const handleChange = jest.fn();
+    const handleRelease = jest.fn();
+    const handleBlur = jest.fn();
+
+    const wrapper = mount(
+      <Slider
+        id="slider"
+        className="extra-class"
+        value={50}
+        min={0}
+        max={100}
+        step={1}
+        onChange={handleChange}
+        onRelease={handleRelease}
+        onBlur={handleBlur}
+        invalid={false}
+      />
+    );
+
+    it('overrides invalid state for invalid prop', () => {
+      const changeEvt = {
+        target: {
+          value: '200',
+        },
+      };
+
+      const blurEvt = {
+        target: {
+          checkValidity: () => false,
+        },
+      };
+
+      wrapper.instance().onChange(changeEvt);
+      wrapper.instance().onBlur(blurEvt);
+      expect(wrapper.state().value).toEqual(200);
+      expect(wrapper.state().isValid).toEqual(true);
+      expect(
+        wrapper
+          .find(`.${prefix}--slider-text-input`)
+          .hasClass(`${prefix}--text-input--invalid`)
+      ).toEqual(false);
+    });
+  });
+
   describe('error handling', () => {
     const handleChange = jest.fn();
     const handleRelease = jest.fn();
@@ -292,6 +352,13 @@ describe('Slider', () => {
       expect(handleChange).not.toHaveBeenCalled();
     });
 
+    it('gracefully tolerates empty event passed to onBlur', () => {
+      const evt = {};
+      wrapper.instance().onBlur(evt);
+      expect(wrapper.state().value).toEqual(''); // from last test
+      expect(handleChange).not.toHaveBeenCalled();
+    });
+
     it('gracefully tolerates empty event passed to onKeyDown', () => {
       const evt = {};
       wrapper.instance().onKeyDown(evt);

diff --git a/packages/react/src/components/Slider/Slider.js b/packages/react/src/components/Slider/Slider.js
@@ -178,6 +178,7 @@ export default class Slider extends PureComponent {
     value: this.props.value,
     left: 0,
     needsOnRelease: false,
+    isValid: true,
   };
 
   /**
@@ -302,7 +303,7 @@ export default class Slider extends PureComponent {
     });
 
     // Set needsOnRelease flag so event fires on next update
-    this.setState({ needsOnRelease: true });
+    this.setState({ needsOnRelease: true, isValid: true });
   };
 
   /**
@@ -332,7 +333,7 @@ export default class Slider extends PureComponent {
     }
 
     const { value, left } = this.calcValue({ clientX });
-    this.setState({ value, left });
+    this.setState({ value, left, isValid: true });
   };
 
   /**
@@ -383,7 +384,8 @@ export default class Slider extends PureComponent {
           ? Math.floor(this.state.value / this.props.step) * this.props.step
           : this.state.value) + delta,
     });
-    this.setState({ value, left });
+
+    this.setState({ value, left, isValid: true });
   };
 
   /**
@@ -393,6 +395,7 @@ export default class Slider extends PureComponent {
    *
    * @param {Event} evt The event.
    */
+
   onChange = (evt) => {
     // Do nothing if component is disabled
     if (this.props.disabled) {
@@ -410,18 +413,34 @@ export default class Slider extends PureComponent {
     if (isNaN(targetValue)) {
       this.setState({ value: evt.target.value });
     } else {
-      // Recalculate the state's value and update the Slider
-      // if it is a valid number
-      if (evt.target.checkValidity() === false) {
-        return;
-      }
-
       const { value, left } = this.calcValue({
         value: targetValue,
         useRawValue: true,
       });
-      this.setState({ value, left, needsOnRelease: true });
+      this.setState({
+        value,
+        left,
+        needsOnRelease: true,
+      });
+    }
+  };
+
+  /**
+   * Checks for validity of input value after clicking out of the input. It also
+   * Handles state change to isValid state.
+   *
+   * @param {Event} evt The event.
+   */
+  onBlur = (evt) => {
+    // Do nothing if we have no valid event, target, or value
+    if (!evt || !('target' in evt) || typeof evt.target.value !== 'string') {
+      return;
     }
+    // determine validity of input change after clicking out of input
+    const validity = evt.target.checkValidity();
+    this.setState({
+      isValid: validity,
+    });
   };
 
   /**
@@ -488,6 +507,25 @@ export default class Slider extends PureComponent {
     return { value: steppedValue, left: steppedPercent * 100 };
   };
 
+  // syncs invalid state and prop
+  static getDerivedStateFromProps(props, state) {
+    const { isValid } = state;
+    // will override state in favor of invalid prop
+    if (props.invalid === true && isValid === true) {
+      return {
+        isValid: false,
+      };
+    }
+
+    if (props.invalid === false && isValid === false) {
+      return {
+        isValid: true,
+      };
+    }
+    //if invalid prop is not provided, state will remain the same
+    return null;
+  }
+
   render() {
     const {
       ariaLabelInput,
@@ -510,13 +548,13 @@ export default class Slider extends PureComponent {
       disabled,
       name,
       light,
-      invalid,
       ...other
     } = this.props;
 
     delete other.onRelease;
+    delete other.invalid;
 
-    const { value, left } = this.state;
+    const { value, left, isValid } = this.state;
 
     const scope = this.context;
     let enabled;
@@ -541,7 +579,7 @@ export default class Slider extends PureComponent {
       `${prefix}--slider-text-input`,
       {
         [`${prefix}--text-input--light`]: light,
-        [`${prefix}--text-input--invalid`]: this.props.invalid,
+        [`${prefix}--text-input--invalid`]: isValid === false,
       }
     );
 
@@ -579,7 +617,7 @@ export default class Slider extends PureComponent {
             onKeyDown={this.onKeyDown}
             role="presentation"
             tabIndex={-1}
-            data-invalid={invalid || null}
+            data-invalid={isValid ? null : true}
             {...other}>
             <div
               className={`${prefix}--slider__thumb`}
@@ -620,8 +658,9 @@ export default class Slider extends PureComponent {
             max={max}
             step={step}
             onChange={this.onChange}
-            data-invalid={invalid || null}
-            aria-invalid={invalid || null}
+            onBlur={this.onBlur}
+            data-invalid={isValid ? null : true}
+            aria-invalid={isValid ? null : true}
           />
         </div>
       </div>

diff --git a/packages/styles/scss/components/slider/_slider.scss b/packages/styles/scss/components/slider/_slider.scss
@@ -135,6 +135,10 @@
     }
   }
 
+  .#{$prefix}--slider-text-input.#{$prefix}--text-input--invalid {
+    padding-right: 1rem;
+  }
+
   .#{$prefix}--slider__thumb:focus ~ .#{$prefix}--slider__filled-track {
     background-color: $interactive;
   }