form_for and partials

I love form_for, but I really hate

<% form_for :person, @person, :url => {...} do |f| %>    ...    <%= render :partial => 'form', :locals => {'f' => f}    ... <% end %>

I've been thinking of instead allowing

<% form_for :person, @person, :url => {...} do |f| %>    ...    <%= f.partial 'form' %>    ... <% end %>

f.partial would be exactly the same as the previous snippet, just making the form builder available in the partial without the hassle of :locals => {'f' => f} the one thing I'm not sure about is how you pick the name under which the form builder should appear in the partial.

Ideally f.partial 'form' would give you something sensible, but I'm not sure how you would pick such a default

f.partial 'form', :name => 'f' would work, but then it's almost as awkward as :locals => {'f' => f}

Any ideas (or is this one just stupid ?) ?

Fred

I've been thinking of instead allowing

<% form_for :person, @person, :url => {...} do |f| %>    ...    <%= f.partial 'form' %>    ... <% end %>

f.partial would be exactly the same as the previous snippet, just making the form builder available in the partial without the hassle of :locals => {'f' => f}

Looks great!

the one thing I'm not sure about is how you pick the name under which the form builder should appear in the partial.

Well, we could leave a simple "f" by default. Most people are probably used to that, right?

Ideally f.partial 'form' would give you something sensible, but I'm not sure how you would pick such a default

Why not something like:

-- environment.rb Rails::Initializer.run do |config|   config.action_view.form_builder_name = :f end

And even:

config.action_view.form_builder = :labelled_form_builder

To always have form_for give you a custom builder.

f.partial 'form', :name => 'f' would work, but then it's almost as awkward as :locals => {'f' => f}

We could leave that option for the very rare case where you want to override the local variable name on a specific partial...

Any ideas (or is this one just stupid ?) ?

I wonder if this is the reason why the scaffold_resource generator doesn't deal with form partials like scaffold did? (ie the lack of a nice way to do it).

Overall, +1!

Why not use name of the partial to refer to form object ?

I hate the form :partial, :locals thing too... feels dirty.

I like the idea of taking the partial name as the local name... that's how things like the :collection partial works already.

Seems like this one has the people's vote. The one think holding me back would be that this conflicts with the current behaviour of partial (ie the object/collection with the write name materialising). It would be pretty weird to have objects called @something_form, but what if you did want to build up a form by iterating over some collection ?

Fred

+1 but I think it might be too succinct for newbies. I guess it’s okay if you think of it as an alternative syntax, like sexy migrations.

Shane

What about...

<%= f.render :partial => 'form' %>

?

And it would still perhaps allow all the usual render params to be passed to it. I'm just thinking of this quickly, haven't thought about it too too much. :slight_smile:

Consistency! +1

  • Mislav

I love form_for, but I really hate

<% form_for :person, @person, :url => {...} do |f| %>    ...    <%= render :partial => 'form', :locals => {'f' => f}    ... <% end %>

I've been thinking of instead allowing

<% form_for :person, @person, :url => {...} do |f| %>    ...    <%= f.partial 'form' %>    ... <% end %>

Isn't this what :object => f is for?

<% form_for :person, @person, :url => {...} do |f| %>    <%= render :partial => 'fields', :object => f <% end %>

_fields.erb:

<%= fields.text_field :name %> (local variable is the name of the partial) <%= fields.text_field :age %>

Joe

It's just a bit of convenience - neither <render :partial => 'fields', :object => f nor <%= render :partial => 'fields', :locals => {'f' => f} are very nice, compared to just "render :partial => form" as one can do with form_tag

Fred

I'm pretty much sold on this. The one thing is of course that it
doesn't have to boil down to a single name. The form builder can
easily be made available under more than one name if there's a demand
for that (so you could just always make it available as f, but I
think that would be a bit ugly.

Or is this super evil :wink:

class FormBuilder    def partial(name, options={}, &block)      builder_name = nil      block_locals = eval('local_variables', block.binding)

     block_locals.each do |local|        value = eval(local, block.binding)        builder_name = local and break if value == self      end

     @template.render options.merge(:partial => name, :object =>
self, :locals => {builder_name => self})    end end

form_for(...) do |f|    f.partial('form'){} end

this will work find what name (or rather one of the names) the form
builder had when the partial method was called

Fred

You can already do:

  <%= render :partial=>'form', :object=>f %>

Which at least avoids the duplication in :locals.

It seems funny to me to have the form builder classes know about rendering partials, or even rendering at all?

To a large extent I'm nostalgic about being able to do <%= render :partial=>'form' %>. :object=>f is better than :locals (should of thought of that one), but it still seems a little redundant to have to say 'and yes I do actually want my form builder available in the partial. Maybe the form builder classes aren't the place for this but I couldn't think of another way.

Fred

Maybe the form builder classes aren't the place for this but I couldn't think of another way.

Perhaps:

<% form_for ... do |f|   <%= render :partial=>f %> <% end %>

The enhance the polymorphic code to take account of FormBuilder and make a FormBuilder for a Customer build:

customers/_form.format.erb ?

so in render_partial, case FormBuilder does render :partial => "#{f.object.class}/_form.format.erb", :object => f (apologies for pseudoness of code) ?

As long as you don't have more than one form per model that you want easily accessible (and even if you did have that I suppose that falls under the general 'make the common case easy etc...', as it's not that bad having to type out the :object if you do have some complicated nesting/set of form partials.

Fred

Neat idea, but that would mean tight coupling of render logic with view helper(s), which IMO is not a path to follow.

so in render_partial, case FormBuilder does render :partial => "# {f.object.class}/_form.format.erb", :object => f (apologies for pseudoness of code) ?

Yeah, that's what I've been doing, and depending on the implementation required it seems like it should be ok...

I can see mislav's point on coupling, but it seems consistent with the current behaviour where:

render :partial=>@foo -> /foos/_foo.html.erb

http://pastie.caboo.se/pastes/101420

This simple patch allows for render :partial => f, begin f a FormBuilder, to render /_form.html.erb, and being f a LabelledFormBuilder to render /_labelled_form.html.erb. It looks for any FormBuilder subclass, and also sets the local variable name to form, labelled_form, or whatever form builder you're using...

I don't know if there's a ticket already for this, and if anybody wants this applied anyway... :slight_smile:

Digging through some old stuff, I don't think this discussion every really came to a conclusion. I still feel the burn every time I write render :partial => 'form', :object => f, and I believe the solution underlined below takes care of it most of the time

Fred

The solution Fred mentions, as a proper patch with tests:

http://dev.rubyonrails.org/ticket/10814

Comments, +1's, everything is welcome :slight_smile: