stouset / twitter_bootstrap_form_for

A Rails FormBuilder DSL for generating Twitter Bootstrap forms
https://github.com/stouset/twitter_bootstrap_form_for
MIT License
409 stars 110 forks source link

Custom form helpers results in multiple labels #32

Closed jperrine closed 12 years ago

jperrine commented 12 years ago

I have a custom form field that I created that just passes off a value and class to a text field and when using it in a twitter_bootstrap_form_for it results in a label being wrapped around the element twice.

The code for my form field is the following:

def date_field(field_name, index, options={})
  options.merge!(:value => object.send(field_name).to_s, :class => "datePicker")

  text_field field_name, options
end

and the code for my form is the following:

= twitter_bootstrap_form_for([@event, @day]) do |f|
  = f.inputs 'Day' do
    = f.text_field :name
    =f.date_field :date
  = f.actions do
    = f.submit "Save"
    = button_tag 'Cancel', :type => 'reset', :class => 'btn'

And the output HTML looks like this:

<div id="day_date_input" class=" clearfix">
  <label for="day_date">Date</label>
  <div class="input">
    <div id="day_date_input" class=" clearfix">
      <label for="day_date">Date</label>
      <div class="input">
        <input type="text" value="12/22/2011" size="30" name="day[date]" id="day_date" class="datePicker hasDatepicker"></div>
      </div>
    </div>
  </div>
</div>

Am I doing something wrong or is this an issue with using custom form helpers with this gem?

stouset commented 12 years ago

This is an unfortunate side effect of how I detect form helpers.

INPUTS = [
  :select,
  *ActionView::Helpers::FormBuilder.instance_methods.grep(%r{
    _(area|field|select)$ # all area, field, and select methods
  }mx).map(&:to_sym)
]

It automatically defines a wrapper around any helper named select, or ending in _area, _field, or _select. This includes your custom helper. So your helper gets a label, then calls another _field method which gives it a second label.

I'm not sure this is something I want to change, since it's a pretty small outside case. Doing it this way gives the plugin a high likelihood of catching any new form helpers Rails adds in the future (as they've done recently for HTML5 helpers). I'm open to suggestion, though.

In the meantime, you have a few options. You can define your helper the way Rails does, which doesn't involve a call to another _field method:

def text_field(object_name, method, options = {})
  InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("text", options)
end

Another option is, I think, to call the method on the FormBuilder's template (this is just a guess, it may or may not work, and it does look kind of crappy):

= twitter_bootstrap_form_for([@event, @day]) do |f|
  = f.inputs 'Day' do
    = f.         text_field :name
    = f.template.date_field :date

Let me know if either of those solutions work for you.

jperrine commented 12 years ago

Cool, thanks for the suggestions it now looks good

stouset commented 12 years ago

Which approach did you use?

jperrine commented 12 years ago

I wasn't able to get the InstanceTag solution to work quite right so I went with the following:

module FormBuilder
  def date_field(method, options={})
    options.merge!(:value => @object.send(method).to_s, :class => "datePicker")
    @template.text_field(@object_name, method, objectify_options(options))
  end
end

ActionView::Helpers::FormBuilder.send(:include, FormBuilder)
stouset commented 12 years ago

Great!