fix(NumberInput): avoid validation for space (carbon-design-system#2280)

Fixes carbon-design-system#2106
joshblack · May 1, 2019 · 290fab3 · 290fab3
1 parent 66c35f9
commit 290fab3
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 2 deletions.
diff --git a/src/components/NumberInput/NumberInput-test.js b/src/components/NumberInput/NumberInput-test.js
@@ -204,6 +204,39 @@ describe('NumberInput', () => {
           expect(wrapper.find('NumberInput').instance().state.value).toEqual(2);
         });
 
+        it('should cap the number given to value prop', () => {
+          // Enzyme doesn't seem to allow setState() in a forwardRef-wrapped class component
+          wrapper
+            .find('NumberInput')
+            .instance()
+            .setState({ value: 0 });
+          wrapper.update();
+          wrapper.setProps({ value: 5, min: 10, max: 20 });
+          // Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
+          expect(wrapper.find('NumberInput').instance().state.value).toEqual(
+            10
+          );
+          wrapper.setProps({ value: 25, min: 10, max: 20 });
+          // Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
+          expect(wrapper.find('NumberInput').instance().state.value).toEqual(
+            20
+          );
+        });
+
+        it('should avoid capping when non-number prop is given to value prop', () => {
+          // Enzyme doesn't seem to allow setState() in a forwardRef-wrapped class component
+          wrapper
+            .find('NumberInput')
+            .instance()
+            .setState({ value: 2 });
+          wrapper.update();
+          wrapper.setProps({ value: '', min: 1, max: 3 });
+          // Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
+          expect(wrapper.find('NumberInput').instance().state.value).toEqual(
+            ''
+          );
+        });
+
         it('should avoid change the value upon setting props, unless there the value actually changes', () => {
           wrapper.setProps({ value: 1 });
           // Enzyme doesn't seem to allow setState() in a forwardRef-wrapped class component

diff --git a/src/components/NumberInput/NumberInput.js b/src/components/NumberInput/NumberInput.js
@@ -29,6 +29,15 @@ const defaultTranslations = {
   [translationIds['decrement.number']]: 'Decrement number',
 };
 
+const capMin = (min, value) =>
+  isNaN(min) || (!min && min !== 0) || isNaN(value) || (!value && value !== 0)
+    ? value
+    : Math.max(min, value);
+const capMax = (max, value) =>
+  isNaN(max) || (!max && max !== 0) || isNaN(value) || (!value && value !== 0)
+    ? value
+    : Math.min(max, value);
+
 class NumberInput extends Component {
   constructor(props) {
     super(props);
@@ -148,12 +157,12 @@ class NumberInput extends Component {
    */
   _inputRef = null;
 
-  static getDerivedStateFromProps({ min, value }, state) {
+  static getDerivedStateFromProps({ min, max, value }, state) {
     const { prevValue } = state;
     return prevValue === value
       ? null
       : {
-          value: isNaN(min) ? value : Math.max(min, value),
+          value: capMax(max, capMin(min, value)),
           prevValue: value,
         };
   }