-
Notifications
You must be signed in to change notification settings - Fork 21.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide form_with as a new alternative to form_for/form_tag #25197
Comments
I can give this a try. |
Ideally we could implement |
That'd work for me.
|
@dhh how would you create a nested form in this way? Just pass an array to form_with(model: Comment.new, url: [ @post, Comment.new ]) do |form|
# ...
end |
No, I would stick with that as a parameter to model. So it's |
I just created a pull request with a prototype - far from ready solution yet. My considerations so far:
Your feedback will be appreciated :) |
Just updated PR (view here). All @dhh examples from this PR are now covered. Questions:If
For Before:
After:
Is there anything more clever we can do here? Form fields now are allowed not correspond to model attributes. i.e.:
Will generate:
Looks good? On implementation side:
Your feedback will be appreciated @kaspth @dhh @vipulnsward. |
|
Labels
I see 3 options:
translates to:
or shorter version: or even shorter - label as an extra param of text_field: which would translate to:
translates to:
select helpers
Is there anything else we would like to do (as we already touching *_select helpers) other then introduce named parameters and revisit parameter names? |
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 @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
I think we have to drop down to actual code to be able to spot that. |
Perhaps ids could still be generated for use with Regardless of id generation, I like the |
@jonathanhefner We could also add parameter to form_for tag, to generate (or not) labels by default for given form:
|
@sevos So there is an issue here after all, thanks for reporting! Since Currently the workaround is to either bundle rails-ujs or set 👆 @samandmoore yup, when that config's enabled it works. |
Also: long since closed through #26976 and follow up commits. |
The Installation guides do not specify a rails version: `gem install rails --no-document` Therefore the new people will install rails 5.1 (as of today). One of the view changes was the introduction of `form_with` instead of `form_for`: rails/rails#25197 I updated both the 'replace the following code' sections as well as the edit code to follow the new helper standards. (using the variable `form` instead of `f` \o/)
Sorry I'm a little bit late to the party, but would you guys mind giving me a pointer? I've read the repo's code and I'm still confused. Is there no way to use 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 |
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 |
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>
.
|
@Schwad looks like I forgot to add it, please do! You should be able to copy a bunch of things from |
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 |
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
is probably what you want |
form_tag
andform_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:Examples:
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.
The text was updated successfully, but these errors were encountered: