AppliedMathematicsANU / plexus-form

A dynamic form component for react using JSON-Schema.
MIT License
134 stars 31 forks source link

Allow custom props to be passed to Form #19

Closed bebraw closed 9 years ago

bebraw commented 9 years ago

It would be handy if

<Form className='pure-form' ...></Form>

worked. Setting custom properties like this would make it easier to build on top of the library.

Technically you would need to do something like this:

var React = require('react/addons');
var update = React.addons.update;

...

props = update(this.props, {
    $merge: {
        valueToRemove: undefined,
        ...
    },
});

...

// pass props to <form>

This way you won't end up passing custom properties to form.

If you want, I can prepare a PR for you although this isn't a big change for you to make.

odf commented 9 years ago

A think passing className through would definitely be a good idea. This is something I've been doing quite a bit elsewhere. Any others that would make sense?

bebraw commented 9 years ago

A think passing className through would definitely be a good idea. This is something I've been doing quite a bit elsewhere. Any others that would make sense?

You can do <form {...props}> after you nill the properties you don't want to expand. I have a full example of this at https://github.com/bebraw/reactabular/blob/master/lib/table.jsx .

The advantage of this approach is that it gives you a lot of flexibility and you don't need to modify the code if you want to hook up some special property.

odf commented 9 years ago

Hmm, I guess that's probably okay, as long as we don't pass things like method or action through to the underlying <form> element.

bebraw commented 9 years ago

Hmm, I guess that's probably okay, as long as we don't pass things like method or action through to the underlying

element.

Yup. If you are worried about that, you can just exclude those from props.

odf commented 9 years ago

After a bit more reflection, I still think for now I'd prefer to just whitelist a few properties that are common and obviously won't interfere with Form functionality, such as className and styles and possibly a few others. I am sympathetic to the desire to customise the <form> element that ends up being rendered, but rather than just passing props through and having to worry about things breaking and correctly mapping semantics, I think that maybe a "bring your own <form>" approach similar to what we talked about in #20 for buttons makes more sense.

Maybe we could have a FormContent component that provides just the core functionality - constructing lists of fields according to the given schema, plus all the validation, normalisation and parsing support - and a relatively small Form component that provides a default wrapper around it. Users with more extensive customisation needs would then be able to write their own wrappers, which, among other things, would allow them to use arbitrary React components instead of the native <form>.

odf commented 9 years ago

The className property is passed through into <form> as of @20c2c0d68d962a060e57fa935edbe9cfa89b7438. If you need more for your use case, I'm happy to accept a PR. Once you're satisfied with this and #20, I'll release v0.1.1.

bebraw commented 9 years ago

@odf Ok. FormContent could be a good idea. Whitelisting is fine.

I gave the changes a quick go. It's much easier to style the form now. In this case I just applied a couple of styles and rewrote buttons. :+1: https://www.dropbox.com/s/bpdsyb15s9697r1/Screenshot%202015-02-23%2009.37.55.png?dl=0 .

Feel free to go ahead with the release!

bebraw commented 9 years ago

Here is something extra to think about: http://purecss.io/forms/#aligned-form . For that layout to work there would have to be a way to wrap label, control pairs. I guess this is one of those cases that could be handled using FormContent given there are suitable hooks that allow manipulation.

odf commented 9 years ago

That kind of thing should already be possible by passing in an appropriate component factory via the fieldwrapper property. The demo app (which by the way now lives in it's own repo/npm package called react-form-example) has an example of that.

But there are other advanced things that users might want to do within a form, e.g. autocompletable fields or file uploaders. There is a FielField component in plexus-form right now, but I think I'll remove that. It's much better to be modular and allow people to define their own components for special tasks or mix and match. At the moment, one has to specify special input components field by field within the schema, but that's of course super-tedious if one wants, say, to use sliders for all numeric input fields. So I'd like to come up with some more general way to associate input fields with handler components.

ETA: We should probably have a separate issue for discussing these things. I might do a bit more pondering and open one shortly.

bebraw commented 9 years ago

That kind of thing should already be possible by passing in an appropriate component factory via the fieldwrapper property. The demo app (which by the way now lives in it's own repo/npm package called react-form-example) has an example of that.

Great. I missed that. :)

ETA: We should probably have a separate issue for discussing these things. I might do a bit more pondering and open one shortly.

Ok. Field type matching sounds like an interesting issue to resolve. I guess you might end up matching by type (type -> React component map) and have some way to specialize (match based on field name).