This project is archived and is in readonly mode.
Allow explicit placement of hidden fields with nested models
Reported by Will Read | September 25th, 2009 @ 09:40 PM | in 2.x
When using fields_for to build DOM structure for nested models, the id hidden field gets emitted implicitly and the developer has not control over the placement which can result in invalid HTML. As an example:
class Foo < ActiveRecord::Base has_many :bars accepts_nested_attributes_for :bars end
and the template
<% form_for @foo do |foo_form| %>
<table>
<tbody>
<% foo_form.fields_for :bars do |bar_fields| %>
<tr>
<td><%= bar_fields.text_field :name %></td>
</tr>
<% end %>
</tbody>
</table>
<% end %>
The hidden input gets placed at the point of the call to fields_for, right after the TBODY, which results in invalid HTML. The consequence is that some browsers, eg. Safari, try to fix the generated HTML for you, resulting in an inconsistent DOM structure across browsers.
I added a method, #hidden_fields, to FormBuilder to explicitly place the hidden fields. If this methods gets called in the #fields_for block the FormBuilder won't implicitly emit the hidden fields.
For example:
<% form_for @foo do |foo_form| %>
<table>
<tbody>
<% foo_form.fields_for :bars do |bar_fields| %>
<tr>
<%= bar_fields.hidden_fields %>
<td><%= bar_fields.text_field :name %></td>
</tr>
<% end %>
</tbody>
</table>
<% end %>
I've attached a patch with the new functionality, including tests.
Comments and changes to this ticket
-
Eloy Duran September 28th, 2009 @ 12:53 PM
I edited your ticket to fix the formatting. Parts of your HTML were missing.
-
Eloy Duran September 28th, 2009 @ 01:10 PM
- Assigned user set to Eloy Duran
I like the idea. However, as we have no other hidden_fields yet, I think it feels a bit heavy. How about just checking if a hidden field for :id was created? Not sure if that would make the code much easier though… Thoughts?
-
Adam Milligan September 29th, 2009 @ 03:30 PM
What about the destroy (née delete) hidden field? Does the form builder not also output that if the model is marked for destruction?
-
Will Read October 8th, 2009 @ 02:53 AM
Elloy, I've taken your suggestion into consideration and created a new patch. The patch now does exactly as you recommended and only checks if a hidden field for :id was created.
-
Tor Erik October 8th, 2009 @ 09:47 AM
+1 for this functionality. And yes, will this cater for the destroy/delete field as well?
-
Eloy Duran October 8th, 2009 @ 07:06 PM
- Milestone set to 2.3.6
@Will Looks good, I'll check it out and apply it probably somewhere next week. Thanks!
@Adam & Tor: Nope, that has to be handled by the user. By either adding a hidden_field or checkbox depending on the situation. I'll happily review patches for this.
-
Will Read October 18th, 2009 @ 04:53 AM
Changed method and variable name "emitted_hidden_field" to "emitted_hidden_id" to be more succinct. Eloy, if you haven't applied this patch yet, please use the latest attachment.
-
Repository November 17th, 2009 @ 03:03 AM
- State changed from new to resolved
(from [88d2e4ca6fdf0d0df9d7d71bae1ecbde4f1d46e2]) Allow explicit placement of hidden id element for nested models.
[#3259 state:resolved]
Signed-off-by: Eloy Duran eloy.de.enige@gmail.com
http://github.com/rails/rails/commit/88d2e4ca6fdf0d0df9d7d71bae1ecb... -
Repository November 17th, 2009 @ 03:03 AM
(from [7fadb3f261fc7cc753d303d0fbe2cbf019385f99]) Allow explicit placement of hidden id element for nested models.
[#3259 state:resolved]
Signed-off-by: Eloy Duran eloy.de.enige@gmail.com
http://github.com/rails/rails/commit/7fadb3f261fc7cc753d303d0fbe2cb... -
W. Andrew Loe III December 1st, 2009 @ 01:09 AM
This went out with 2.3.5, it should probably be on the 2.3.5 milestone.
-
Eloy Duran December 1st, 2009 @ 01:34 PM
- Milestone changed from 2.3.6 to 2.x
@Andrew: I'm not sure what it is that you mean, but the commit is in 2.3.5: see the commit on 2009-11-15 http://github.com/rails/rails/commits/v2.3.5
-
Eloy Duran December 1st, 2009 @ 01:35 PM
Ah, do you mean that the ticket's milestone should read 2.3.5? :)
If so, I understand, alas, I don't have that much authorization…
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 3259 Allow explicit placement of hidden fields with nested models [#3259 state:resolved]
- 3259 Allow explicit placement of hidden fields with nested models [#3259 state:resolved]
- 3416 fields_for renders invalid html This appears to be a duplicate of the (now resolved) #3259