Provide form_with as a new alternative to form_for/form_tag

Question

Provide form_with as a new alternative to form_for/form_tag

dhh opened this issue 8 years ago · 36 comments

form_tag and form_for provide two very similiar interfaces to much of the same thing. We should unify that usage and subsequent calls to field tags. Additionally, there are several deficiencies with the current helpers that I'd like to solve at the same time, now that we will have a new form method to change defaults with:

Don't set DOM id or classes on the form or the fields any more. It frequently created duplicate ids and its not used often enough to warrant being a default behavior. You can easily add the id or class if need be.
Don't require HTML tags, like class and id, to be wrapped in a html: {} key. There aren't a high likelihood of conflict and it complicates the default cases.
Allow form fields that do not correspond to model attributes. This makes it easier to mix and match a form with some fields that correspond to a model and others that will trigger other behaviors (like sending a welcome email).
Make remote: true the default. Full-page changes after submissions are rough. When using Turbolinks, a normal redirect will generate a Turbolinks.visit() call, and otherwise there's SJR. (We could consider having config.action_view.forms_remote_by_default that you could set to false, for people going old school).

Examples:

# Passing model: @post will 1) set scope: :post, 2) set url: url_for(@post)
form_with(model: @post) do |form|
  form.text_field :title # Will reference @post.title as normal
  form.text_area :description, "Overwrite @post.description if present, if not, it will still work"

  form.submit
end

form_with(scope: :post, url: posts_path) do |form|
  form.text_field :title # post[title]
  form.text_area :description, "Overwrite @post.description or ignore if it's not present"

  form.submit
end

# Submits title=X&description=Y
form_with(url: different_path, class: 'something', id: 'specific') do |form|
  form.text_field :title, 'This has is the value of the title'

  form.text_area :description, class: 'No value has been supplied here'

  form.fields(:permission) do |fields|
    # on/off instead of positional parameters for setting values
    fields.check_box :admin, on: 'yes', off: 'no'
  end

  form.select :category, Post::CATEGORIES, blank: 'None'
  form.select :author_id, Person.all.collect { |p| [ p.name, p.id ] }, blank: 'Pick someone'

  form.submit
end

There's still a fair amount of design work to deal with, especially around the FormOptionsHelper. We should move away from positional parameters entirely and replace them with named ones. But we should also try to cut down on needless options and pick better defaults. And finally we should simply drop a lot of the overly-specialized select option methods.

Note: For 5.x, form_for and form_tag should just be soft deprecations, no warnings. Then we can deprecate with a warning, perhaps, in Rails 6.x.

PritiKumr commented 7 years ago

👍

Answer 1 · 2016-05-30T15:55:43.000Z

I can give this a try.

Answer 2 · 2016-05-30T18:14:21.000Z

Ideally we could implement form_for and form_tag on top of this and move it to a gem, which would lower the maintenance cost if we choose not to deprecate it (It would be incredibly painful to deprecate those)

Answer 3 · 2016-05-30T19:20:24.000Z

That'd work for me.

On May 30, 2016, at 20:15, Sean Griffin notifications@github.com wrote:

Ideally we could implement form_for and form_tag on top of this and move it to a gem, which would lower the maintenance cost if we choose not to deprecate it.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Answer 4 · 2016-06-28T08:26:11.000Z

@dhh how would you create a nested form in this way? Just pass an array to :url?

form_with(model: Comment.new, url: [ @post, Comment.new ]) do |form|
  # ...
end

Answer 5 · 2016-06-28T11:54:49.000Z

No, I would stick with that as a parameter to model. So it's form_with(model: [ @post, Comment.new ]) do ...

Answer 6 · 2016-06-28T20:18:34.000Z

I just created a pull request with a prototype - far from ready solution yet.

My considerations so far:

Is the general approach right (new module, new builder, using tag helpers (ie text_field) in implementation, note: I assume FormWithBuilder will not be inheriting from FormBuilder in final implementation).
Do we plan to deprecate tag helpers (ie text_field) at some point?
Implementation of url only forms (ie form_with(url: different_path) ...) - seems to be far away from model or url+scope (ie form_with(scope: :post, url: posts_path)). I think about using separate builder based on pure tag helpers (ie text_field_tag instead of text_field) would be a good idea here (one abstract problem - building tags, to concrete implementations depending on parameters), but would make it more difficult to create custom builders.
Any thoughts on the syntax of following?

Allow form fields that do not correspond to model attributes

Your feedback will be appreciated :)

@kaspth @dhh @vipulnsward

Answer 7 · 2016-07-10T11:02:03.000Z

Just updated PR (view here). All @dhh examples from this PR are now covered.

Questions:

If id are not set by default, how do we handle for attribute in labels? i.e.:

<label for=?>Title</label>
<input type='text' name='post[title]' value='Catch 22' />

For select, collection_select, etc helpers will benefit from named parameters, i.e.

Before:

f.select :category, categories, { include_blank: "Select one" }, class: "nice"
f.collection_select(:post, :author_name, :authors, :id, :name, { include_blank: "Select one" }, class: "nice"

After:

f.select :category, categories, blank: "Select one" , class: "nice"
f.collection_select :post, :author_name, :authors, :id, :name, blank: "Select one", class: 'beauty'

Is there anything more clever we can do here?

Form fields now are allowed not correspond to model attributes. i.e.:

f.text_field :title
f.text_field :title, scope: 'custom'
f.text_field :title, scope: nil

Will generate:

<input type='text' name='post[title]' value='...' />")
<input type='text' name='custom[title]' value='...' />")
<input type='text' name='title' value='...' />")

Looks good?

On implementation side:

Builder now uses Tag::Base family instead of form tags helpers, so it will be easy to deprecate helpers.
Still todo: nested forms, get rid of old form_builder as a base, tests for all the fields methods and cases, docs

Your feedback will be appreciated @kaspth @dhh @vipulnsward.

Answer 8 · 2016-07-11T17:01:17.000Z

Labels: Let's see what it looks like when the label code just requires an id to be set manually on the element.
select/collection_select: Not sure I understand the question? Looks like using named stuff is just nicer?
Form fields: Look good, but we should be using HTML5 tags, so close with > not />.

Answer 9 · 2016-07-11T18:56:02.000Z

@dhh

Labels

Labels: Let's see what it looks like when the label code just requires an id to be set manually on the element

I see 3 options:

Option 1: Manually add for and id:

f.label(:title, "The Title")      # or f.label("The Title", for: 'title')
f.text_field :title, id: 'title'

translates to:

<label for='title'>The Title</label>
<input type='text' name='post[title]' id='title'/>

Option 2: We can recommend to use nesting feature of labels:

f.label("The Title") do
  f.text_field :title
end

or shorter version:
f.label("The Title") { f.text_field :title }

or even shorter - label as an extra param of text_field:
f.text_field :title, label: "The Title"

which would translate to:

<label>The Title <input type='text' name='post[title]' /></label>

Option 3: Keep the old semantics and generate id i.e.

f.label :title, "The Title"
f.text_field :title

translates to:

<label for='post_title'>The Title</label>
<input type='text' name='post[title]' value='Catch 22' />

select helpers

select/collection_select: Not sure I understand the question? Looks like using named stuff is just nicer?

Is there anything else we would like to do (as we already touching *_select helpers) other then introduce named parameters and revisit parameter names?

Answer 10 · 2016-07-11T20:36:02.000Z

Whatever we decide with labels if would be nice to tie them to checkboxes if that's the field you're rendering. I think that's a good default for Rails to help out with. Which we could solve with the :label option.

@dhh I'm curious what you mean by this in your original comment:

form.text_area :description, "Overwrite @post.description if present, if not, it will still work"

Why should it only overwrite the description if present? And how will it work if the @post has no description?

Is there anything else we would like to do (as we already touching *_select helpers) other then introduce named parameters and revisit parameter names?

I think we have to drop down to actual code to be able to spot that.

Answer 11 · 2016-07-12T02:42:01.000Z

Perhaps ids could still be generated for use with <label for="">, but it could be made obvious that they are generated. For example, choose random number when the builder is instantiated (in case there are multiple builders per page) and concat that number with an internal counter which is incremented per field.

Regardless of id generation, I like the f.text_field :title, label: "The Title" API. Using label: false could skip the label, and a default of label: true could produce whatever f.label :title would. This would give the built-in form builder most of the brevity that popular (but bloated) form builder gems have, which I think is their chief selling point.

Answer 12 · 2016-07-12T19:45:41.000Z

@jonathanhefner We could also add parameter to form_for tag, to generate (or not) labels by default for given form:

form_with model: @post, labels: true do
...
end

Answer 13 · 2016-09-15T18:33:59.000Z

@dhh Do you envision form_with to be used by scaffolds as part of this issue?

For instance, would rails g scaffold post create a posts/_form.html.erb partial that starts with

<%= form_with(pizza) do |f| %>

replacing the current

<%= form_for(post) do |f| %>

?

Answer 14 · 2016-09-15T20:17:53.000Z

Can't see why we wouldn't use form_with in scaffolds, so yes 👍

Answer 15 · 2016-09-15T20:24:33.000Z

Indeed. I would consider form_for soft-deprecated after we get form_with.

On Sep 15, 2016, at 15:18, Kasper Timm Hansen notifications@github.com wrote:

Can't see why we wouldn't use form_with in scaffolds, so yes 👍

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Answer 16 · 2016-09-16T11:50:55.000Z

Late to the party on this one, but I really like form_with instead of having two different methods that do almost the same thing. This is a step in the right direction.

Answer 17 · 2016-11-13T17:43:27.000Z

What's the value in using positional args over a named one for the value of fields?

text_field :title, "override value"
# Versus:
text_field :title, value: "override value" # Haven't checked if this works out of the box yet

I think the former will force us to revise the signature on every field method in the FormHelper as well as the FormBuilder.

Answer 18 · 2016-11-15T13:51:31.000Z

value is fine by me.

Answer 19 · 2017-01-02T20:52:12.000Z

Going to mark this as done for 5.1 and pull it off the milestone, then we can dig further in 5.2 👍

Answer 20 · 2017-02-24T10:06:39.000Z

Good work all, looking forward to playing around with this on the 5.1 beta! :)

Answer 21 · 2017-03-18T09:19:37.000Z

Hey, it appears the authenticity token is not added to a form created by form_with helper, but it's added to a form created with form_tag

Answer 22 · 2017-03-20T18:33:41.000Z

@sevos form_with generates remote forms by default, which don't output authenticity tokens in the form but rather fetch it from a meta tag in the head on submit. 😬

Answer 23 · 2017-03-20T18:36:03.000Z

Somehow it does not play along with Turbolinks. I've had to fall back to form_tag.
What was special about my form? It was a form with single file upload field (multipart: true)

Answer 24 · 2017-03-20T18:54:28.000Z

@sevos general help is best sought on Stack Overflow. We keep the issue tracker to bugs only.

But if you can find a bug and reproduce it in a sample app, we can take a look at that.

Answer 25 · 2017-03-21T03:20:31.000Z

form_with generates remote forms by default, which don't output authenticity tokens in the form but rather fetch it from a meta tag in the head on submit. 😬

Is that true even with per form authenticity tokens enabled?

Answer 26 · 2017-03-21T03:46:42.000Z

I will try to isolate the problem!

Answer 27 · 2017-03-21T19:40:32.000Z

@sevos So there is an issue here after all, thanks for reporting! Since form_with defaults to remote forms it's best we embed authenticity tokens in the forms, so people don't have to rely on rails-ujs.

Currently the workaround is to either bundle rails-ujs or set config.action_view.embed_authenticity_tokens_in_remote_forms = true in application.rb.

👆 @samandmoore yup, when that config's enabled it works.

Answer 28 · 2017-03-25T16:10:18.000Z

Also: long since closed through #26976 and follow up commits.

Answer 29 · 2017-04-19T08:09:24.000Z

@kaspth regarding the token issue: if remote is false and token is true, there still no tokens rendered

#28796

Answer 30 · 2018-01-01T21:08:37.000Z

Sorry I'm a little bit late to the party, but would you guys mind giving me a pointer?
https://stackoverflow.com/questions/48053022/rails-5-1-strong-parameters-deprecated

I've read the repo's code and I'm still confused. Is there no way to use from_with for a nested route? For example, users/:user_id/friendships/:id

Sorry for the newb question! I read through the comments thoroughly and didn't spot an example of that.

Maybe if I understand I can help you guys out with documentation in the future? lol. I've always wanted to contribute to Rails

Answer 31 · 2018-07-27T11:04:17.000Z

Not sure if this is mentioned elsewhere, but do we need to update actionview guides to reflect this addition? https://guides.rubyonrails.org/form_helpers.html

Lots of talk about form_tag and form_for but no form_with. If someone can verify I'm more than happy to take a crack at it. Thank you! :)

Answer 32 · 2018-07-27T15:12:33.000Z

Yes, please do take a stab at it. We should use form_with wherever possible.

…

On Fri, Jul 27, 2018 at 4:05 AM, Nick Schwaderer ***@***.***> wrote: Not sure if this is mentioned elsewhere, but do we need to update actionview guides to reflect this addition? https://guides.rubyonrails. org/form_helpers.html Lots of talk about form_tag and form_for but no form_with. If someone can verify I'm more than happy to take a crack at it. Thank you! :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#25197 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAKtb127Tue8YeDDzrc-k7TahKWugWGks5uKvQHgaJpZM4Ip4zx> .

Answer 33 · 2018-07-28T11:28:29.000Z

@Schwad looks like I forgot to add it, please do! You should be able to copy a bunch of things from
https://api.rubyonrails.org/classes/ActionView/Helpers/FormHelper.html#method-i-form_with

Answer 34 · 2018-08-10T10:14:00.000Z

Okay, I've whipped together a PR. Would appreciate a peek from @dhh or @kaspth .

Essentially I did what you recommended @kaspth - I informed the documentation from what you put together there and formatted it for the guide. I think if form_with does become the default convention (it's all I use now) then a second PR should be considered to put it ahead of form_for and form_tag in the guides.... But for now this at least includes the existing documentation in the full guide.

#33523

Answer 35 · 2021-06-08T19:22:17.000Z

For anyone googling why their form labels aren't showing up by default as per the docs, it appears the decision on form labels and field id's for some subset of Rails 5.x.x was to add a "for" option to labels and require explicitly using an "id" option on fields. So calling something like

<%= form.label :url, "URL", for: :url %>
<%= form.url_field :url, id: :url %>

is probably what you want