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

Incorrect Error Markup #23

Closed derekprior closed 12 years ago

derekprior commented 12 years ago

In the twitter bootstrap documentation, a field with errors is presented with the following markup:

<div class="clearfix error">
  <label for="errorInput">Input with error</label>
    <div class="input">
      <input class="xlarge error" id="errorInput" name="errorInput" size="30" type="text">
      <span class="help-inline">Small snippet of help text</span>
  </div>
</div>

I created the following form where login is a required field

= twitter_bootstrap_form_for ['admin', @location] do |f|
  = f.text_field :name

The HTML that is generated on error is:

<div class=" clearfix error" id="location_name_input">
  <div class="field_with_errors">
    <label for="location_name">Name</label>
  </div>
  <div class="input">
    <div class="field_with_errors">
      <input id="location_name" name="location[name]" size="30" type="text" />
    </div>
    <span class="help-inline">can't be blank</span>
  </div>
</div>

There seem to be two extra divs (one each wrapping the label and the actual input tag). I think these are rails artifacts. The problem is, the "can't be blank" text gets rendered below the input rather than next to it because the div.field_with_errors is a block element. I can work around this with

.field_with_errors {
  display: inline;
}

but is there any way those extra divs can be eliminated entirely?

stouset commented 12 years ago

I have code intended to work around this. https://github.com/stouset/twitter_bootstrap_form_for/blob/master/lib/twitter_bootstrap_form_for/form_builder.rb#L31

It's been working for me. What version of Rails and this gem are you using?

derekprior commented 12 years ago

On Mon, Nov 14, 2011 at 10:20 PM, Stephen Touset reply@reply.github.com wrote:

I have code intended to work around this. https://github.com/stouset/twitter_bootstrap_form_for/blob/master/lib/twitter_bootstrap_form_for/form_builder.rb#L31

It's been working for me. What version of Rails and this gem are you using?


Reply to this email directly or view it on GitHub: https://github.com/stouset/twitter_bootstrap_form_for/issues/23#issuecomment-2740464

stouset commented 12 years ago

Strange. Again, it works for me. I'm about to hit the sack, but I'll look tomorrow morning if you haven't found anything by then.

stouset commented 12 years ago

Pulled your request to close this issue. However, for future compatibility with Twitter Bootstrap, you should wrap input groups with a fieldset by wrapping them in a form.inputs { ... } block.

derekprior commented 12 years ago

Not my pull request - that was another user. I'll test it out and comment in the pull request. I don't have a reason for not using a fieldset other than that it didn't trike me as necessary semantically or stylistically. I tend to use fieldsets to group multi-input sub-objects in a form, but not generally on the main form.

stouset commented 12 years ago

Whoops! So it was.