[Input] should accommodate number and string values (#7791)

* [Input] implement controlled tests for both string and numeric values

* [Input] isControlled should accomodate number | string values as indicated in Props.

* extensive testing of isDirty and hasValue to support both strings and numbers

This is based on the previous implementation, still not sure that 0 !isDirty

* [Input] ensure a value of `0` will mark the field as dirty as it’s a valid value

* Refactor Input hasValue/isDirty using logic from master

Author: rosskevin

src/Input/Input.js

@@ -6,11 +6,33 @@ import classNames from 'classnames';
 import withStyles from '../styles/withStyles';
 import Textarea from './Textarea';
 
+/**
+ * Supports determination of isControlled().
+ * Controlled input accepts its current value as a prop.
+ *
+ * @see https://facebook.github.io/react/docs/forms.html#controlled-components
+ * @param value
+ * @returns {boolean} true if string (including '') or number (including zero)
+ */
+export function hasValue(value: ?(number | string)) {
+  return value !== undefined && value !== null && !(Array.isArray(value) && value.length === 0);
+}
+
+/**
+ * Determine if field is dirty (a.k.a. filled).
+ *
+ * Response determines if label is presented above field or as placeholder.
+ *
+ * @param obj
+ * @param SSR
+ * @returns {boolean} False when not present or empty string.
+ *                    True when any number or string with length.
+ */
 export function isDirty(obj, SSR = false) {
   return (
     obj &&
-    ((obj.value && obj.value.toString().length) ||
-      (SSR && obj.defaultValue && obj.defaultValue.toString().length)) > 0
+    ((hasValue(obj.value) && obj.value !== '') ||
+      (SSR && hasValue(obj.defaultValue) && obj.defaultValue !== ''))
   );
 }
 
@@ -369,8 +391,14 @@ class Input extends Component<DefaultProps, AllProps, State> {
     }
   };
 
+  /**
+   * A controlled input accepts its current value as a prop.
+   *
+   * @see https://facebook.github.io/react/docs/forms.html#controlled-components
+   * @returns {boolean} true if string (including '') or number (including zero)
+   */
   isControlled() {
-    return typeof this.props.value === 'string';
+    return hasValue(this.props.value);
   }
 
   checkDirty(obj) {

src/Input/Input.spec.js

@@ -5,7 +5,7 @@ import { assert } from 'chai';
 import { spy } from 'sinon';
 import { createShallow, createMount, getClasses } from '../test-utils';
 import Textarea from './Textarea';
-import Input, { isDirty } from './Input';
+import Input, { hasValue, isDirty } from './Input';
 
 describe('<Input />', () => {
   let shallow;
@@ -95,39 +95,50 @@ describe('<Input />', () => {
   });
 
   describe('controlled', () => {
-    let wrapper;
-    let handleDirty;
-    let handleClean;
-
-    before(() => {
-      handleClean = spy();
-      handleDirty = spy();
-      wrapper = shallow(<Input value="" onDirty={handleDirty} onClean={handleClean} />);
-    });
-
-    it('should check that the component is controlled', () => {
-      const instance = wrapper.instance();
-      assert.strictEqual(instance.isControlled(), true, 'isControlled() should return true');
-    });
-
-    it('should have called the handleClean callback', () => {
-      assert.strictEqual(handleClean.callCount, 1, 'should have called the onClean cb');
-    });
+    ['', 0].forEach(value => {
+      describe(`${typeof value} value`, () => {
+        let wrapper;
+        let handleDirty;
+        let handleClean;
+
+        before(() => {
+          handleClean = spy();
+          handleDirty = spy();
+          wrapper = shallow(<Input value={value} onDirty={handleDirty} onClean={handleClean} />);
+        });
 
-    it('should fire the onDirty callback when dirtied', () => {
-      assert.strictEqual(handleDirty.callCount, 0, 'should not have called the onDirty cb yet');
-      wrapper.setProps({ value: 'hello' });
-      assert.strictEqual(handleDirty.callCount, 1, 'should have called the onDirty cb');
-    });
+        it('should check that the component is controlled', () => {
+          const instance = wrapper.instance();
+          assert.strictEqual(instance.isControlled(), true, 'isControlled() should return true');
+        });
 
-    it('should fire the onClean callback when dirtied', () => {
-      assert.strictEqual(
-        handleClean.callCount,
-        1,
-        'should have called the onClean cb once already',
-      );
-      wrapper.setProps({ value: '' });
-      assert.strictEqual(handleClean.callCount, 2, 'should have called the onClean cb again');
+        // don't test number because zero is a dirty state, whereas '' is not
+        if (typeof value !== 'number') {
+          it('should have called the handleClean callback', () => {
+            assert.strictEqual(handleClean.callCount, 1, 'should have called the onClean cb');
+          });
+
+          it('should fire the onDirty callback when dirtied', () => {
+            assert.strictEqual(
+              handleDirty.callCount,
+              0,
+              'should not have called the onDirty cb yet',
+            );
+            wrapper.setProps({ value: typeof value === 'number' ? 2 : 'hello' });
+            assert.strictEqual(handleDirty.callCount, 1, 'should have called the onDirty cb');
+          });
+
+          it('should fire the onClean callback when dirtied', () => {
+            assert.strictEqual(
+              handleClean.callCount,
+              1,
+              'should have called the onClean cb once already',
+            );
+            wrapper.setProps({ value });
+            assert.strictEqual(handleClean.callCount, 2, 'should have called the onClean cb again');
+          });
+        }
+      });
     });
   });
 
@@ -345,14 +356,37 @@ describe('<Input />', () => {
     });
   });
 
+  describe('hasValue', () => {
+    ['', 0].forEach(value => {
+      it(`is true for ${value}`, () => {
+        assert.strictEqual(hasValue(value), true);
+      });
+    });
+
+    [null, undefined].forEach(value => {
+      it(`is false for ${value}`, () => {
+        assert.strictEqual(hasValue(value), false);
+      });
+    });
+  });
+
   describe('isDirty', () => {
-    it('should support integer', () => {
-      assert.strictEqual(
-        isDirty({
-          value: 3,
-        }),
-        true,
-      );
+    [' ', 0].forEach(value => {
+      it(`is true for value ${value}`, () => {
+        assert.strictEqual(isDirty({ value }), true);
+      });
+
+      it(`is true for SSR defaultValue ${value}`, () => {
+        assert.strictEqual(isDirty({ defaultValue: value }, true), true);
+      });
+    });
+    [null, undefined, ''].forEach(value => {
+      it(`is false for value ${value}`, () => {
+        assert.strictEqual(isDirty({ value }), false);
+      });
+      it(`is false for SSR defaultValue ${value}`, () => {
+        assert.strictEqual(isDirty({ defaultValue: value }, true), false);
+      });
     });
   });
 });