Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

There was a problem

You must be a member of this account.

This project is archived and is in readonly mode.

[PATCH] form_for for new objects with :index set

#3226

If you have multiple forms for new objects of the same object type in a template, currently rails will create multiple form tags with the same DOM id which is clearly syntactically invalid HTML.

Example:

form_for(@post, :index => 5)
form_for(@post, :index => 6)

Current Behavior:
<form class="new_post" action="/posts" method="post" id="new_post"></form> <form class="new_post" action="/posts" method="post" id="new_post"></form>

New Behavior:
<form class="new_post" action="/posts" method="post" id="new_post_5"></form> <form class="new_post" action="/posts" method="post" id="new_post_6"></form>

When no index is passed it defaults back to the original behavior.

This patch modifies form_helper.rb and includes a new test in form_helper_test.rb to cover this case.

Cheers,
Stephen Blackstone

Reported by Stephen Blackstone · September 17th, 2009 @ 07:59 PM

State: wontfix
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Stephen Blackstone
    Stephen Blackstone

    Recreated same patch using git master instead of manually diffing rails 2.3.4

    September 18th, 2009 @ 03:56 PM

  2. JasonKing
    JasonKing

    As mentioned in the core mailing list...

    I wonder if there's room in your patch for something feeding automatically from the object? As well as having the explicit option with the :index value, perhaps you could also accept :index => true and use .to_param of the object in question in that case?

    September 18th, 2009 @ 05:43 PM

  3. CancelProfileIsBroken
  4. Stephen Blackstone
    Stephen Blackstone

    Just to keep everything together - the conversation from this ticket went on on [Rails Core]....


    Stephen Wrote:

    "I'm not sure about passing :index => true since form_for passes along the value of index to fields_for... It seems like this is already the case for forms when the object is not a new object, i.e.
    form_for(@user) ..... etc
    yields
    the only case where we seem to end up with an ambiguous form tag is multiple forms to create the same object type.."


    and then Jason replied back:

    Yes, of course, you're right. I had been thinking of the edit scenario in fields_for (which I think would be cool if it did what I suggested when passed true). The excitement of the coincidence confused me, and I didn't really read properly that you were talking about new objects.

    +1 on your patch. It makes sense to be consistent between that and what is already done in tags.

    September 27th, 2009 @ 04:27 PM

  5. Mike Enriquez
    Mike Enriquez

    -1 The same result can be accomplished by doing

    form_for(@post, :html => { :id => "new_post_6" })
    

    The patch provides a cleaner syntax, but I don't think it is common enough to be included into core.

    September 27th, 2009 @ 08:26 PM

  6. Gaius Centus Novus
    Gaius Centus Novus

    -1 I agree with Mike Enriquez

    September 27th, 2009 @ 10:44 PM

  7. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag cleared.
    • State changed from new to wontfix

    September 28th, 2009 @ 01:32 AM