form-horizontal support for Bootstrap 3.x #94

Merged
merged 1 commit into from Jan 11, 2015

Conversation

Projects
None yet
9 participants
Collaborator

jtomaszewski commented Feb 2, 2014

It seems the form-group markup for horizontal forms has been changed in bootstrap 3.x. Thus, the old way (#34) doesn't work any more.

potenza/bootstrap_form gem solved this by defining default class for :left and :right column in #form_for method. I think we could place it similarly but in #inputs method, like:

= f.inputs(label_class: 'col-sm-2', wrapper_class: 'col-sm-10')

and add some wrapper around the input.control-group, to make it look like this:

<div class="form-group">
  <label class="col-sm-2">Foo</label>
  <div class="form-wrapper col-sm-10">
    <input type="text" name="foo" class="form-control" />
  </div>
</div>

Before this issue gets fixed, we could hack it ourselves with label_html and input_html options for now but.. there is no label_html options in current version of formtastic =( (see justinfrench/formtastic#865)

Some other ideas?

Collaborator

jtomaszewski commented Feb 2, 2014

With these changes, I managed to make my form look correctly with .form-horizontal class (screen here). I styled wrapper classes as grid, with bootstrap mixins (make-sm-column)

The wrappers I added (.form-label, .form-wrapper) should be divs. However I made them as spans, so they won't disrupt existing .form-inline forms.

Before it's ready to merge with master, we should:

  • fix speces (I didn't manage to run them - I keep getting some Rails::VERSION NameError while running them with bundle exec rspec - I'll maybe try later)
  • add some way to change wrapper classes, so the user could do something like this:
= semantic_form_for @foo, label_wrapper_class: 'col-sm-3', input_wrapper_class: 'col-sm-9' do |f|
  = ...

What do you think about this?

Collaborator

sodabrew commented Feb 13, 2014

Needs a rebase after I merged your work today.

Collaborator

sodabrew commented Feb 14, 2014

I'm not sure where specs are at these days.
I like your suggestion for the wrapper class options.

Owner

mjbellantoni commented Feb 15, 2014

Much to my embarrassment the specs are a mess. I thought they were passing cleanly, but before working on the initial Bootstrap 3.0 work it became clear they were very broken. It would be great to get them back in order.

Collaborator

sodabrew commented Mar 18, 2014

Ping @jtomaszewski Would you rebase and commit this one? Horizontal forms looking good with it!

Add span wrappers around label and input contents.
- `label.control-label` is now surrounded with `span.form-label`
- `input_content` (f.e. all data inputs & hint row) is now surrounded with `span.form-wrapper`

You can style them to f.e. make your form look more like `.form-horizontal`.
Collaborator

jtomaszewski commented Mar 18, 2014

OK I rebased it, but I wouldn't merge it yet - we should finish what I described in my comment above before we merge it IMHO. I'll try to work on this in this week.

Collaborator

sodabrew commented Mar 18, 2014

Thanks! I'll try to take a look this week, too.

mtobias commented Apr 2, 2014

+1

+1

fredngo commented Apr 25, 2014

+1

Collaborator

sodabrew commented Apr 25, 2014

Do these +1's mean "tested this PR and it works, let's merge it!" or just "I want this plz thx." ?

fredngo commented Apr 26, 2014

Heh. Can't speak for others but for me it is "I want this plz thx". ;-)

Collaborator

sodabrew commented May 3, 2014

At long last I have updated my app that uses FBS to version 3.0 and I'm going through the Bootstrap 3 conversion – it's a pain! I was using code like this before, which doesn't work anymore. So I'm testing against this PR to get the horizontal look back.

<%= semantic_form_for(@foo, :html => {:class => 'form-horizontal'}) do |f| %>

mbhnyc commented Jul 28, 2014

Is this OK to merge? Need this support myself as well if possible.

Zhomart commented Aug 17, 2014

Bootstrap 3.2 has no wrapper around label

<form class="form-horizontal" role="form">
 <div class="form-group">
    <label for="inputEmail3" class="col-sm-2 control-label">Email</label>
    <div class="col-sm-10">
      <input type="email" class="form-control" id="inputEmail3" placeholder="Email">
    </div>
  </div>
</div>
<form role="form">
  <div class="form-group">
    <label for="exampleInputEmail1">Email address</label>
    <input type="email" class="form-control" id="exampleInputEmail1" placeholder="Enter email">
  </div>
</div>
Collaborator

sodabrew commented Nov 3, 2014

@jtomaszewski I'm updating PRs today and wonder if this is good to merge yet. Do you want to work on it some more, or should I push ahead?

Collaborator

jtomaszewski commented Nov 6, 2014

Well, It can be merged, but IMHO it still needs some work (see #94 (comment) ). wrapper_classes should be extracted to a option passed to semantic_form_for.

P.S. Are the tests working okay now? I've just ran rake for current origin/master and 141 specs out of 1227 currently fail.

so what is the current status of this request? I need support for horizontal forms in my application, and just adding the form-horizontal class seems to not work...

Thanks!

sodabrew added a commit that referenced this pull request Jan 11, 2015

Merge pull request #94 from jtomaszewski/master
form-horizontal support for Bootstrap 3.x

@sodabrew sodabrew merged commit 8fcd655 into mjbellantoni:master Jan 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment