diegohaz / arc

React starter kit based on Atomic Design
https://arc.js.org
2.91k stars 295 forks source link

Atom <Input> does not support type="select" despite having presentation-layer code expecting it. #331

Closed dotkas closed 6 years ago

dotkas commented 6 years ago

Is it possible that the Input-tag atom does not in fact support the <option> children, when setting type="select"?

I have not been able to find it in the src-example, so I have made a suggestion, let me know what you think.

diff --git a/src-example/components/atoms/Input/index.js b/src-example/components/atoms/Input/index.js
index e001a57..517608e 100644
--- a/src-example/components/atoms/Input/index.js
+++ b/src-example/components/atoms/Input/index.js
@@ -38,7 +38,13 @@ const Input = ({ ...props }) => {
   if (props.type === 'textarea') {
     return <StyledTextarea {...props} />
   } else if (props.type === 'select') {
-    return <StyledSelect {...props} />
+    return (
+      <StyledSelect { ...props }>
+        {
+          props.options.map((e) => { return <option key={ e.key } value={ e.value }>{ e.label }</option>; })
+        }
+      </StyledSelect>
+    );
   }
   return <StyledInput {...props} />
 }
@@ -48,6 +54,21 @@ Input.propTypes = {
   reverse: PropTypes.bool,
   height: PropTypes.number,
   invalid: PropTypes.bool,
+  options: (props, propName, componentName) => {
+    if (props.type !== 'select') {
+      return;
+    }
+
+    for (const element of props.options) {
+      if (!props.options.hasOwnProperty(element)) {
+        continue;
+      }
+
+      if (typeof(element) !== 'object' || !element.hasOwnProperty('key') || !element.hasOwnProperty('value') || !element.hasOwnProperty('label')) {
+        return new Error(`Invalid select element {${propName}:${JSON.stringify(props.value)}} supplied to ${componentName} Validation failed. For a 'select' type, each object in an array must have a key, value and label specified.`);
+      }
+    }
+  }
 }

 Input.defaultProps = {

Which will then require an array of objects as such: { key: 'DK', value: '45', label: 'DK' }, { key: 'SE', value: '44', label: 'SE' }

And then use it something like:

<Field name="areaCode" label="SomeLabel" type="select" options={config.supportedCountryCodes} component={ ReduxField }/>

And in a wrapping function somewhere:

(...)
initialValues: {
    areaCode: config.supportedCountryCodes[0].value
}
(...)
diegohaz commented 6 years ago

Didn't get it.

Aren't you able to do this?

      <Field name="test" label="Test" type="select" component={ReduxField}>
        <option>Option 1</option>
        <option>Option 2</option>
        <option>Option 3</option>
      </Field>
dotkas commented 6 years ago

Yes, but if you were to do it that way in the redux branch, where most logic is moved out into containers?

In a component such as: https://github.com/diegohaz/arc/blob/redux/src-example/components/organisms/PostForm/index.js - the values are all generated programatically through the redux-form props.

diegohaz commented 6 years ago

Still don't understand it.

If you are able to pass options={config.supportedCountryCodes}, you can pass the following:

<Field name="test" label="Test" type="select" component={ReduxField}>
  {config.supportedCountryCodes.map(e => <option key={e.key} value={e.value}>{e.label}</option>)}
</Field>

Why creating an abstraction for options when you can just use it as native HTML?

dotkas commented 6 years ago

I thought this was the intention of the <Input> atom. I guess I have misunderstood it. Closing again.