From a914f9e0713a5570017317d9ae9a632da3f22a6f Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 00:24:40 -0600 Subject: [PATCH 1/8] fixed a missing reference to source component in addFieldToState --- dist/components/Form.js | 4 ++-- dist/helpers/utilities.js | 4 ++-- src/components/Form.jsx | 4 ++-- src/helpers/utilities.jsx | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dist/components/Form.js b/dist/components/Form.js index 3782455..cf24d75 100644 --- a/dist/components/Form.js +++ b/dist/components/Form.js @@ -40,7 +40,7 @@ var Form = function (_React$Component) { _this.state = {}; _react2.default.Children.map(props.children, function (child) { - return _this.addFieldsToState(child, false); + return _this.addFieldsToState(_this, child, false); }); return _this; } @@ -67,7 +67,7 @@ var Form = function (_React$Component) { var _this2 = this; _react2.default.Children.map(this.props.children, function (child) { - return _this2.addFieldsToState(child, true); + return _this2.addFieldsToState(_this2, child, true); }); } }, { diff --git a/dist/helpers/utilities.js b/dist/helpers/utilities.js index 097b6b3..76d6551 100644 --- a/dist/helpers/utilities.js +++ b/dist/helpers/utilities.js @@ -90,8 +90,8 @@ function buildStateForField(fieldProps) { return newState; } -function addFieldsToState(child) { - var mounted = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : false; +function addFieldsToState(component, child) { + var mounted = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : false; if (typeof child.type === 'function' && child.type.name === 'Field') { var name = child.props.name; diff --git a/src/components/Form.jsx b/src/components/Form.jsx index 18c7adb..c60b309 100644 --- a/src/components/Form.jsx +++ b/src/components/Form.jsx @@ -12,7 +12,7 @@ const Form = class extends React.Component { this.state = {}; - React.Children.map(props.children, child => this.addFieldsToState(child, false)); + React.Children.map(props.children, child => this.addFieldsToState(this, child, false)); } onFieldChange({ name, value, valid, pristine }) { @@ -27,7 +27,7 @@ const Form = class extends React.Component { } reset() { - React.Children.map(this.props.children, child => this.addFieldsToState(child, true)); + React.Children.map(this.props.children, child => this.addFieldsToState(this, child, true)); } render() { diff --git a/src/helpers/utilities.jsx b/src/helpers/utilities.jsx index eb3fffd..02821b8 100644 --- a/src/helpers/utilities.jsx +++ b/src/helpers/utilities.jsx @@ -46,7 +46,7 @@ export function buildStateForField(fieldProps) { return newState; } -export function addFieldsToState(child, mounted = false) { +export function addFieldsToState(component, child, mounted = false) { if (typeof child.type === 'function' && child.type.name === 'Field') { const name = child.props.name; const fieldState = buildStateForField(child.props); From a78f150b13e2ccafe5b44118e85eb659b46efab0 Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 00:32:16 -0600 Subject: [PATCH 2/8] fixed Form locating nested Fields --- src/helpers/utilities.jsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/helpers/utilities.jsx b/src/helpers/utilities.jsx index 02821b8..5620ce6 100644 --- a/src/helpers/utilities.jsx +++ b/src/helpers/utilities.jsx @@ -51,12 +51,14 @@ export function addFieldsToState(component, child, mounted = false) { const name = child.props.name; const fieldState = buildStateForField(child.props); if (mounted) { - this.setState({ [name]: fieldState }); + component.setState({ + [name]: fieldState }); } else { - this.state[name] = fieldState; + component.state[name] = fieldState; // eslint-disable-line } } else if (child.props && child.props.children) { - React.Children.forEach(child.props.children, nextChild => addFieldsToState(nextChild, mounted)); + React.Children.forEach(child.props.children, + nextChild => addFieldsToState(component, nextChild, mounted)); } } From 988acb88e55087c334e4423c3c82f88a50e8fe1f Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 00:38:40 -0600 Subject: [PATCH 3/8] fixed case where didn't pass props down to nested element --- dist/helpers/utilities.js | 9 +++++---- src/helpers/utilities.jsx | 6 ++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/dist/helpers/utilities.js b/dist/helpers/utilities.js index 76d6551..da8d771 100644 --- a/dist/helpers/utilities.js +++ b/dist/helpers/utilities.js @@ -97,13 +97,13 @@ function addFieldsToState(component, child) { var name = child.props.name; var fieldState = buildStateForField(child.props); if (mounted) { - this.setState(_defineProperty({}, name, fieldState)); + component.setState(_defineProperty({}, name, fieldState)); } else { - this.state[name] = fieldState; + component.state[name] = fieldState; // eslint-disable-line } } else if (child.props && child.props.children) { _react2.default.Children.forEach(child.props.children, function (nextChild) { - return addFieldsToState(nextChild, mounted); + return addFieldsToState(component, nextChild, mounted); }); } } @@ -127,7 +127,8 @@ function makeFieldProps(child, onChange, state) { function mapPropsToChild(child, type, props) { if (child.type === type || typeof child.type === 'function' && child.type.name === type) { return _react2.default.cloneElement(child, props); - } else if (child.props && child.props.children) { + } + if (child.props && child.props.children) { var newChildren = _react2.default.Children.map(child.props.children, function (nestedChild) { return mapPropsToChild(nestedChild, type, props); }); diff --git a/src/helpers/utilities.jsx b/src/helpers/utilities.jsx index 5620ce6..949691c 100644 --- a/src/helpers/utilities.jsx +++ b/src/helpers/utilities.jsx @@ -52,7 +52,8 @@ export function addFieldsToState(component, child, mounted = false) { const fieldState = buildStateForField(child.props); if (mounted) { component.setState({ - [name]: fieldState }); + [name]: fieldState, + }); } else { component.state[name] = fieldState; // eslint-disable-line } @@ -77,7 +78,8 @@ export function makeFieldProps(child, onChange, state) { export function mapPropsToChild(child, type, props) { if (child.type === type || (typeof child.type === 'function' && child.type.name === type)) { return React.cloneElement(child, props); - } else if (child.props && child.props.children) { + } + if (child.props && child.props.children) { const newChildren = React.Children.map(child.props.children, nestedChild => ( mapPropsToChild(nestedChild, type, props))); return React.cloneElement(child, null, newChildren); From b2f579b87227206578269349f7cd80e0d4196d58 Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 00:53:00 -0600 Subject: [PATCH 4/8] changed mapPropsToChild to accept a function which returns props. This enabled onChange handlers to get passed to nested Fields --- dist/components/Field.js | 4 +++- dist/components/Form.js | 6 ++++-- dist/helpers/utilities.js | 6 +++--- src/components/Field.jsx | 2 +- src/components/Form.jsx | 8 ++++++-- src/helpers/utilities.jsx | 6 +++--- 6 files changed, 20 insertions(+), 12 deletions(-) diff --git a/dist/components/Field.js b/dist/components/Field.js index 1a45412..989f153 100644 --- a/dist/components/Field.js +++ b/dist/components/Field.js @@ -138,7 +138,9 @@ var Field = function (_React$Component) { 'div', null, _react2.default.Children.map(this.props.children, function (child) { - return (0, _utilities.mapPropsToChild)(child, 'input', inputProps); + return (0, _utilities.mapPropsToChild)(child, 'input', function () { + return inputProps; + }); }) ); } diff --git a/dist/components/Form.js b/dist/components/Form.js index cf24d75..fb370fa 100644 --- a/dist/components/Form.js +++ b/dist/components/Form.js @@ -78,8 +78,10 @@ var Form = function (_React$Component) { return _react2.default.createElement( 'form', { onSubmit: this.onSubmit }, - _react2.default.Children.map(this.props.children, function (child) { - return (0, _utilities.mapPropsToChild)(child, 'Field', (0, _utilities.makeFieldProps)(child, _this3.onFieldChange, _this3.state)); + _react2.default.Children.map(this.props.children, function (directChild) { + return (0, _utilities.mapPropsToChild)(directChild, 'Field', function (child) { + return (0, _utilities.makeFieldProps)(child, _this3.onFieldChange, _this3.state); + }); }) ); } diff --git a/dist/helpers/utilities.js b/dist/helpers/utilities.js index da8d771..841e54a 100644 --- a/dist/helpers/utilities.js +++ b/dist/helpers/utilities.js @@ -124,13 +124,13 @@ function makeFieldProps(child, onChange, state) { return null; } -function mapPropsToChild(child, type, props) { +function mapPropsToChild(child, type, propFunction) { if (child.type === type || typeof child.type === 'function' && child.type.name === type) { - return _react2.default.cloneElement(child, props); + return _react2.default.cloneElement(child, propFunction(child)); } if (child.props && child.props.children) { var newChildren = _react2.default.Children.map(child.props.children, function (nestedChild) { - return mapPropsToChild(nestedChild, type, props); + return mapPropsToChild(nestedChild, type, propFunction(nestedChild)); }); return _react2.default.cloneElement(child, null, newChildren); } diff --git a/src/components/Field.jsx b/src/components/Field.jsx index 69970de..92b72b2 100644 --- a/src/components/Field.jsx +++ b/src/components/Field.jsx @@ -107,7 +107,7 @@ const Field = class extends React.Component { return (
{React.Children - .map(this.props.children, child => mapPropsToChild(child, 'input', inputProps))} + .map(this.props.children, child => mapPropsToChild(child, 'input', () => inputProps))}
); } diff --git a/src/components/Form.jsx b/src/components/Form.jsx index c60b309..c00806b 100644 --- a/src/components/Form.jsx +++ b/src/components/Form.jsx @@ -34,8 +34,12 @@ const Form = class extends React.Component { return (
{React.Children - .map(this.props.children, child => - mapPropsToChild(child, 'Field', makeFieldProps(child, this.onFieldChange, this.state)))} + .map(this.props.children, directChild => + mapPropsToChild( + directChild, + 'Field', + child => makeFieldProps(child, this.onFieldChange, this.state), + ))}
); } diff --git a/src/helpers/utilities.jsx b/src/helpers/utilities.jsx index 949691c..d36fde8 100644 --- a/src/helpers/utilities.jsx +++ b/src/helpers/utilities.jsx @@ -75,13 +75,13 @@ export function makeFieldProps(child, onChange, state) { return null; } -export function mapPropsToChild(child, type, props) { +export function mapPropsToChild(child, type, propFunction) { if (child.type === type || (typeof child.type === 'function' && child.type.name === type)) { - return React.cloneElement(child, props); + return React.cloneElement(child, propFunction(child)); } if (child.props && child.props.children) { const newChildren = React.Children.map(child.props.children, nestedChild => ( - mapPropsToChild(nestedChild, type, props))); + mapPropsToChild(nestedChild, type, propFunction(nestedChild)))); return React.cloneElement(child, null, newChildren); } return child; From 3e183e66e17cde59dfcb7d8470ba7cca29cc167c Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 09:46:44 -0600 Subject: [PATCH 5/8] fixed a bug where a propFunction wasn't properly being passed to mapPropsToChild --- src/helpers/utilities.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/utilities.jsx b/src/helpers/utilities.jsx index d36fde8..3557725 100644 --- a/src/helpers/utilities.jsx +++ b/src/helpers/utilities.jsx @@ -81,7 +81,7 @@ export function mapPropsToChild(child, type, propFunction) { } if (child.props && child.props.children) { const newChildren = React.Children.map(child.props.children, nestedChild => ( - mapPropsToChild(nestedChild, type, propFunction(nestedChild)))); + mapPropsToChild(nestedChild, type, propFunction))); return React.cloneElement(child, null, newChildren); } return child; From 0892876e8d75eae18c38656b28729eadcfd65e60 Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 09:49:37 -0600 Subject: [PATCH 6/8] rebuilt dist/ after bugfix --- dist/components/Form.js | 6 +++--- dist/helpers/utilities.js | 2 +- src/components/Form.jsx | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dist/components/Form.js b/dist/components/Form.js index fb370fa..e7656ed 100644 --- a/dist/components/Form.js +++ b/dist/components/Form.js @@ -78,9 +78,9 @@ var Form = function (_React$Component) { return _react2.default.createElement( 'form', { onSubmit: this.onSubmit }, - _react2.default.Children.map(this.props.children, function (directChild) { - return (0, _utilities.mapPropsToChild)(directChild, 'Field', function (child) { - return (0, _utilities.makeFieldProps)(child, _this3.onFieldChange, _this3.state); + _react2.default.Children.map(this.props.children, function (child) { + return (0, _utilities.mapPropsToChild)(child, 'Field', function (grandChild) { + return (0, _utilities.makeFieldProps)(grandChild, _this3.onFieldChange, _this3.state); }); }) ); diff --git a/dist/helpers/utilities.js b/dist/helpers/utilities.js index 841e54a..6ec1d2a 100644 --- a/dist/helpers/utilities.js +++ b/dist/helpers/utilities.js @@ -130,7 +130,7 @@ function mapPropsToChild(child, type, propFunction) { } if (child.props && child.props.children) { var newChildren = _react2.default.Children.map(child.props.children, function (nestedChild) { - return mapPropsToChild(nestedChild, type, propFunction(nestedChild)); + return mapPropsToChild(nestedChild, type, propFunction); }); return _react2.default.cloneElement(child, null, newChildren); } diff --git a/src/components/Form.jsx b/src/components/Form.jsx index c00806b..d8f3ee6 100644 --- a/src/components/Form.jsx +++ b/src/components/Form.jsx @@ -34,11 +34,11 @@ const Form = class extends React.Component { return (
{React.Children - .map(this.props.children, directChild => + .map(this.props.children, child => mapPropsToChild( - directChild, + child, 'Field', - child => makeFieldProps(child, this.onFieldChange, this.state), + grandChild => makeFieldProps(grandChild, this.onFieldChange, this.state), ))}
); From a40942e73b0736e3394387ab441601bec9224adf Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 18:10:02 -0600 Subject: [PATCH 7/8] improved number validation to match behavior of html5 number input types --- dist/components/Field.js | 4 ++-- dist/helpers/validators.js | 3 +-- src/components/Field.jsx | 4 ++-- src/helpers/validators.jsx | 7 ++++--- tests/spec_helpers/index.js | 2 +- tests/utilities/validators.spec.js | 23 ++++++++++++++++------- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/dist/components/Field.js b/dist/components/Field.js index 989f153..6875ffc 100644 --- a/dist/components/Field.js +++ b/dist/components/Field.js @@ -51,7 +51,7 @@ var Field = function (_React$Component) { _createClass(Field, [{ key: 'componentWillUpdate', value: function componentWillUpdate(nextProps) { - if (nextProps.value !== this.props.value && nextProps.value !== this.finalValue) { + if (nextProps.value !== this.props.value && nextProps.value !== this.state.value) { this.cancelBroadcast(); this.setState({ value: nextProps.value }); this.finalValue = nextProps.value; @@ -65,7 +65,7 @@ var Field = function (_React$Component) { }, { key: 'shouldComponentUpdate', value: function shouldComponentUpdate(nextProps) { - if (nextProps.value !== this.finalValue) return true; + if (nextProps.value !== this.state.value) return true; if (this.state.value !== this.finalValue) return true; if (this.props.match !== nextProps.match) return true; return false; diff --git a/dist/helpers/validators.js b/dist/helpers/validators.js index f8ea9a1..4f301f2 100644 --- a/dist/helpers/validators.js +++ b/dist/helpers/validators.js @@ -75,9 +75,8 @@ function alpha() { } function numeric() { - var numericRegex = /[^0-9\s]/i; return function (value) { - return typeof value === 'number' || typeof value === 'string' && !numericRegex.test(value); + return typeof value === 'number' || typeof value === 'string' && (!value || !value.replace(/([-+]{0,1})[0-9]+(\.[0-9]+)?([eE]([+-]{0,1})[0-9]+)?/, '')); }; } diff --git a/src/components/Field.jsx b/src/components/Field.jsx index 92b72b2..de0922a 100644 --- a/src/components/Field.jsx +++ b/src/components/Field.jsx @@ -29,7 +29,7 @@ const Field = class extends React.Component { } componentWillUpdate(nextProps) { - if ((nextProps.value !== this.props.value) && (nextProps.value !== this.finalValue)) { + if ((nextProps.value !== this.props.value) && (nextProps.value !== this.state.value)) { this.cancelBroadcast(); this.setState({ value: nextProps.value }); this.finalValue = nextProps.value; @@ -42,7 +42,7 @@ const Field = class extends React.Component { } shouldComponentUpdate(nextProps) { - if (nextProps.value !== this.finalValue) return true; + if (nextProps.value !== this.state.value) return true; if (this.state.value !== this.finalValue) return true; if (this.props.match !== nextProps.match) return true; return false; diff --git a/src/helpers/validators.jsx b/src/helpers/validators.jsx index e291360..effc906 100644 --- a/src/helpers/validators.jsx +++ b/src/helpers/validators.jsx @@ -48,12 +48,13 @@ export function alpha() { } export function numeric() { - const numericRegex = /[^0-9\s]/i; - return value => typeof value === 'number' || (typeof value === 'string' && !numericRegex.test(value)); + return value => typeof value === 'number' || (typeof value === 'string' && + (!value || !value.replace(/([-+]{0,1})[0-9]+(\.[0-9]+)?([eE]([+-]{0,1})[0-9]+)?/, ''))); } export function max(criteria) { - return value => ((typeof value === 'string' && value) || typeof value === 'number') && (Number(value) <= Number(criteria)); + return value => ((typeof value === 'string' && value) || typeof value === 'number') && + (Number(value) <= Number(criteria)); } export function min(criteria) { diff --git a/tests/spec_helpers/index.js b/tests/spec_helpers/index.js index 97fbc6a..be461f7 100644 --- a/tests/spec_helpers/index.js +++ b/tests/spec_helpers/index.js @@ -6,7 +6,7 @@ export function updateInput(DOM, value = '', type = 'text') { DOM.find('input').simulate('change', { target: { value }, type }); } -export function buildField(mountingFunction, validator, value, type) { +export function buildField(mountingFunction, validator, value, type="text") { const validatorToObj = { [validator]: value, }; diff --git a/tests/utilities/validators.spec.js b/tests/utilities/validators.spec.js index 551d8c2..8a04e90 100644 --- a/tests/utilities/validators.spec.js +++ b/tests/utilities/validators.spec.js @@ -1,4 +1,4 @@ -/* globals describe it before beforeEach after afterEach */ +/* globals describe it before beforeEach after afterEach xit */ import { expect } from 'chai'; // eslint-disable-line import sinon from 'sinon'; // eslint-disable-line import { shallow, mount } from 'enzyme'; // eslint-disable-line @@ -260,38 +260,47 @@ describe('Validator Functionality', () => { const wrapper = buildField(mount, 'number', true); const valFunc = validators.numeric(); - it('returns `true` for strings consisting of only number and space characters', () => { + it('returns `true` for strings consisting of only number characters', () => { expect(valFunc('')).to.equal(true); + expect(valFunc('4e-10')).to.equal(true); + expect(valFunc('0.5E+10')).to.equal(true); expect(valFunc('0123')).to.equal(true); + expect(valFunc(123e-10)).to.equal(true); + expect(valFunc(456e-10)).to.equal(true); expect(valFunc(123)).to.equal(true); - expect(valFunc('123 123 123')).to.equal(true); - expect(valFunc('\t\n ')).to.equal(true); }); it('returns `false` for text inputs with non-numeric/non-space characters', () => { + expect(valFunc('\t\n ')).to.equal(false); expect(valFunc('_')).to.equal(false); expect(valFunc('123!')).to.equal(false); expect(valFunc('890*')).to.equal(false); + expect(valFunc('123 123 123')).to.equal(false); + expect(valFunc('1234-1234-1234-1234')).to.equal(false); expect(valFunc('$112233')).to.equal(false); expect(valFunc('$!\.')).to.equal(false); expect(valFunc('Is this valid?')).to.equal(false); }); it('is properly used by a `Field` component to validate', () => { - expect(wrapper.state()).to.have.property('valid', false); expect(wrapper.state()).to.have.property('value', ''); + expect(wrapper.state()).to.have.property('valid', false); updateInput(wrapper, '12345\t12345 12345'); - expect(wrapper.state()).to.have.property('valid', true); expect(wrapper.state()).to.have.property('value', '12345\t12345 12345'); + expect(wrapper.state()).to.have.property('valid', false); updateInput(wrapper, '1234-1234-1234-1234'); - expect(wrapper.state()).to.have.property('valid', false); expect(wrapper.state()).to.have.property('value', '1234-1234-1234-1234'); + expect(wrapper.state()).to.have.property('valid', false); updateInput(wrapper, 123, 'number'); expect(wrapper.state()).to.have.property('valid', true); expect(wrapper.state()).to.have.property('value', 123); + + updateInput(wrapper, 0.5e3, 'number'); + expect(wrapper.state()).to.have.property('valid', true); + expect(wrapper.state()).to.have.property('value', 0.5e3); }); }); From 7bc600b07c75e4349447ffc0f3ef4bc4c1d23fea Mon Sep 17 00:00:00 2001 From: Chris LoCasto Date: Sun, 12 Feb 2017 18:14:34 -0600 Subject: [PATCH 8/8] updated readme to reflect changes to number validation --- readme.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 79cadd9..238e735 100644 --- a/readme.md +++ b/readme.md @@ -202,6 +202,8 @@ The `Field` component will behave as follows with respect to its children: > @param {String} [type='text'] - The input type of the wrapped input element. The input type for the wrapped input element. Defaults to `text`. + + *Note:* When number input is desired, it is preferred to use 'text' inputs with a `number` validator if it is expected that the user will enter `+`, `-`, or `e` characters. See `https://github.com/facebook/react/issues/1549`. #### `props[validator] = [validator]` > @param {\?} [validator=\?] - Optional. One or more validators to apply to the `Field`'s state. @@ -261,9 +263,9 @@ There are also a handful of different validators and properties (debounce, lengt This validates that the string input is comprised only of english alphabet characters and space characters. #### `props.number = numericValidation` -> @param {Boolean} [numericValidation=true] Optional. Will toggle validation for only numeric and space characters. +> @param {Boolean} [numericValidation=true] Optional. Will toggle validation for only numeric characters. - This validates that the string or number input is comprised only of numeric and space characters. + This validates that the string or number input is comprised only of numeric characters. This will allow appropriately placed `+`, `-`, `e`, and `.` characters. #### `props.max = maxValue` > @param {Number} maxValue - Validates an input field to be less than or equal to the maxValue.