From d771a59d58e6b09c7fdadad2595168bf0fb078f6 Mon Sep 17 00:00:00 2001 From: Aaron Panchal Date: Wed, 8 Feb 2017 11:08:37 -0800 Subject: [PATCH 1/6] ap - make BoundForm provide value, error, and change contexts --- src/components/BoundForm.js | 14 ++++++++++++++ test/components/BoundForm.spec.js | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/src/components/BoundForm.js b/src/components/BoundForm.js index 546c7b934..23e9a87e5 100644 --- a/src/components/BoundForm.js +++ b/src/components/BoundForm.js @@ -19,6 +19,12 @@ class BoundForm extends React.Component { onChange: noop }; + static childContextTypes = { + value: React.PropTypes.object, + errors: React.PropTypes.object, + onChange: React.PropTypes.func + } + constructor(props) { super(props); @@ -27,6 +33,14 @@ class BoundForm extends React.Component { }; } + getChildContext() { + return { + value: this.state.formData, + errors: this.props.errors, + onChange: this.handleChange + } + } + onSubmit = e => { e.preventDefault(); this.props.onSubmit(e, this.state.formData); diff --git a/test/components/BoundForm.spec.js b/test/components/BoundForm.spec.js index 036370443..a5dafb76e 100644 --- a/test/components/BoundForm.spec.js +++ b/test/components/BoundForm.spec.js @@ -48,6 +48,13 @@ describe('', () => { ); + it('should provide a context', () => { + const context = component.instance().getChildContext(); + assert.equal(context.value, component.state().formData); + assert.equal(context.errors, errors); + assert.equal(context.onChange, component.instance().handleChange); + }); + it('should not add props to non-FormRows', () => { const title = component.find('h1'); assert.equal(title.prop('value'), undefined); From 843fee94535f597d0857e42f4c790efea40894f9 Mon Sep 17 00:00:00 2001 From: Aaron Panchal Date: Wed, 8 Feb 2017 15:12:44 -0800 Subject: [PATCH 2/6] ap - create BoundFormRow to read context from BoundForm --- src/components/BoundFormRow.js | 19 +++++++++++++ test/components/BoundFormRow.spec.js | 41 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/components/BoundFormRow.js create mode 100644 test/components/BoundFormRow.spec.js diff --git a/src/components/BoundFormRow.js b/src/components/BoundFormRow.js new file mode 100644 index 000000000..ca0346af3 --- /dev/null +++ b/src/components/BoundFormRow.js @@ -0,0 +1,19 @@ +import React from 'react'; +import FormRow from './FormRow'; + +const BoundFormRow = (props, { value = {}, errors = {}, onChange }) => ( + +); + +BoundFormRow.contextTypes = { + value: React.PropTypes.object, + errors: React.PropTypes.object, + onChange: React.PropTypes.func +}; + +export default BoundFormRow; diff --git a/test/components/BoundFormRow.spec.js b/test/components/BoundFormRow.spec.js new file mode 100644 index 000000000..63131fa99 --- /dev/null +++ b/test/components/BoundFormRow.spec.js @@ -0,0 +1,41 @@ +import React from 'react'; +import sinon from 'sinon'; +import assert from 'assert'; +import { shallow } from 'enzyme'; + +import BoundFormRow from '../../src/components/BoundFormRow'; + +describe('', () => { + const onChange = sinon.stub(); + const component = shallow( + , { + context: { + value: { + firstName: 'First Name', + lastName: 'Last Name' + }, + errors: { + firstName: 'Not Right', + lastName: 'Something Else' + }, + onChange + } + } + ); + + it('should forward props', () => { + assert.equal(component.prop('name'), 'firstName'); + }); + + it('should pass the value from context', () => { + assert.equal(component.prop('value'), 'First Name'); + }); + + it('should pass the error from context', () => { + assert.equal(component.prop('feedback'), 'Not Right'); + }); + + it('should pass the onChange handler from context', () => { + assert.equal(component.prop('onChange'), onChange('firstName')); + }); +}); From 4388d726fca6399577d9e0ca7ae6b5185c44dfa4 Mon Sep 17 00:00:00 2001 From: Aaron Panchal Date: Thu, 9 Feb 2017 11:10:28 -0800 Subject: [PATCH 3/6] ap - clean up BoundForm to use context --- src/components/BoundForm.js | 17 +------ src/components/BoundFormRow.js | 4 ++ src/index.js | 2 + stories/Forms.js | 22 ++++----- test/components/BoundForm.spec.js | 81 ++++++++----------------------- 5 files changed, 39 insertions(+), 87 deletions(-) diff --git a/src/components/BoundForm.js b/src/components/BoundForm.js index 23e9a87e5..5456b088d 100644 --- a/src/components/BoundForm.js +++ b/src/components/BoundForm.js @@ -53,23 +53,8 @@ class BoundForm extends React.Component { } render() { - const children = React.Children.map(this.props.children, child => { - if (child.type !== FormRow) { return child; } // TODO make this better, using Context? - - const value = this.state.formData[child.props.name] || ''; - const feedback = this.props.errors[child.props.name]; - const color = feedback ? 'danger' : null; - - return React.cloneElement(child, { - feedback, - color, - value, - onChange: this.handleChange(child.props.name) - }); - }); - return ( -
+ ); } } diff --git a/src/components/BoundFormRow.js b/src/components/BoundFormRow.js index ca0346af3..95d7a49b9 100644 --- a/src/components/BoundFormRow.js +++ b/src/components/BoundFormRow.js @@ -16,4 +16,8 @@ BoundFormRow.contextTypes = { onChange: React.PropTypes.func }; +BoundFormRow.propTypes = { + name: React.PropTypes.string.isRequired +}; + export default BoundFormRow; diff --git a/src/index.js b/src/index.js index 28e4d44a3..4f2fdef93 100755 --- a/src/index.js +++ b/src/index.js @@ -69,6 +69,7 @@ import AddressInput from './components/AddressInput.js'; import Alert from './components/Alert.js'; import BlockPanel from './components/BlockPanel.js'; import BoundForm from './components/BoundForm'; +import BoundFormRow from './components/BoundFormRow'; import CurrencyInput from './components/CurrencyInput.js'; import Datapair from './components/Datapair.js'; import DateMonth from './components/datemonth/DateMonth.js'; @@ -160,6 +161,7 @@ export { Alert, BlockPanel, BoundForm, + BoundFormRow, CurrencyInput, Datapair, DateMonth, diff --git a/stories/Forms.js b/stories/Forms.js index f953bf373..cc4bd59df 100644 --- a/stories/Forms.js +++ b/stories/Forms.js @@ -2,7 +2,7 @@ import React from 'react'; import { Container, Input } from 'reactstrap'; import { storiesOf, action } from '@kadira/storybook'; -import { BoundForm, FormRow, FormChoice, CurrencyInput, AddressInput } from '../src'; +import { BoundForm, BoundFormRow, FormRow, FormChoice, CurrencyInput, AddressInput } from '../src'; import { text, boolean, number, object, select } from '@kadira/storybook-addon-knobs'; const formData = { @@ -84,27 +84,27 @@ storiesOf('Forms', module) object={formData} errors={object('errors', { lastName: "can't be blank" })} onSubmit={action('submit')}> - - - - + + + + A New Hope The Empire Strikes Back The Force Awakens - - + + Darth Vader Luke Skywalker Emperor Palpatine Rey TK-421 - - + + Death Star Millennium Falcon Imperial Shuttle - - + + )); diff --git a/test/components/BoundForm.spec.js b/test/components/BoundForm.spec.js index a5dafb76e..8e386652f 100644 --- a/test/components/BoundForm.spec.js +++ b/test/components/BoundForm.spec.js @@ -7,7 +7,7 @@ import { shallow } from 'enzyme'; import { Input } from 'reactstrap'; import BoundForm from '../../src/components/BoundForm'; -import FormRow from '../../src/components/FormRow'; +import BoundFormRow from '../../src/components/BoundFormRow'; import FormChoice from '../../src/components/FormChoice'; describe('', () => { @@ -19,8 +19,6 @@ describe('', () => { } }; - data[undefined] = 'not good' // For testing items that don't have a name - const errors = { firstName: "Can't be Glenn" } @@ -28,24 +26,8 @@ describe('', () => { const submitFunc = sinon.stub(); const changeFunc = sinon.stub(); - const Composite = props => ( -
- - -
- ); - const component = shallow( - -

Title

- - - - Darth Vader - - - -
+ ); it('should provide a context', () => { @@ -55,69 +37,48 @@ describe('', () => { assert.equal(context.onChange, component.instance().handleChange); }); - it('should not add props to non-FormRows', () => { - const title = component.find('h1'); - assert.equal(title.prop('value'), undefined); - }); - - it('should provide value from data object', () => { - const row = component.find('[name="firstName"]'); - assert.equal(row.prop('value'), 'Glenn'); - }); - - it('should provide default value for inputs without data', () => { - const row = component.find('[name="lastName"]'); - assert.equal(row.prop('value'), ''); - }); - it('should update data on change', () => { - const row = component.find('[name="firstName"]'); - row.simulate('change', 'Desmond'); + const firstNameOnChange = component.instance().handleChange('firstName'); + firstNameOnChange('Desmond'); assert.equal(data.firstName, 'Glenn'); assert.equal(component.state('formData').firstName, 'Desmond'); }); it('should emit onChange when changed', () => { - const row = component.find('[name="firstName"]'); - row.simulate('change', 'Desmond'); + const firstNameOnChange = component.instance().handleChange('firstName'); + firstNameOnChange('Desmond'); assert(changeFunc.calledWith(Object.assign({}, data, { firstName: 'Desmond' }))); }); - it('should support nested data', () => { - const row = component.find('[name="address"]'); - assert.equal(row.prop('value'), component.state('formData').address); - }); - it('should support updating nested data', () => { - const row = component.find('[name="address"]'); - row.simulate('change', { address1: '456 cool' }); + const addressOnChange = component.instance().handleChange('address'); + addressOnChange({ address1: '456 cool' }); + assert.equal(data.address.address1, '123 awesome'); assert.deepEqual(component.state('formData').address, { address1: '456 cool' }); }); it('should create nested data', () => { - const row = component.find('[name="thing"]'); - row.simulate('change', { stuff: 'here' }); + const thingOnChange = component.instance().handleChange('thing'); + thingOnChange({ stuff: 'here' }); assert.equal(data.thing, undefined); assert.deepEqual(component.state('formData').thing, { stuff: 'here' }); }); + it('should update data with target', () => { + const lastNameOnChange = component.instance().handleChange('lastName'); + const target = document.createElement('input'); + target.setAttribute('value', 'LastName'); + lastNameOnChange({ target }); + + assert.equal(data.lastName, undefined); + assert.equal(component.state('formData').lastName, 'LastName'); + }); + it('should submit with formData', () => { const preventDefault = sinon.stub(); component.simulate('submit', { preventDefault }); assert.equal(preventDefault.calledOnce, true); assert.equal(submitFunc.calledWith({ preventDefault }, component.state('formData')), true); }); - - it('should append errors', () => { - const row = component.find('[name="firstName"]'); - assert.equal(row.prop('feedback'), errors.firstName); - assert.equal(row.prop('color'), 'danger'); - }); - - it('should not have color for valid rows', () => { - const row = component.find('[name="address"]'); - assert.equal(row.prop('feedback'), ''); - assert.equal(row.prop('color'), null); - }); }); From aa0ce6897228b2d370532600bb76a055a296147d Mon Sep 17 00:00:00 2001 From: Aaron Panchal Date: Thu, 9 Feb 2017 11:33:30 -0800 Subject: [PATCH 4/6] ap - add default values for BoundFormRow --- src/components/BoundFormRow.js | 4 +- test/components/BoundFormRow.spec.js | 67 +++++++++++++++++----------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/components/BoundFormRow.js b/src/components/BoundFormRow.js index 95d7a49b9..e87d8850b 100644 --- a/src/components/BoundFormRow.js +++ b/src/components/BoundFormRow.js @@ -3,8 +3,8 @@ import FormRow from './FormRow'; const BoundFormRow = (props, { value = {}, errors = {}, onChange }) => ( diff --git a/test/components/BoundFormRow.spec.js b/test/components/BoundFormRow.spec.js index 63131fa99..294791ac2 100644 --- a/test/components/BoundFormRow.spec.js +++ b/test/components/BoundFormRow.spec.js @@ -6,36 +6,53 @@ import { shallow } from 'enzyme'; import BoundFormRow from '../../src/components/BoundFormRow'; describe('', () => { - const onChange = sinon.stub(); - const component = shallow( - , { - context: { - value: { - firstName: 'First Name', - lastName: 'Last Name' - }, - errors: { - firstName: 'Not Right', - lastName: 'Something Else' - }, - onChange + describe('with context', () => { + const onChange = sinon.stub(); + const component = shallow( + , { + context: { + value: { + firstName: 'First Name', + lastName: 'Last Name' + }, + errors: { + firstName: 'Not Right', + lastName: 'Something Else' + }, + onChange + } } - } - ); + ); - it('should forward props', () => { - assert.equal(component.prop('name'), 'firstName'); - }); + it('should forward props', () => { + assert.equal(component.prop('name'), 'firstName'); + }); - it('should pass the value from context', () => { - assert.equal(component.prop('value'), 'First Name'); - }); + it('should pass the value from context', () => { + assert.equal(component.prop('value'), 'First Name'); + }); + + it('should pass the error from context', () => { + assert.equal(component.prop('feedback'), 'Not Right'); + }); - it('should pass the error from context', () => { - assert.equal(component.prop('feedback'), 'Not Right'); + it('should pass the onChange handler from context', () => { + assert.equal(component.prop('onChange'), onChange('firstName')); + }); }); - it('should pass the onChange handler from context', () => { - assert.equal(component.prop('onChange'), onChange('firstName')); + describe('with defaults', () => { + const onChange = sinon.stub(); + const component = shallow( + , { context: { onChange } } + ); + + it('should set the default context value', () => { + assert.equal(component.prop('value'), ''); + }); + + it('should set the default context errors', () => { + assert.equal(component.prop('feedback'), ''); + }); }); }); From 725d4e1c17e5472d2dbb9d8c0cd29990ab21db88 Mon Sep 17 00:00:00 2001 From: Aaron Panchal Date: Thu, 9 Feb 2017 13:57:11 -0800 Subject: [PATCH 5/6] ap - have FormRow determine color/state --- src/components/FormRow.js | 5 +++-- test/components/FormRow.spec.js | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/components/FormRow.js b/src/components/FormRow.js index 3b860bbb6..db682522f 100644 --- a/src/components/FormRow.js +++ b/src/components/FormRow.js @@ -34,9 +34,10 @@ const FormRow = props => { } = props; const InputElement = determineElement(type); + const rowColor = color || state || (feedback && 'danger'); return ( - +