heilhead / react-bootstrap-validation

Form validation for react-bootstrap
MIT License
136 stars 50 forks source link

Splitting out ValidatedInputs into sub components. #8

Open wkerswell opened 9 years ago

wkerswell commented 9 years ago

I have quite a large form and would like to split is out into sub components. When I do this I get the message 'Input must be placed inside the Form component'. Is there a way I can do this?

I have created a SO question to illustrate my issue.

heilhead commented 9 years ago

Looks like it should be working, but it does not. I'll take a deeper look at the problem when I have more time.

wkerswell commented 9 years ago

Thanks for the reply. I will take a look with the update and let you know.

mszyndel commented 9 years ago

Same problem here! I'm not proficient with react though yet to help fix it but will be glad to test if you find a solution.

mszyndel commented 9 years ago

I dug into it a little and problem seems to be that child.props.children is undefined in line 127 of Form.js

wkerswell commented 9 years ago

I thought I would be something to do with form.js.78 if (child.type === ValidatedInput || child.type === RadioGroup) { so it wouldn't register the child elements. I did think about trying to extend the ValidatedInput class but I have multiple inputs in the sub-component so that wouldn't work. Could do with recursively going through the children to find ValidatedInputs but as you said I am not proficient with react either.

mszyndel commented 9 years ago

@wkerswell that's not the problem. The problem is elements we put in render are not available in element.props.children for this component code. You can quickly test it with following change to you SO code, it will work:

<TitleSelect handleChange={this.updateDropDown} value={this.state.form.title}>
  <ValidatedInput ...>
</TitleSelect>

and in TitleSelect

return (
  {this.props.children}
}
wkerswell commented 9 years ago

Ah I see now. Saves me from looking in the wrong place.

mszyndel commented 9 years ago

I think a proper solution to that problem would be to create a Fieldset component similar to Form and use it for sub-forms. I will try to make a pull request.

heilhead commented 9 years ago

The problem is that in the case with custom component, it's inner children are not in the props object. Therefore child.props.children is empty. Proper solution would be to migrate to contexts, but I don't have time for that atm. Maybe I'll have on the weekend, not sure. For now, I would recommend just splitting render() method (e.g. have renderTitleSelect()). There is no quick fix for this, sorry.

mszyndel commented 9 years ago

@heilhead thanks for answering! Could you point me in right direction, maybe I can figure something out?

heilhead commented 9 years ago

You can take a look at how contexts work in this article Keep in mind that feature is still not finalized (as most of react...). Also, there's a problem with two different context types: owner & parent. At the moment, react uses owner context, which means that inputs placed inside a form, won't get the context from the form (parent), but from the owner component (which is probably some random div that wraps everything in your main render()). I'll see what I can do, but probably we'll have to wait for 0.14 release of react and hopefully contexts will be parent based there. You can read more about the context problem here.

mszyndel commented 9 years ago

Luckily for me the owner component can be Form if I understand it right (it's top element in my main render, I just include react parts inside my normal website)

heilhead commented 9 years ago

There's one ugly workaround that can be used until we have proper contexts. It would be to pass form somehow to all the custom elements inside a form. And within a custom element you would take the form and pass it along to the inputs. But it's really ugly and I would not want to do that. I'll try to see if there's a way to do it pretty.

wkerswell commented 9 years ago

Thanks for looking into it, really is a big help. I think it is a useful feature that people will use. I do really like the library.

crazyUI commented 9 years ago

Is there a work around for this issue ? Any idea by when this can be fixed..

heilhead commented 9 years ago

Sorry, don't have time right now to experiment with workarounds. We should probably wait for react team to implement proper contexts, hopefully in 0.15.

impleri commented 8 years ago

What about using context like react-router-bootstrap does for its LinkContainer? Have Form provide the contect and an InputContainer (or ValidatorContainer) wrap the react-bootstrap Input component, passing bsStyle, etc as a props on render.