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

Add small fixes for bootstrap 3. Add tests. #85

Open schneikai opened 10 years ago

schneikai commented 10 years ago

Hi,

I use this gem in a current Rails 4 project so I added some fixes to make it work better with Bootstrap 3. I also added tests for everything.

Details: I removed the example code for inline. Correct me if I'm wrong but this wasn't TWBS 3 markup. Inline form elements in horizontal forms are also not mentioned in the Bootstrap docs so maybe we should remove this? If it should stay, I think the right markup would be to add a row container and add a column class to every form-group inside that row.

Also the date_select won't render 3 fields on one line without extra markup in TWBS 3 so the old screenshot is wrong. We would need to add extra css to make the 3 select inputs float or wrap them with a row and add column classes like for inline inputs.

Updated the height sizing classes to use input-sm instead of small.

Added the select element styling fix from https://github.com/stouset/twitter_bootstrap_form_for/pull/78

Updated the screenshot.

stouset commented 10 years ago

The example screenshot no longer has inline form fields, as it does in the original example image.

stouset commented 10 years ago

Outside of the comments, it looks good. Thanks.

schneikai commented 10 years ago

About the inline form fields: I removed the example code for inline. Correct me if I'm wrong but this wasn't TWBS 3 markup. Inline form elements in horizontal forms are also not mentioned in the Bootstrap docs so maybe we should remove this? If it should stay, I think the right markup would be to add a row container and add a column class to every form-group inside that row.

I could add it like this if you want to.

stouset commented 10 years ago

If we can keep some way to have inline form fields, it would be great. Use your best judgment, though — if TB doesn't support anything like it now, and it looks like any approach would be likely to break in future updates, we should probably abandon it.