Move validation errors for check boxes and radio buttons into last div.form-check. (#448)

* Test from 419-check-radio-errors.
* Tests from 419-check-radio-errors-b and 419-check-radio-errors-c.
* Plain check_box working.
* radio_button error_message.
* Collections radios and checks working.
* Correct options to Rails helper.
* Upgrade doc first draft.
* Fix tests and code for file and date/time selects.
* Update CHANGELOG.
* Remove commented-out test

This test case was removed as it was something that's too difficult to
support with Bootstrap 4.

    test 'form_group renders the "error" class and message corrrectly when object is invalid' do
      @user.email = nil
      assert @user.invalid?

      output = @builder.form_group :email do
        %{<p class="form-control-static">Bar</p>}.html_safe
      end

      expected = <<-HTML.strip_heredoc
        <div class="form-group">
          <p class="form-control-static">Bar</p>
          <div class="invalid-feedback">can't be blank, is too short (minimum is 5 characters)</div>
        </div>
      HTML
      assert_equivalent_xml expected, output
    end

In its place, we've recommended a workaround in the UPGRADE-4.0 doc,
which is to output the error message manually. This workaround has a
corresponding test to verify its correctness.

* Error message must be sibling of input

Author: mattbrictson

CHANGELOG.md

@@ -18,9 +18,11 @@ In addition to these necessary markup changes, the bootstrap_form API itself has
 * `hide_label: true` and `skip_label: true` on individual check boxes and radio buttons apply Bootstrap 4 markup. This means the appearance of a page may change if you're upgrading from the Bootstrap 3 version of `bootstrap_form`, and you used `check_box` or `radio_button` with either of those options
 * `static_control` will no longer properly show error messages. This is the result of bootstrap changes.
 * `static_control` will also no longer accept a block, use the `value` option instead.
-* Your contribution here!
+* `form_group` with a block that produces arbitrary text needs to be modified to produce validation error messages (see the UPGRADE-4.0 document). [@lcreid](https://github.com/lcreid).
+* `form_group` with a block that contains more than one `check_box` or `radio_button` needs to be modified to produce validation error messages (see the UPGRADE-4.0 document). [@lcreid](https://github.com/lcreid).
 * [#456](https://github.com/bootstrap-ruby/bootstrap_form/pull/456): Fix label `for` attribute when passing non-english characters using `collection_check_boxes` - [@ug0](https://github.com/ug0).
 * [#449](https://github.com/bootstrap-ruby/bootstrap_form/pull/449): Bootstrap 4 no longer mixes in `.row` in `.form-group`. `bootstrap_form` adds `.row` to `div.form-group` when layout is horizontal.
+* Your contribution here!
 
 ### New features
 
@@ -32,6 +34,7 @@ In addition to these necessary markup changes, the bootstrap_form API itself has
 * Adds support for `label_as_placeholder` option, which will set the label text as an input fields placeholder (and hiding the label for sr_only).
 * [#449](https://github.com/bootstrap-ruby/bootstrap_form/pull/449): Passing `.form-row` overrides default `.form-group.row` in horizontal layouts.
 * Added an option to the `submit` (and `primary`, by transitivity) form tag helper, `render_as_button`, which when truthy makes the submit button render as a button instead of an input. This allows you to easily provide further styling to your form submission buttons, without requiring you to reinvent the wheel and use the `button` helper (and having to manually insert the typical Bootstrap classes). - [@jsaraiva](https://github.com/jsaraiva).
+* Add `:error_message` option to `check_box` and `radio_button`, so they can output validation error messages if needed. [@lcreid](https://github.com/lcreid).
 * Your contribution here!
 
 ### Bugfixes

UPGRADE-4.0.md

@@ -1,3 +1,63 @@
+# Upgrading to `bootstrap_form` 4.0
+We made every effort to make the upgrade from `bootstrap_form` v2.7 (Bootstrap 3) to `bootstrap_form` v4.0 (Bootstrap 4) as easy as possible. However, Bootstrap 4 is fundamentally different from Bootstrap 3, so some changes may be necessary in your code.
+
+## Bootstrap 4 Changes
+If you made use of Bootstrap classes or Javascript, you should read the [Bootstrap 4 migration guide](https://getbootstrap.com/docs/4.0/migration/).
+
+## Validation Error Messages
+With Bootstrap 4, in order for validation error messages to display, the message has to be a sibling of the `input` tag, and the `input` tag has to have the `.is-invalid` class. This was different from Bootstrap 3, and forced some changes to `bootstrap_form` that may affect programs that used `bootstrap_form` v2.7.
+
+### Arbitrary Text in `form_group` Blocks
+In `bootstrap_form` v2.7, it was possible to write something like this:
+```
+<%= bootstrap_form_for(@user) do |f| %>
+  <%= f.form_group(:email) do %>
+    <p class="form-control-static">Bar</p>
+  <%= end %>
+<%= end %>
+```
+and, if `@user.email` had validation errors, it would render:
+```
+<div class="form-group has-error">
+  <p class="form-control-static">Bar</p>
+  <span class="help-block">can't be blank, is too short (minimum is 5 characters)</span>
+</div>
+```
+which would show an error message in red.
+
+That doesn't work in Bootstrap 4. Outputting error messages had to be moved to accommodate other changes, so `form_group` no longer outputs error messages unless whatever is inside the block is a `bootstrap_form` helper.
+
+One way to make the above behave the same in `bootstrap_form` v4.0 is to write it like this:
+
+```
+<%= bootstrap_form_for(@user) do |f| %>
+  <%= f.form_group(:email) do %>
+    <p class="form-control-plaintext">Bar</p>
+    <%= content_tag(:div, @user.errors[:email].join(", "), class: "invalid-feedback", style: "display: block;") unless @user.errors[:email].empty? %>
+  <%= end %>
+<%= end %>
+```
+
+### Check Boxes and Radio Buttons
+Bootstrap 4 marks up check boxes and radio buttons differently. In particular, Bootstrap 4 wraps the `input` and `label` tags in a `div.form-check` tag. Because validation error messages have to be siblings of the `input` tag, there is now an `error_message` option to `check_box` and `radio_button` to cause them to put the validation error messages inside the `div.form-check`.
+
+This change is mostly invisible to existing programs:
+
+- Since the default for `error_message` is false, use of `check_box` and `radio_button` all by themselves behaves the same as in `bootstrap_form` v2.7
+- All the `collection*` helpers that output radio buttons and check boxes arrange to produce the validation error message on the last check box or radio button of the group, like `bootstrap_form` v2.7 did
+
+There is one situation where an existing program will have to change. When rendering one or more check boxes or radio buttons inside a `form_group` block, the last call to `check_box` or `radio_button` in the block will have to have `error_message: true` added to its parameters, like this:
+
+```
+<%= bootstrap_form_for(@user) do |f| %>
+  <%= f.form_group(:education) do %>
+    <%= f.radio_button(:misc, "primary school") %>
+    <%= f.radio_button(:misc, "high school") %>
+    <%= f.radio_button(:misc, "university", error_message: true) %>
+  <%= end %>
+<%= end %>
+```
+
 ## `form-group` and Horizontal Forms
 In Bootstrap 3, `.form-group` mixed in `.row`. In Bootstrap 4, it doesn't. So `bootstrap_form` automatically adds `.row` to the `div.form-group`s that it creates, if the form group is in a horizontal layout. When migrating forms from the Bootstrap 3 version of `bootstrap_form` to the Bootstrap 4 version, check all horizontal forms to be sure they're being rendered properly.
 
@@ -17,4 +77,3 @@ bootstrap_form_for(@user, layout: "horizontal") do |f|
     ...
   end
 end
-```

lib/bootstrap_form/form_builder.rb

@@ -38,7 +38,9 @@ def initialize(object_name, object, template, options)
 
       define_method(with_method_name) do |name, options = {}|
         form_group_builder(name, options) do
-          send(without_method_name, name, options)
+          prepend_and_append_input(name, options) do
+            send(without_method_name, name, options)
+          end
         end
       end
 
@@ -50,28 +52,32 @@ def initialize(object_name, object, template, options)
       without_method_name = "#{method_name}_without_bootstrap"
 
       define_method(with_method_name) do |name, options = {}, html_options = {}|
-        prevent_prepend_and_append!(options)
         form_group_builder(name, options, html_options) do
-          content_tag(:div, send(without_method_name, name, options, html_options), class: control_specific_class(method_name))
+          content_tag(:div, class: control_specific_class(method_name)) do
+            input_with_error(name) do
+              send(without_method_name, name, options, html_options)
+            end
+          end
         end
       end
 
       bootstrap_method_alias method_name
     end
 
     def file_field_with_bootstrap(name, options = {})
-      prevent_prepend_and_append!(options)
       options = options.reverse_merge(control_class: "custom-file-input")
       form_group_builder(name, options) do
         content_tag(:div, class: "custom-file") do
-          placeholder = options.delete(:placeholder) || "Choose file"
-          placeholder_opts = { class: "custom-file-label" }
-          placeholder_opts[:for] = options[:id] if options[:id].present?
-
-          input = file_field_without_bootstrap(name, options)
-          placeholder_label = label(name, placeholder, placeholder_opts)
-          concat(input)
-          concat(placeholder_label)
+          input_with_error(name) do
+            placeholder = options.delete(:placeholder) || "Choose file"
+            placeholder_opts = { class: "custom-file-label" }
+            placeholder_opts[:for] = options[:id] if options[:id].present?
+
+            input = file_field_without_bootstrap(name, options)
+            placeholder_label = label(name, placeholder, placeholder_opts)
+            concat(input)
+            concat(placeholder_label)
+          end
         end
       end
     end
@@ -80,49 +86,52 @@ def file_field_with_bootstrap(name, options = {})
 
     def select_with_bootstrap(method, choices = nil, options = {}, html_options = {}, &block)
       form_group_builder(method, options, html_options) do
-        select_without_bootstrap(method, choices, options, html_options, &block)
+        prepend_and_append_input(method, options) do
+          select_without_bootstrap(method, choices, options, html_options, &block)
+        end
       end
     end
 
     bootstrap_method_alias :select
 
     def collection_select_with_bootstrap(method, collection, value_method, text_method, options = {}, html_options = {})
-      prevent_prepend_and_append!(options)
       form_group_builder(method, options, html_options) do
-        collection_select_without_bootstrap(method, collection, value_method, text_method, options, html_options)
+        input_with_error(method) do
+          collection_select_without_bootstrap(method, collection, value_method, text_method, options, html_options)
+        end
       end
     end
 
     bootstrap_method_alias :collection_select
 
     def grouped_collection_select_with_bootstrap(method, collection, group_method, group_label_method, option_key_method, option_value_method, options = {}, html_options = {})
-      prevent_prepend_and_append!(options)
       form_group_builder(method, options, html_options) do
-        grouped_collection_select_without_bootstrap(method, collection, group_method, group_label_method, option_key_method, option_value_method, options, html_options)
+        input_with_error(method) do
+          grouped_collection_select_without_bootstrap(method, collection, group_method, group_label_method, option_key_method, option_value_method, options, html_options)
+        end
       end
     end
 
     bootstrap_method_alias :grouped_collection_select
 
     def time_zone_select_with_bootstrap(method, priority_zones = nil, options = {}, html_options = {})
-      prevent_prepend_and_append!(options)
       form_group_builder(method, options, html_options) do
-        time_zone_select_without_bootstrap(method, priority_zones, options, html_options)
+        input_with_error(method) do
+          time_zone_select_without_bootstrap(method, priority_zones, options, html_options)
+        end
       end
     end
 
     bootstrap_method_alias :time_zone_select
 
     def check_box_with_bootstrap(name, options = {}, checked_value = "1", unchecked_value = "0", &block)
-      prevent_prepend_and_append!(options)
       options = options.symbolize_keys!
-      check_box_options = options.except(:label, :label_class, :help, :inline, :custom, :hide_label, :skip_label)
+      check_box_options = options.except(:label, :label_class, :error_message, :help, :inline, :custom, :hide_label, :skip_label)
       check_box_classes = [check_box_options[:class]]
       check_box_classes << "position-static" if options[:skip_label] || options[:hide_label]
+      check_box_classes << "is-invalid" if has_error?(name)
       if options[:custom]
-        validation = nil
-        validation = "is-invalid" if has_error?(name)
-        check_box_options[:class] = (["custom-control-input", validation] + check_box_classes).compact.join(' ')
+        check_box_options[:class] = (["custom-control-input"] + check_box_classes).compact.join(' ')
       else
         check_box_options[:class] = (["form-check-input"] + check_box_classes).compact.join(' ')
       end
@@ -148,45 +157,48 @@ def check_box_with_bootstrap(name, options = {}, checked_value = "1", unchecked_
         div_class.append("custom-control-inline") if layout_inline?(options[:inline])
         label_class = label_classes.prepend("custom-control-label").compact.join(" ")
         content_tag(:div, class: div_class.compact.join(" ")) do
-          if options[:skip_label]
+          html = if options[:skip_label]
             checkbox_html
           else
             # TODO: Notice we don't seem to pass the ID into the custom control.
             checkbox_html.concat(label(label_name, label_description, class: label_class))
           end
+          html.concat(generate_error(name)) if options[:error_message]
+          html
         end
       else
         wrapper_class = "form-check"
         wrapper_class += " form-check-inline" if layout_inline?(options[:inline])
         label_class = label_classes.prepend("form-check-label").compact.join(" ")
         content_tag(:div, class: wrapper_class) do
-          if options[:skip_label]
+          html = if options[:skip_label]
             checkbox_html
           else
             checkbox_html
               .concat(label(label_name,
                             label_description,
                             { class: label_class }.merge(options[:id].present? ? { for: options[:id] } : {})))
           end
+          html.concat(generate_error(name)) if options[:error_message]
+          html
         end
       end
     end
 
     bootstrap_method_alias :check_box
 
     def radio_button_with_bootstrap(name, value, *args)
-      prevent_prepend_and_append!(options)
       options = args.extract_options!.symbolize_keys!
-      radio_options = options.except(:label, :label_class, :help, :inline, :custom, :hide_label, :skip_label)
+      radio_options = options.except(:label, :label_class, :error_message, :help, :inline, :custom, :hide_label, :skip_label)
       radio_classes = [options[:class]]
       radio_classes << "position-static" if options[:skip_label] || options[:hide_label]
+      radio_classes << "is-invalid" if has_error?(name)
       if options[:custom]
         radio_options[:class] = radio_classes.prepend("custom-control-input").compact.join(' ')
       else
         radio_options[:class] = radio_classes.prepend("form-check-input").compact.join(' ')
       end
-      args << radio_options
-      radio_html = radio_button_without_bootstrap(name, value, *args)
+      radio_html = radio_button_without_bootstrap(name, value, radio_options)
 
       disabled_class = " disabled" if options[:disabled]
       label_classes  = [options[:label_class]]
@@ -197,32 +209,35 @@ def radio_button_with_bootstrap(name, value, *args)
         div_class.append("custom-control-inline") if layout_inline?(options[:inline])
         label_class = label_classes.prepend("custom-control-label").compact.join(" ")
         content_tag(:div, class: div_class.compact.join(" ")) do
-          if options[:skip_label]
+          html = if options[:skip_label]
             radio_html
           else
             # TODO: Notice we don't seem to pass the ID into the custom control.
             radio_html.concat(label(name, options[:label], value: value, class: label_class))
           end
+          html.concat(generate_error(name)) if options[:error_message]
+          html
         end
       else
         wrapper_class = "form-check"
         wrapper_class += " form-check-inline" if layout_inline?(options[:inline])
         label_class = label_classes.prepend("form-check-label").compact.join(" ")
         content_tag(:div, class: "#{wrapper_class}#{disabled_class}") do
-          if options[:skip_label]
+          html = if options[:skip_label]
             radio_html
           else
             radio_html
               .concat(label(name, options[:label], { value: value, class: label_class }.merge(options[:id].present? ? { for: options[:id] } : {})))
           end
+          html.concat(generate_error(name)) if options[:error_message]
+          html
         end
       end
     end
 
     bootstrap_method_alias :radio_button
 
     def collection_check_boxes_with_bootstrap(*args)
-      prevent_prepend_and_append!(options)
       html = inputs_collection(*args) do |name, value, options|
         options[:multiple] = true
         check_box(name, options, value, nil)
@@ -233,7 +248,6 @@ def collection_check_boxes_with_bootstrap(*args)
     bootstrap_method_alias :collection_check_boxes
 
     def collection_radio_buttons_with_bootstrap(*args)
-      prevent_prepend_and_append!(options)
       inputs_collection(*args) do |name, value, options|
         radio_button(name, value, options)
       end
@@ -253,7 +267,7 @@ def form_group(*args, &block)
 
       content_tag(:div, options.except(:append, :id, :label, :help, :icon, :input_group_class, :label_col, :control_col, :layout, :prepend)) do
         label = generate_label(options[:id], name, options[:label], options[:label_col], options[:layout]) if options[:label]
-        control = prepend_and_append_input(name, options, &block).to_s
+        control = capture(&block)
 
         help = options[:help]
         help_text = generate_help(name, help).to_s
@@ -405,10 +419,6 @@ def form_group_builder(method, options, html_options = nil)
         class: wrapper_class
       }
 
-      form_group_options[:append] = options.delete(:append) if options[:append]
-      form_group_options[:prepend] = options.delete(:prepend) if options[:prepend]
-      form_group_options[:input_group_class] = options.delete(:input_group_class) if options[:input_group_class]
-
       if wrapper_options.is_a?(Hash)
         form_group_options.merge!(wrapper_options)
       end
@@ -515,7 +525,7 @@ def inputs_collection(name, collection, value, text, options = {}, &block)
       form_group_builder(name, options) do
         inputs = ""
 
-        collection.each do |obj|
+        collection.each_with_index do |obj, i|
           input_options = options.merge(label: text.respond_to?(:call) ? text.call(obj) : obj.send(text))
 
           input_value = value.respond_to?(:call) ? value.call(obj) : obj.send(value)
@@ -527,7 +537,7 @@ def inputs_collection(name, collection, value, text, options = {}, &block)
           end
 
           input_options.delete(:class)
-          inputs << block.call(name, input_value, input_options)
+          inputs << block.call(name, input_value, input_options.merge(error_message: i == collection.size - 1))
         end
 
         inputs.html_safe

lib/bootstrap_form/helpers/bootstrap.rb

@@ -89,12 +89,9 @@ def prepend_and_append_input(name, options, &block)
         input
       end
 
-      # Some helpers don't currently accept prepend and append. However, it's not
-      # clear if that's corrent. In the meantime, strip to options before calling
-      # methods that don't accept prepend and append.
-      def prevent_prepend_and_append!(options)
-        options.delete(:append)
-        options.delete(:prepend)
+      def input_with_error(name, &block)
+        input = capture(&block)
+        input << generate_error(name)
       end
 
       def input_group_content(content)