Refactoring a Rails view

Question

I'm new to Rails development, so please bear with me. I'm creating a view that has several fields on it that look very similar. It's just begging to be refactored somehow but I haven't been able to figure it out. See code sample below:

<%= form_for(@tapelog) do |f| %>
  <div class="container">
    <div class="field span-16">
      <div class="span-8 labelRight">
        <%= f.label :client %>
      </div>
      <div class="span-8 last">
        <%= f.collection_select :client_id, Client.find(:all), :id, :name,
                                { :prompt => "Select a Client..." },
                                { :class => "automatixSelect" } %>
      </div>
    </div>
    <div class="field span-16">
      <div class="span-8 labelRight">
        <%= f.label :employer %>
      </div>
      <div class="span-8 last">
        <%= f.collection_select :employer_id, Employer.find(:all), :id, :name,
                                { :prompt => "Select an Employer..." },
                                { :class  => "automatixSelect" } %>
      </div>
    </div>
  ....
<% end %>

There are about 7 fields like that. I tried to put them all into partials just so I could reduce the clutter on this page but that errors out because 'f' is not defined. Any ideas on how I could reduce some of the clutter here? Any other tips in general on Ruby refactoring would also be welcome.

Thanks, -- A.

is this like a full refactoring? first off, you can take out all those divs out. proper form structure usually takes the form of a fieldset and an unordered(or ordered) list of form elements. take a look here: github.com/justinfrench/formtastic — corroded
– corroded, Commented Jun 7, 2011 at 1:21
Would love to explore that some more. It's going a little over my head right now, but I've bookmarked it for when I get more than 6 days of Rails coding experience :) — Adnan
– Adnan, Commented Jun 7, 2011 at 1:31
no prob, its best if you look for plugins that might help you. it saves you a TON of coding and work :). also i see you're using a grid framework(960 or blueprint?) you can also use the compass gem( compass-style.org ) so you dont have to add those classes to your markup. it's all in the css where it should be! :) — corroded
– corroded, Commented Jun 7, 2011 at 2:40

LuisVM · Accepted Answer · 2012-07-06 19:19:26Z

3

If you want to use the 'f' in the subsequent partials, pass it as a parameter

<%= render :partial => :some_partial, :locals => { :f => f } %>

edited Jul 6, 2012 at 19:19

answered Jun 7, 2011 at 1:15

LuisVM

2,8333 gold badges22 silver badges22 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Dan Croak · Accepted Answer · 2011-06-07 02:30:57Z

The first thing I'd do is rename the "f" local variable to "form" for clarity. Then, extracting the code to a partial is reasonable:

<div class="field span-16">
  <div class="span-8 labelRight">
    <%= form.label :employer %>
  </div>
  <div class="span-8 last">
    <%= form.collection_select :employer_id, Employer.find(:all), :id, :name,
                               { :prompt => "Select an Employer..." },
                               { :class  => "automatixSelect" } %>
  </div>
</div>

It looks to me like this is a belongs_to relationship, so I might create a partial called "belongs_to" and render it like:

<%= render :belongs_to, :parent => :employer, :form => form %>

The idea is we'll have a local named "parent" that we can alter in the partial. Note that I used the shorthand partial syntax. It's the same as:

<%= render :partial => :belongs_to, :locals => { :parent => :employer, :form => form } %>

<div class="field span-16">
  <div class="span-8 labelRight">
    <%= form.label parent %>
  </div>
  <div class="span-8 last">
    <%= form.collection_select :"#{parent}_id", parent.to_s.capitalize.constantize.find(:all), :id, :name,
                               { :prompt => "Select an #{parent.to_s.capitalize}..." },
                               { :class  => "automatixSelect" } %>
  </div>
</div>

This is good. I can use this to have a single partial for all my "drop down select" type fields. Nice!

Collectives™ on Stack Overflow

Refactoring a Rails view

2 Answers 2

Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Related