Support for Rails 5.1 form_with (#369)

* has-error is now has-danger

* inputs with errors should have form-control-feedback

* control-label is now form-control-label

* btn-default is now btn-secondary

* form-horizontal is no longer a required class

* input field should have form-control-danger on error

* checkbox is now form-check

* checkbox input should have form-check-input

* non inline checkbox labels should have form-check-label

* Support :prepend and :append for the `select` helper (#327)

* Support :prepend and :append for the `select` helper
Fixes #147

* Bootstrap 4 has renamed `control-label` to `form-control-label`

* `form_with` mostly working.

The main changes were around the fact that `form_with` doesn't
automatically add class and id attributes in many places that the
other form builders did.

Added a `for:` to the label options if an ID was specified for
the input element, so that the label would get the correct value
for the `for` attribute.

Sub-classed the form builder to override methods to make `form_with`
work without the default Rails-generated DOM IDs.

* Make it parse under Ruby 1.9.2

The `bootstrap_form_with` helper uses Ruby 2+ syntax. I put a test
for the Rails version, so the helper is only parse if running
Rails 5.1+.

* Try another way to fix Ruby 1.9.2 parse problem.

* Try Ruby 1.9.2 syntax yet again.

* Close, but going to try keying on Rails ID generation.

* Using the underlying option for id generation works.

* Use fields instead of fields_for.

* Finally remove FormBuilderFormWith.

* Remove some unnecessary diffs.

Remove confusing comment.

Remove some more unnecessary diffs.

* Fix select test case for :prepend, :append.

* Fix test case that hadn't been converted to form_with.

* Documentation tests.
Rails 5.2 not tested yet.

* Fix out-of-date HTML in test case.

* Uncomment and fix Rails 5.1 collection_radio_button tests.

* Fix README to remove false statements about 5.2.

* heredoc

* bootstrap_form_with_test working with 5.1 and 5.2.

* Handle Rails 5.1 and 5.2+.

* Use heredoc.strip.

* Comments and more heredoc formatting.

* Partially updated test cases.

* Don't add class="" to label if no classes specified.

* Intermediate check-in.

* Intermediate.

* Intermediate.

* Intermediate.

* Most of custom IDs working.

* Uncomment test cases.

* Resolve conflicts.

* Saving work with tests red and debugging.

* Commit before saving to future branch.

* Delete redundant test files.

* Remove puts and TODOs.

* Tests passing up to Rails 5.1.

* Some tests for form_with. All pass.

* Documentation tested.

* Update Rails 5.1 examples in documentation.

* Pin minitest to exactly 5.10.1 for Rails 5.0.

* Fence off one form_with test.

* Add tests for fields method. Pass when they shouldn't.

* Go back to using minitest 5.10.3 to do bisect.

* Test only form_for in one case.
The time zone text strings have an "&" in them. When it's converted to
XML and then back to a string, the "&" gets lost.

* Test and show that bootstrap_form fields helper not needed

* Remove unwanted tests.

* Feedback from review.
Removed unneeded documentation.
Moved `bootstrap_form_with` to after `bootstrap_form_for`.

* Minimize test cases.

* Fix mistaken commit of version.

* Remove code per review.

* Remove duplicate CHANGELOG entry

CHANGELOG.md

@@ -17,6 +17,7 @@ In addition to these necessary markup changes, the bootstrap_form API itself has
 
 ### New features
 
+* Support for Rails 5.1 `form_with` - [@lcreid](https://github.com/lcreid).
 * Support Bootstrap v4's [Custom Checkboxes and Radios](https://getbootstrap.com/docs/4.0/components/forms/#checkboxes-and-radios-1) with a new `custom: true` option
 * Allow HTML in help translations by using the `_html` suffix on the key - [@unikitty37](https://github.com/unikitty37)
 * Your contribution here!

README.md

@@ -13,7 +13,7 @@ Bootstrap v4-style forms into your Rails application.
 ## Requirements
 
 * Ruby 2.2.2+
-* Rails 5.0+
+* Rails 5.0+ (Rails 5.1+ for `bootstrap_form_with`)
 * Bootstrap 4.0.0+
 
 ## Installation
@@ -84,6 +84,53 @@ If your form is not backed by a model, use the `bootstrap_form_tag`. Usage of th
 <% end %>
 ```
 
+### `bootstrap_form_with` (Rails 5.1+)
+
+Note that `form_with` in Rails 5.1 does not add IDs to form elements and labels by default, which are both important to Bootstrap markup. This behavior is corrected in Rails 5.2.
+
+To get started, just use the `bootstrap_form_with` helper in place of `form_with`. Here's an example:
+
+```erb
+<%= bootstrap_form_with(model: @user, local: true) do |f| %>
+  <%= f.email_field :email %>
+  <%= f.password_field :password %>
+  <%= f.check_box :remember_me %>
+  <%= f.submit "Log In" %>
+<% end %>
+```
+
+This generates:
+
+```html
+<form role="form" action="/users" accept-charset="UTF-8" method="post">
+  <input name="utf8" type="hidden" value="&#x2713;" />
+  <div class="form-group">
+    <label class="required" for="user_email">Email</label>
+    <input class="form-control" type="email" value="steve@example.com" name="user[email]" />
+  </div>
+  <div class="form-group">
+    <label for="user_password">Password</label>
+    <input class="form-control" type="password" name="user[password]" />
+    <small class="form-text text-muted">A good password should be at least six characters long</small>
+  </div>
+  <div class="form-check">
+    <label class="form-check-label" for="user_remember_me">
+    <input name="user[remember_me]" type="hidden" value="0" />
+    <input class="form-check-input" type="checkbox" value="1" name="user[remember_me]" /> Remember me</label>
+  </div>
+  <input type="submit" name="commit" value="Log In" class="btn btn-secondary" data-disable-with="Log In" />
+</form>
+```
+
+`bootstrap_form_with` supports both the `model:` and `url:` use cases
+in `form_with`.
+
+`form_with` has some important differences compared to `form_for` and `form_tag`, and these differences apply to `bootstrap_form_with`. A good summary of the differences can be found at: https://m.patrikonrails.com/rails-5-1s-form-with-vs-old-form-helpers-3a5f72a8c78a, or in the [Rails documentation](api.rubyonrails.org).
+
+### Future Compatibility
+
+The Rails team has [suggested](https://github.com/rails/rails/issues/25197) that `form_for` and `form_tag` may be deprecated and then removed in future versions of Rails. `bootstrap_form` will continue to support `bootstrap_form_for` and `bootstrap_form_tag` as long as Rails supports `form_for` and `form_tag`.
+
 ## Form Helpers
 
 This gem wraps the following Rails form helpers:

lib/bootstrap_form/form_builder.rb

@@ -235,7 +235,10 @@ def form_group(*args, &block)
     end
 
     def fields_for_with_bootstrap(record_name, record_object = nil, fields_options = {}, &block)
-      fields_options, record_object = record_object, nil if record_object.is_a?(Hash) && record_object.extractable_options?
+      if record_object.is_a?(Hash) && record_object.extractable_options?
+        fields_options = record_object
+        record_object = nil
+      end
       fields_options[:layout] ||= options[:layout]
       fields_options[:label_col] = fields_options[:label_col].present? ? "#{fields_options[:label_col]}" : options[:label_col]
       fields_options[:control_col] ||= options[:control_col]
@@ -246,6 +249,10 @@ def fields_for_with_bootstrap(record_name, record_object = nil, fields_options =
 
     bootstrap_method_alias :fields_for
 
+    # the Rails `fields` method passes its options
+    # to the builder, so there is no need to write a `bootstrap_form` helper
+    # for the `fields` method.
+
     private
 
     def horizontal?
@@ -357,11 +364,11 @@ def form_group_builder(method, options, html_options = nil)
           label_text ||= options.delete(:label)
         end
 
-        form_group_options.merge!(label: {
+        form_group_options[:label] = {
           text: label_text,
           class: label_class,
           skip_required: options.delete(:skip_required)
-        })
+        }
       end
 
       form_group(method, form_group_options) do
@@ -370,12 +377,18 @@ def form_group_builder(method, options, html_options = nil)
     end
 
     def convert_form_tag_options(method, options = {})
-      options[:name] ||= method
-      options[:id] ||= method
+      unless @options[:skip_default_ids]
+        options[:name] ||= method
+        options[:id] ||= method
+      end
       options
     end
 
     def generate_label(id, name, options, custom_label_col, group_layout)
+      # id is the caller's options[:id] at the only place this method is called.
+      # The options argument is a small subset of the options that might have
+      # been passed to generate_label's caller, and definitely doesn't include
+      # :id.
       options[:for] = id if acts_like_form_tag
       classes = [options[:class]]
 
@@ -398,7 +411,6 @@ def generate_label(id, name, options, custom_label_col, group_layout)
       else
         label(name, options[:text], options.except(:text))
       end
-
     end
 
     def generate_help(name, help_text)
@@ -467,6 +479,5 @@ def get_help_text_by_i18n_key(name)
         help_text
       end
     end
-
   end
 end

lib/bootstrap_form/helper.rb

@@ -4,12 +4,7 @@ module Helper
     def bootstrap_form_for(object, options = {}, &block)
       options.reverse_merge!({builder: BootstrapForm::FormBuilder})
 
-      options[:html] ||= {}
-      options[:html][:role] ||= 'form'
-
-      if options[:layout] == :inline
-        options[:html][:class] = [options[:html][:class], "form-inline"].compact.join(" ")
-      end
+      options = process_options(options)
 
       temporarily_disable_field_error_proc do
         form_for(object, options, &block)
@@ -22,6 +17,31 @@ def bootstrap_form_tag(options = {}, &block)
       bootstrap_form_for("", options, &block)
     end
 
+    def bootstrap_form_with(options = {}, &block)
+      options.reverse_merge!(builder: BootstrapForm::FormBuilder)
+
+      options = process_options(options)
+
+      temporarily_disable_field_error_proc do
+        form_with(options, &block)
+      end
+    end
+
+    private
+
+    def process_options(options)
+      options[:html] ||= {}
+      options[:html][:role] ||= 'form'
+
+      if options[:layout] == :inline
+        options[:html][:class] = [options[:html][:class], 'form-inline'].compact.join(' ')
+      end
+
+      options
+    end
+
+    public
+
     def temporarily_disable_field_error_proc
       original_proc = ActionView::Base.field_error_proc
       ActionView::Base.field_error_proc = proc { |input, instance| input }

test/bootstrap_fields_test.rb

@@ -142,6 +142,30 @@ class BootstrapFieldsTest < ActionView::TestCase
     assert_equivalent_xml expected, @builder.text_area(:comments)
   end
 
+  if ::Rails::VERSION::STRING > '5.1' && ::Rails::VERSION::STRING < '5.2'
+    test "text areas are wrapped correctly form_with Rails 5.1" do
+      expected = <<-HTML.strip_heredoc
+      <div class="form-group">
+        <label for="user_comments">Comments</label>
+        <textarea class="form-control" name="user[comments]">\nmy comment</textarea>
+      </div>
+      HTML
+      assert_equivalent_xml expected, form_with_builder.text_area(:comments)
+    end
+  end
+
+  if ::Rails::VERSION::STRING > '5.2'
+    test "text areas are wrapped correctly form_with Rails 5.2+" do
+      expected = <<-HTML.strip_heredoc
+      <div class="form-group">
+        <label for="user_comments">Comments</label>
+        <textarea class="form-control" id="user_comments" name="user[comments]">\nmy comment</textarea>
+      </div>
+      HTML
+      assert_equivalent_xml expected, form_with_builder.text_area(:comments)
+    end
+  end
+
   test "text fields are wrapped correctly" do
     expected = <<-HTML.strip_heredoc
       <div class="form-group">
@@ -250,6 +274,7 @@ class BootstrapFieldsTest < ActionView::TestCase
   test "fields_for correctly passes inline style from parent builder" do
     @user.address = Address.new(street: '123 Main Street')
 
+    # NOTE: This test works with even if you use `fields_for_without_bootstrap`
     output = bootstrap_form_for(@user, layout: :inline) do |f|
       f.fields_for :address do |af|
         af.text_field(:street)

test/bootstrap_form_test.rb

@@ -14,6 +14,19 @@ class BootstrapFormTest < ActionView::TestCase
     assert_equivalent_xml expected, bootstrap_form_for(@user) { |f| nil }
   end
 
+  if  ::Rails::VERSION::STRING >= '5.1'
+    # No need to test 5.2 separately for this case, since 5.2 does *not*
+    # generate a default ID for the form element.
+    test "default-style forms bootstrap_form_with Rails 5.1+" do
+      expected = <<-HTML.strip_heredoc
+        <form accept-charset="UTF-8" action="/users" data-remote="true" method="post" role="form">
+          <input name="utf8" type="hidden" value="&#x2713;" />
+        </form>
+      HTML
+      assert_equivalent_xml expected, bootstrap_form_with(model: @user) { |f| nil }
+    end
+  end
+
   test "inline-style forms" do
     expected = <<-HTML.strip_heredoc
       <form accept-charset="UTF-8" action="/users" class="form-inline" id="new_user" method="post" role="form">

test/test_helper.rb

@@ -36,6 +36,13 @@ def setup_test_fixture
     })
   end
 
+  # Originally only used in one test file but placed here in case it's needed in others in the future.
+  def form_with_builder
+    builder = nil
+    bootstrap_form_with(model: @user) { |f| builder = f }
+    builder
+  end
+
   def sort_attributes doc
     doc.dup.traverse do |node|
       if node.is_a?(Nokogiri::XML::Element)
@@ -86,5 +93,4 @@ def assert_equivalent_xml(expected, actual)
       ).to_s(:color)
     }
   end
-
 end