team-formalist / formalist-rb

Flexible form builder
MIT License
22 stars 4 forks source link

Post plain form data from the forms, instead of the AST #20

Closed timriley closed 8 years ago

timriley commented 8 years ago

Right now we don't include any extra information in the AST sent back from the form on the page that isn't already in the form's own definition on the server-side.

Could we consider just posting plain HTML form elements instead?

Things we'd need to handle:

makenosound commented 8 years ago

We could, though I think it’d be conceptually neater to have it as an aside to the main set of inputs. It could be another form renderer that just takes the AST and outputs correctly named hidden inputs with their values.

timriley commented 8 years ago

Dear @makenosound,

We could, though I think it’d be conceptually neater to have it as an aside to the main set of inputs.

Yep, this is fair enough.

It could be another form renderer that just takes the AST and outputs correctly named hidden inputs with their values.

Is your thinking here that this would be a separate component within the current form renderer that you're building now, basically something to handle adding data to the eventual form post based on the AST (as opposed to handling the display of UI based on the AST)?

If so, I think that would be great. We'll just have to have a discussion about what conventions to pick to shim e.g. arrays of objects into the form data.

Let me know what you think :)

makenosound commented 8 years ago

Is your thinking here that this would be a separate component within the current form renderer

Yeah, something like that. There are a couple of different ways to approach it I think, we could either:

  1. Have a totally separate form renderer that is only responsible for rendering hidden inputs. You’d have it subscribe to changes in the visible form AST and then clobber its state each time.

    Downside here is that you’re duplicating the state, and you need to have the visible inputs sit outside <form> probably.

  2. Have some way of passing in a little extra rendered context to the react-standard renderer so the main visible form can have those hidden inputs sit side-by-side with the visible ones. Downside is just that it’s messier.
timriley commented 8 years ago

Seems like (1) is definitely cleaner. I wonder if we could do it on the form's submit event (i.e. just in time), so it's a one-off operation and doesn't need to subscribe to changes?

I do wonder whether we really need this, but it's true that there's absolutely nothing in the returned AST right now that we don't already know from the form's server-side definition, so it really is about just wanting the keys/values. And having them returned as a conventional HTML form post would give us more flexibility to use them on the server (and an established convention for doing this would help with non JS-heavy renderers later).

That said, I think this is not critical for our MVP, since we already have things working with the AST submission.

makenosound commented 8 years ago

I wonder if we could do it on the form's submit event (i.e. just in time), so it's a one-off operation and doesn't need to subscribe to changes?

Yeah, good idea, that’s totally possible.

timriley commented 8 years ago

From @solnic's feedback, sounds like we should try and make this happen reasonably soon. Once our apps’ operation objects are using formalist form objects directly, their input data is then required to match the form object's input data, and if that is a crazy-large AST-as-json, it becomes pretty awkward to work with those objects in isolation, like in tests. And ease of testing is a bit part of why we have them as standalone operations.

makenosound commented 8 years ago

@timriley Yeah, I don’t think this would be particularly hard — happy to take a look at today.

timriley commented 8 years ago

Cool, thanks @makenosound.

timriley commented 8 years ago

@makenosound has done this an it is excellent!