Improve InputGroup component

This commit adds support for the InputGroup component to receive a function as a child instead of a component. This lets the consumer handle the rendering completely, which in turn lets the consumer send in several children to the InputGroup. As a consequence of this, it's no longer possible to specify your own id-prop. Although this is breaking, it was an anti-pattern to begin with, and should not be heavily adopted.
SpareBank1 · Nov 2, 2017 · 84f6bf7 · 84f6bf7
1 parent 17d52f1
commit 84f6bf7
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,13 @@
 # Changelog for ffe-form-react
 
+## v.2.0.0
+* BREAKING: `InputGroup` no longer supports sending in your own ID - it will now be generated
+and applied automatically when needed.
+* `InputGroup` now supports sending a function as a child, which lets you deal with sending in
+several children (i.e. text nodes). See the README.md for how to use this.
+* `InputGroup` will throw a descriptive error when used with several children instead of
+with just one.
+
+
 ## v.1.0.0
 * First release.
diff --git a/README.md b/README.md
@@ -29,7 +29,7 @@ export const InputExample = props => (
     >
         <Input
             id="InputExample"
-            className="ffe-input-field--small"
+            className="some-modifier"
             type="tel"
             name="textInput"
             onChange={ e => console.log('onChange', e.target.value) }
@@ -45,7 +45,7 @@ export const InputExample = props => (
 
 #### InputGroup
 ```
-children: node,
+children: oneOfType([ node, func ]),
 className: string,
 fieldMessage: oneOfType([
     string,
@@ -57,12 +57,29 @@ label: oneOfType([ string, instanceOfComponent(Label) ]),
 tooltip: oneOfType([ string, instanceOfComponent(Tooltip) ])
 ```
 
-* `children`: Content of the InputGroup. 
+* `children`: Content of the InputGroup.
 * `className`: ClassName for the InputGroup div
 * `fieldMessage`: `ErrorFieldMessage`, `SuccessFieldMessage`, `InfoFieldMessage` or `string` that is displayed under `children`. If only a string is passed this defaults to an ErrorFieldMessage.
 * `label`: `Label` or `string` that is displayed above `children`.
 * `tooltip`: `Tooltip` or `string` that is displayed when the user presses the ?-button.
 
+Note that the `InputGroup` component only accepts a single child. This is due to the fact that the `InputGroup`
+decorates it's children with a few extra props like an auto-generated ID and aria-tags. If you need to have several
+children, please use the function-as-a-child pattern like the following example:
+
+```javascript
+<InputField label="How old are you?">
+  {(extraProps) => (
+    <div>
+        <InputField {...extraProps} />
+        <span> years old</span>
+    </div>
+  )}
+</InputField>
+```
+
+The `extraProps` argument is an object that needs to be spread on your input element. You can skip the enclosing `<div />`tag if
+you are using React@^16.0.0.
 
 #### Input
 ```
@@ -95,7 +112,7 @@ htmlFor: string.isRequired
 ```
 
 * `block`: Adds the 'ffe-form-label--block' class to the wrapper element.
-* `children`: Content of the label.  
+* `children`: Content of the label.
 * `className`: class of det label element.
 * `htmlFor`: Id of input element that this label is for.
 
@@ -129,7 +146,7 @@ onClick: func,
 
 ## Styling
 
-This package has peer dependencies for styling on `ffe-core` and `ffe-form`, so make sure you import 
+This package has peer dependencies for styling on `ffe-core` and `ffe-form`, so make sure you import
 those into your application's less file.
 
 ## Development

diff --git a/package.json b/package.json
@@ -1,13 +1,14 @@
 {
   "name": "ffe-form-react",
-  "version": "1.0.0",
+  "version": "2.0.0",
   "description": "React component for form elements in ffe",
   "main": "lib/index.js",
   "scripts": {
     "start": "webpack-dev-server",
     "prebuild": "npm run test && npm run test:nsp",
     "build": "babel -d lib/. --ignore=*.test.jsx src/.",
     "test": "mocha --require babel-register src/**/*.test.jsx",
+    "test:watch": "npm run test -- --watch",
     "test:nsp": "nsp check"
   },
   "repository": {

diff --git a/src/InputGroup.jsx b/src/InputGroup.jsx
@@ -1,5 +1,6 @@
-import React from 'react';
+import React, { Component } from 'react';
 import {
+    func,
     node,
     oneOf,
     oneOfType,
@@ -14,47 +15,65 @@ import ErrorFieldMessage from './ErrorFieldMessage';
 import SuccessFieldMessage from './SuccessFieldMessage';
 import InfoFieldMessage from './InfoFieldMessage';
 
-const InputGroup = ({
-    children,
-    className,
-    label,
-    fieldMessage,
-    tooltip,
-    ...rest
-}) => {
-    const childId = !!children.props.id ? children.props.id : uuid.v4();
-    const invalid = !!fieldMessage && (typeof fieldMessage === 'string' || fieldMessage.type === ErrorFieldMessage);
+class InputGroup extends Component {
+    constructor() {
+        super();
 
-    return (
-        <div
-            className={ classNames(
-                'ffe-input-group',
-                className
-            )}
-            { ...rest }
-        >
+        this.id = uuid.v4();
+    }
 
-            { typeof label === 'string' && <Label htmlFor={ childId }>{ label }</Label> }
-            { React.isValidElement(label) && React.cloneElement( label, { htmlFor : childId }) }
+    render() {
+        const {
+            children,
+            className,
+            label,
+            fieldMessage,
+            tooltip,
+            ...rest
+        } = this.props;
 
-            { typeof tooltip === 'string' && <Tooltip>{ tooltip }</Tooltip> }
-            { React.isValidElement(tooltip) && tooltip }
+        if (React.Children.count(children) > 1) {
+            throw new Error(
+                'This element does not support more than one child. If you need more than one element inside your ' +
+                'InputGroup, please use the function-as-a-child pattern outlined in the documentation.'
+            );
+        }
 
-            { React.cloneElement(children, {
-                'aria-invalid': String(invalid),
-                id: childId
-            })}
+        const extraProps = {
+            id: this.id,
+            'aria-invalid': String(!!fieldMessage && (typeof fieldMessage === 'string' || fieldMessage.type === ErrorFieldMessage)),
+        };
 
-            { typeof fieldMessage === 'string' && <ErrorFieldMessage>{ fieldMessage }</ErrorFieldMessage> }
-            { React.isValidElement(fieldMessage) && fieldMessage }
-        </div>
-    );
-};
+        const modifiedChildren = typeof children === 'function' ? children(extraProps) : React.cloneElement(children, extraProps);
+
+        return (
+            <div
+                className={ classNames(
+                    'ffe-input-group',
+                    className
+                )}
+                { ...rest }
+            >
+
+                { typeof label === 'string' && <Label htmlFor={ this.id }>{ label }</Label> }
+                { React.isValidElement(label) && React.cloneElement( label, { htmlFor : this.id }) }
+
+                { typeof tooltip === 'string' && <Tooltip>{ tooltip }</Tooltip> }
+                { React.isValidElement(tooltip) && tooltip }
+
+                { modifiedChildren }
+
+                { typeof fieldMessage === 'string' && <ErrorFieldMessage>{ fieldMessage }</ErrorFieldMessage> }
+                { React.isValidElement(fieldMessage) && fieldMessage }
+            </div>
+        );
+    }
+}
 
 const instanceOfComponent = component => shape({ type: oneOf([ component ])});
 
 InputGroup.propTypes = {
-    children: node.isRequired,
+    children: oneOfType({ func, node }).isRequired,
     className: string,
     fieldMessage: oneOfType([
         string,

diff --git a/src/InputGroup.test.jsx b/src/InputGroup.test.jsx
@@ -29,35 +29,29 @@ describe('<InputGroup>', () => {
     it('renders the given child', () => {
         const component = shallow(
             <InputGroup label="label">
-                <Input
-                    id="InputId"
-                />
+                <Input />
             </InputGroup>
         );
         expect(component.find('Input')).to.have.length(1);
-        expect(component.find('Input')).to.have.prop('id', 'InputId');
     });
 
     it('renders a Label if a string passed from the label prop', () => {
         const component = shallow(
             <InputGroup label="label">
-                <Input
-                    id="InputId"
-                />
+                <Input />
             </InputGroup>
         );
         expect(component.find('Label')).to.have.prop('children', 'label');
     });
 
-    it('renders a Label with htmlFor set to the same value of the id of children', () => {
+    it('renders a Label with htmlFor set to the same value of the children id', () => {
         const component = shallow(
             <InputGroup label="label">
-                <Input
-                    id="InputId"
-                />
+                <Input />
             </InputGroup>
         );
-        expect(component.find('Label')).to.have.prop('htmlFor', 'InputId');
+        const inputId = component.find('Input').prop('id');
+        expect(component.find('Label').prop('htmlFor')).to.equal(inputId);
     });
 
     it('generates a id for children and set it as htmlFor on Label', () => {
@@ -83,7 +77,7 @@ describe('<InputGroup>', () => {
         expect(component.find('Tooltip')).to.have.prop('children', 'tooltip');
     });
 
-    it('renders a ErrorFieldMessage and sets aria-invalid if a string is passed as fieldMessage', () => {
+    it('renders an ErrorFieldMessage and sets aria-invalid if a string is passed as fieldMessage', () => {
         const component = shallow(
             <InputGroup
                 label="InputLabel"
@@ -96,16 +90,15 @@ describe('<InputGroup>', () => {
         expect(component.find('Input')).to.have.prop('aria-invalid', 'true');
     });
 
-    it('renders a Label component if passed as label prop', () => {
+    it('renders a Label component if passed a label prop', () => {
         const component = shallow(
             <InputGroup
                 label={<Label htmlFor="inputId">LabelComponent</Label>}
             >
-                <Input id="inputId"/>
+                <Input/>
             </InputGroup>
         );
         expect(component.find('Label')).to.have.prop('children', 'LabelComponent');
-        expect(component.find('Label')).to.have.prop('htmlFor', 'inputId');
     });
 
     it('renders a Tooltip if passed as tooltip prop', () => {
@@ -145,4 +138,32 @@ describe('<InputGroup>', () => {
         expect(component.find('SuccessFieldMessage')).to.have.prop('children', 'SuccessFieldMessage');
     });
 
+    it('throws error when receiving multiple children', () => {
+        const component = () => shallow(
+            <InputGroup label="How much money do you make?">
+                <span>About...</span>
+                <Input />
+                <span> moneys</span>
+            </InputGroup>
+        );
+
+        expect(component).to.throw(Error);
+    });
+
+    it('supplies id and aria-invalid to function when child is a function', () => {
+        const component = shallow(
+            <InputGroup label="Fancy pattern">
+                {(props => (
+                    <div>
+                        <span>so cool</span>
+                        <Input {...props} />
+                        <span> easy stuff</span>
+                    </div>
+                ))}
+            </InputGroup>
+        );
+
+        expect(component.find('Input').prop('id')).to.have.lengthOf(36);
+        expect(component.find('Input').prop('aria-invalid')).to.equal('false');
+    });
 });