From a3d9d7365db7c697974f29477527b65aab267ad0 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 10:13:35 -0800 Subject: [PATCH 01/17] Test from 419-check-radio-errors. --- test/bootstrap_checkbox_test.rb | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/bootstrap_checkbox_test.rb b/test/bootstrap_checkbox_test.rb index f7793061a..6e99b06dc 100644 --- a/test/bootstrap_checkbox_test.rb +++ b/test/bootstrap_checkbox_test.rb @@ -518,4 +518,34 @@ class BootstrapCheckboxTest < ActionView::TestCase HTML assert_equivalent_xml expected, @builder.check_box(:terms, {label: 'I agree to the terms', custom: true, hide_label: true}) end + + test 'collection_check_boxes renders error after last check box' do + collection = [Address.new(id: 1, street: 'Foo'), Address.new(id: 2, street: 'Bar')] + @user.errors.add(:misc, "a box must be checked") + + expected = <<-HTML.strip_heredoc +
+ + +
+ +
+ + +
+
+ + +
a box must be checked
+
+
+
+ HTML + + actual = bootstrap_form_for(@user) do |f| + f.collection_check_boxes(:misc, collection, :id, :street) + end + + assert_equivalent_xml expected, actual + end end From 32996dba7d67098e9d7ba91ff3acd6686cb90c90 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 10:25:35 -0800 Subject: [PATCH 02/17] Tests from 419-check-radio-errors-b and 419-check-radio-errors-c. --- test/bootstrap_checkbox_test.rb | 49 ++++++++++++++++++++++++- test/bootstrap_radio_button_test.rb | 55 ++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/test/bootstrap_checkbox_test.rb b/test/bootstrap_checkbox_test.rb index 6e99b06dc..a55670f83 100644 --- a/test/bootstrap_checkbox_test.rb +++ b/test/bootstrap_checkbox_test.rb @@ -109,6 +109,25 @@ class BootstrapCheckboxTest < ActionView::TestCase assert_equivalent_xml expected, @builder.check_box(:terms, label: 'I agree to the terms', inline: true) end + test "inline checkboxes from form layout" do + expected = <<-HTML.strip_heredoc +
+ +
+ + + +
+
+ HTML + actual = bootstrap_form_for(@user, layout: :inline) do |f| + f.check_box(:terms, label: 'I agree to the terms') + end + assert_equivalent_xml expected, actual + end + test "disabled inline check_box" do expected = <<-HTML.strip_heredoc
@@ -144,8 +163,8 @@ class BootstrapCheckboxTest < ActionView::TestCase
+ With a help!
- With a help!
HTML @@ -548,4 +567,32 @@ class BootstrapCheckboxTest < ActionView::TestCase assert_equivalent_xml expected, actual end + + test 'collection_check_boxes renders multiple check boxes with error correctly' do + @user.errors.add(:misc, "error for test") + collection = [Address.new(id: 1, street: 'Foo'), Address.new(id: 2, street: 'Bar')] + expected = <<-HTML.strip_heredoc +
+ + +
+ +
+ + +
+
+ + +
error for test
+
+
+
+ HTML + + actual = bootstrap_form_for(@user) do |f| + f.collection_check_boxes(:misc, collection, :id, :street, checked: collection) + end + assert_equivalent_xml expected, actual + end end diff --git a/test/bootstrap_radio_button_test.rb b/test/bootstrap_radio_button_test.rb index 8cb8a627c..a6bf7c7bc 100644 --- a/test/bootstrap_radio_button_test.rb +++ b/test/bootstrap_radio_button_test.rb @@ -65,6 +65,24 @@ class BootstrapRadioButtonTest < ActionView::TestCase assert_equivalent_xml expected, @builder.radio_button(:misc, '1', label: 'This is a radio button', inline: true) end + test "radio_button inline label is set correctly from form level" do + expected = <<-HTML.strip_heredoc +
+ +
+ + +
+
+ HTML + actual = bootstrap_form_for(@user, layout: :inline) do |f| + f.radio_button(:misc, '1', label: 'This is a radio button') + end + assert_equivalent_xml expected, actual + end + test "radio_button disabled inline label is set correctly" do expected = <<-HTML.strip_heredoc
@@ -99,8 +117,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase + With a help!
- With a help! HTML @@ -126,6 +144,33 @@ class BootstrapRadioButtonTest < ActionView::TestCase assert_equivalent_xml expected, @builder.collection_radio_buttons(:misc, collection, :id, :street) end + test 'collection_radio_buttons renders multiple radios with error correctly' do + @user.errors.add(:misc, "error for test") + collection = [Address.new(id: 1, street: 'Foo'), Address.new(id: 2, street: 'Bar')] + expected = <<-HTML.strip_heredoc +
+ +
+ +
+ + +
+
+ + +
error for test
+
+
+
+ HTML + + actual = bootstrap_form_for(@user) do |f| + f.collection_radio_buttons(:misc, collection, :id, :street) + end + assert_equivalent_xml expected, actual + end + test 'collection_radio_buttons renders inline radios correctly' do collection = [Address.new(id: 1, street: 'Foo'), Address.new(id: 2, street: 'Bar')] expected = <<-HTML.strip_heredoc @@ -172,8 +217,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
+ With a help!
- With a help! HTML @@ -188,8 +233,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
+ With a help!
- With a help! HTML @@ -242,8 +287,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
+ With a help!
- With a help! HTML @@ -258,8 +303,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
+ With a help!
- With a help! HTML From 55f399eddd243d46b96b1ab903b85029ff70148e Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 11:20:28 -0800 Subject: [PATCH 03/17] Additional tests. --- test/bootstrap_checkbox_test.rb | 49 ++++++++++++++++++++++++--- test/bootstrap_form_group_test.rb | 2 +- test/bootstrap_radio_button_test.rb | 52 +++++++++++++++++++++++++---- 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/test/bootstrap_checkbox_test.rb b/test/bootstrap_checkbox_test.rb index a55670f83..aee9d155e 100644 --- a/test/bootstrap_checkbox_test.rb +++ b/test/bootstrap_checkbox_test.rb @@ -163,8 +163,8 @@ class BootstrapCheckboxTest < ActionView::TestCase
- With a help!
+ With a help! HTML @@ -549,11 +549,11 @@ class BootstrapCheckboxTest < ActionView::TestCase
- +
- +
a box must be checked
@@ -578,11 +578,11 @@ class BootstrapCheckboxTest < ActionView::TestCase
- +
- +
error for test
@@ -595,4 +595,43 @@ class BootstrapCheckboxTest < ActionView::TestCase end assert_equivalent_xml expected, actual end + + test 'check_box renders error when asked' do + @user.errors.add(:terms, "You must accept the terms.") + expected = <<-HTML.strip_heredoc +
+ +
+ + + +
error for test
+
+
+ HTML + actual = bootstrap_form_for(@user) do |f| + f.check_box(:terms, label: 'I agree to the terms', error_message: true) + end + assert_equivalent_xml expected, actual + end + + test "check_box with error is wrapped correctly with custom option set" do + expected = <<-HTML.strip_heredoc +
+ +
+ + + +
error for test
+
+
+ HTML + actual = bootstrap_form_for(@user) do |f| + f.check_box(:terms, {label: 'I agree to the terms', custom: true, error_message: true}) + end + assert_equivalent_xml expected, actual + end end diff --git a/test/bootstrap_form_group_test.rb b/test/bootstrap_form_group_test.rb index a484ad751..93590193f 100644 --- a/test/bootstrap_form_group_test.rb +++ b/test/bootstrap_form_group_test.rb @@ -199,8 +199,8 @@ class BootstrapFormGroupTest < ActionView::TestCase
- This is required
+ This is required
HTML assert_equivalent_xml expected, @horizontal_builder.text_field(:email, help: "This is required") diff --git a/test/bootstrap_radio_button_test.rb b/test/bootstrap_radio_button_test.rb index a6bf7c7bc..038cde88e 100644 --- a/test/bootstrap_radio_button_test.rb +++ b/test/bootstrap_radio_button_test.rb @@ -17,6 +17,26 @@ class BootstrapRadioButtonTest < ActionView::TestCase assert_equivalent_xml expected, @builder.radio_button(:misc, '1', label: 'This is a radio button') end + test "radio_button with error is wrapped correctly" do + @user.errors.add(:misc, "error for test") + expected = <<-HTML.strip_heredoc +
+ +
+ + +
error for test
+
+
+ HTML + actual = bootstrap_form_for(@user) do |f| + f.radio_button(:misc, '1', label: 'This is a radio button', error_message: true) + end + assert_equivalent_xml expected, actual + end + test "radio_button disabled label is set correctly" do expected = <<-HTML.strip_heredoc
@@ -117,8 +137,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase - With a help!
+ With a help!
HTML @@ -153,11 +173,11 @@ class BootstrapRadioButtonTest < ActionView::TestCase
- +
- +
error for test
@@ -217,8 +237,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
- With a help!
+ With a help!
HTML @@ -233,8 +253,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
- With a help!
+ With a help! HTML @@ -287,8 +307,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
- With a help!
+ With a help! HTML @@ -303,8 +323,8 @@ class BootstrapRadioButtonTest < ActionView::TestCase
- With a help!
+ With a help! HTML @@ -359,6 +379,24 @@ class BootstrapRadioButtonTest < ActionView::TestCase assert_equivalent_xml expected, @builder.radio_button(:misc, '1', {label: 'This is a radio button', custom: true}) end + test "radio_button with error is wrapped correctly with custom option set" do + @user.errors.add(:misc, "error for test") + expected = <<-HTML.strip_heredoc +
+ +
+ + +
error for test
+
+
+ HTML + actual = bootstrap_form_for(@user) do |f| + f.radio_button(:misc, '1', {label: 'This is a radio button', custom: true, error_message: true}) + end + assert_equivalent_xml expected, actual + end + test "radio_button is wrapped correctly with custom and inline options set" do expected = <<-HTML.strip_heredoc
From aa9caa1659df38b174c697a2e0cac1395e1d543a Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:10:51 -0800 Subject: [PATCH 04/17] Comment out test case. --- test/bootstrap_form_group_test.rb | 33 ++++++++++++++++--------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/test/bootstrap_form_group_test.rb b/test/bootstrap_form_group_test.rb index 93590193f..2b6aea61b 100644 --- a/test/bootstrap_form_group_test.rb +++ b/test/bootstrap_form_group_test.rb @@ -374,22 +374,23 @@ class BootstrapFormGroupTest < ActionView::TestCase assert_equivalent_xml expected, output end - test 'form_group renders the "error" class and message correctly when object is invalid' do - @user.email = nil - assert @user.invalid? - - output = @builder.form_group :email do - %{}.html_safe - end - - expected = <<-HTML.strip_heredoc -
- -
can't be blank, is too short (minimum is 5 characters)
-
- HTML - assert_equivalent_xml expected, output - end + # NOTE: Removing this test case as 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 + # %{

Bar

}.html_safe + # end + # + # expected = <<-HTML.strip_heredoc + #
+ #

Bar

+ #
can't be blank, is too short (minimum is 5 characters)
+ #
+ # HTML + # assert_equivalent_xml expected, output + # end test "adds class to wrapped form_group by a field" do expected = <<-HTML.strip_heredoc From 7b0debeaae6cb0d79f1ade637876b7b9e6d8c298 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:18:42 -0800 Subject: [PATCH 05/17] Most (all?) regressions fixed. --- lib/bootstrap_form/form_builder.rb | 56 ++++++++++++++++--------- lib/bootstrap_form/helpers/bootstrap.rb | 6 +++ 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 0164e4843..f05572cf5 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/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 @@ -52,7 +54,9 @@ def initialize(object_name, object, template, options) 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)) + input_with_error(name) do + content_tag(:div, send(without_method_name, name, options, html_options), class: control_specific_class(method_name)) + end end end @@ -63,15 +67,17 @@ 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 + 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) + end end end end @@ -80,7 +86,9 @@ 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 @@ -89,7 +97,9 @@ def select_with_bootstrap(method, choices = nil, options = {}, html_options = {} 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 @@ -98,7 +108,9 @@ def collection_select_with_bootstrap(method, collection, value_method, text_meth 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 @@ -107,7 +119,9 @@ def grouped_collection_select_with_bootstrap(method, collection, group_method, g 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 @@ -253,7 +267,9 @@ 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 = prepend_and_append_input(name, options, &block).to_s + # puts "Just before capturing block #{name} #{options}" + control = capture(&block) help = options[:help] help_text = generate_help(name, help).to_s @@ -405,9 +421,9 @@ 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] + # 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) @@ -437,7 +453,9 @@ def form_group_builder(method, options, html_options = nil) end end + # puts "Just before form_group #{method} #{options}" form_group(method, form_group_options) do + # puts "block in form_group #{method} #{options}" yield end end diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index a4b28a5f5..cfc3e7b16 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -77,6 +77,7 @@ def custom_control(*args, &block) end def prepend_and_append_input(name, options, &block) + # puts "WELL HERE WE ARE #{name} #{options}" options = options.extract!(:prepend, :append, :input_group_class) input_group_class = ["input-group", options[:input_group_class]].compact.join(' ') @@ -89,6 +90,11 @@ def prepend_and_append_input(name, options, &block) input end + def input_with_error(name, &block) + input = capture(&block) + input << generate_error(name) + 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. From 203944f9f2e7072e64400cd30876b41770180e52 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:34:24 -0800 Subject: [PATCH 06/17] Fix test. --- test/bootstrap_checkbox_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/bootstrap_checkbox_test.rb b/test/bootstrap_checkbox_test.rb index aee9d155e..cc56d83ab 100644 --- a/test/bootstrap_checkbox_test.rb +++ b/test/bootstrap_checkbox_test.rb @@ -607,7 +607,7 @@ class BootstrapCheckboxTest < ActionView::TestCase -
error for test
+
You must accept the terms.
HTML @@ -618,6 +618,7 @@ class BootstrapCheckboxTest < ActionView::TestCase end test "check_box with error is wrapped correctly with custom option set" do + @user.errors.add(:terms, "You must accept the terms.") expected = <<-HTML.strip_heredoc
@@ -625,7 +626,7 @@ class BootstrapCheckboxTest < ActionView::TestCase -
error for test
+
You must accept the terms.
HTML From 6a2f220a95e0a968ad9c31323e27fbef321dd5c1 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:34:36 -0800 Subject: [PATCH 07/17] Plain check_box working. --- lib/bootstrap_form/form_builder.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index f05572cf5..c43b05830 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -128,15 +128,13 @@ def time_zone_select_with_bootstrap(method, priority_zones = nil, options = {}, 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 @@ -162,19 +160,21 @@ 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 @@ -182,6 +182,8 @@ def check_box_with_bootstrap(name, options = {}, checked_value = "1", unchecked_ 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 From aa345c080ce09d7527f1cc8a12a74710ff18e475 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:39:23 -0800 Subject: [PATCH 08/17] radio_button error_message. --- lib/bootstrap_form/form_builder.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index c43b05830..f53baba8a 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -191,11 +191,11 @@ def check_box_with_bootstrap(name, options = {}, checked_value = "1", unchecked_ 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 @@ -213,24 +213,28 @@ 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 From 2978360fd68abfeae3332d5092d75989dbf46243 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:43:22 -0800 Subject: [PATCH 09/17] Collections radios and checks working. --- lib/bootstrap_form/form_builder.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index f53baba8a..0f9a6ca9e 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -539,7 +539,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) @@ -551,7 +551,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 From 1d0663169521eba943bacb45201e8447d222ae24 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 12:46:40 -0800 Subject: [PATCH 10/17] Remove unneeded method. --- lib/bootstrap_form/form_builder.rb | 7 ------- lib/bootstrap_form/helpers/bootstrap.rb | 8 -------- 2 files changed, 15 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 0f9a6ca9e..1a7b278c3 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -52,7 +52,6 @@ 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 input_with_error(name) do content_tag(:div, send(without_method_name, name, options, html_options), class: control_specific_class(method_name)) @@ -64,7 +63,6 @@ def initialize(object_name, object, template, options) 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 input_with_error(name) do @@ -95,7 +93,6 @@ def select_with_bootstrap(method, choices = nil, options = {}, html_options = {} 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 input_with_error(method) do collection_select_without_bootstrap(method, collection, value_method, text_method, options, html_options) @@ -106,7 +103,6 @@ def collection_select_with_bootstrap(method, collection, value_method, text_meth 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 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) @@ -117,7 +113,6 @@ def grouped_collection_select_with_bootstrap(method, collection, group_method, g 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 input_with_error(method) do time_zone_select_without_bootstrap(method, priority_zones, options, html_options) @@ -242,7 +237,6 @@ def radio_button_with_bootstrap(name, value, *args) 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) @@ -253,7 +247,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 diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index cfc3e7b16..ba67c216a 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -95,14 +95,6 @@ def input_with_error(name, &block) input << generate_error(name) 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) - end - def input_group_content(content) return content if content.match(/btn/) content_tag(:span, content, class: 'input-group-text') From e17e6ef89eb9879adf225e0a539080f3b3c1196d Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 17:10:09 -0800 Subject: [PATCH 11/17] Correct options to Rails helper. --- lib/bootstrap_form/form_builder.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 1a7b278c3..d6ca0d67f 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -196,8 +196,7 @@ def radio_button_with_bootstrap(name, value, *args) 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]] From 4cfccf447d6435486d5627cb530afd4dbfff4240 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 17:13:11 -0800 Subject: [PATCH 12/17] Upgrade doc first draft. --- UPGRADE-4.0.md | 61 ++++++++++++++++++++++++++++++- test/bootstrap_form_group_test.rb | 54 +++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 8e195bbe3..7b0348581 100644 --- a/UPGRADE-4.0.md +++ b/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 %> +

Bar

+ <%= end %> +<%= end %> +``` +and, if `@user.email` had validation errors, it would render: +``` +
+

Bar

+ can't be blank, is too short (minimum is 5 characters) +
+``` +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 %> +

Bar

+ <%= 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 -``` diff --git a/test/bootstrap_form_group_test.rb b/test/bootstrap_form_group_test.rb index 2b6aea61b..37bd7b95a 100644 --- a/test/bootstrap_form_group_test.rb +++ b/test/bootstrap_form_group_test.rb @@ -392,6 +392,60 @@ class BootstrapFormGroupTest < ActionView::TestCase # assert_equivalent_xml expected, output # end + # It could be argued that this test replaces the commented-out test above. + test 'upgrade doc for 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 + html = %{

Bar

}.html_safe + html.concat(content_tag(:div, @user.errors[:email].join(", "), class: "invalid-feedback", style: "display: block;")) unless @user.errors[:email].empty? + html + end + + expected = <<-HTML.strip_heredoc +
+

Bar

+
can't be blank, is too short (minimum is 5 characters)
+
+ HTML + assert_equivalent_xml expected, output + end + + test 'upgrade doc for form_group renders check box corrrectly when object is invalid' do + @user.errors.add(:misc, "Must select one.") + + output = bootstrap_form_for(@user) do |f| + f.form_group :email do + f.radio_button(:misc, "primary school") + .concat(f.radio_button(:misc, "high school")) + .concat(f.radio_button(:misc, "university", error_message: true)) + end + end + + expected = <<-HTML.strip_heredoc +
+ +
+
+ + +
+
+ + +
+
+ + +
Must select one.
+
+
+
+ HTML + assert_equivalent_xml expected, output + end + test "adds class to wrapped form_group by a field" do expected = <<-HTML.strip_heredoc
From 0d763008e674079cf9ae65a880f8df2f10eea4da Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 18:55:05 -0800 Subject: [PATCH 13/17] Clean up comments, etc. --- lib/bootstrap_form/form_builder.rb | 8 -------- lib/bootstrap_form/helpers/bootstrap.rb | 1 - 2 files changed, 9 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index d6ca0d67f..90d50ed99 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -265,8 +265,6 @@ 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 - # puts "Just before capturing block #{name} #{options}" control = capture(&block) help = options[:help] @@ -419,10 +417,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 @@ -451,9 +445,7 @@ def form_group_builder(method, options, html_options = nil) end end - # puts "Just before form_group #{method} #{options}" form_group(method, form_group_options) do - # puts "block in form_group #{method} #{options}" yield end end diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index ba67c216a..401b8bd8a 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -77,7 +77,6 @@ def custom_control(*args, &block) end def prepend_and_append_input(name, options, &block) - # puts "WELL HERE WE ARE #{name} #{options}" options = options.extract!(:prepend, :append, :input_group_class) input_group_class = ["input-group", options[:input_group_class]].compact.join(' ') From 769cf009cbf6b6ce94f00d076210f2d5268ce44c Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 19:11:21 -0800 Subject: [PATCH 14/17] Fix tests and code for file and date/time selects. --- lib/bootstrap_form/form_builder.rb | 10 ++++++---- test/bootstrap_fields_test.rb | 2 +- test/bootstrap_selects_test.rb | 6 +++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 90d50ed99..0d4582f06 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -53,8 +53,10 @@ def initialize(object_name, object, template, options) define_method(with_method_name) do |name, options = {}, html_options = {}| form_group_builder(name, options, html_options) do - input_with_error(name) 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 @@ -65,8 +67,8 @@ def initialize(object_name, object, template, options) def file_field_with_bootstrap(name, options = {}) options = options.reverse_merge(control_class: "custom-file-input") form_group_builder(name, options) do - input_with_error(name) do - content_tag(:div, class: "custom-file") do + content_tag(:div, class: "custom-file") do + 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? diff --git a/test/bootstrap_fields_test.rb b/test/bootstrap_fields_test.rb index a26d544f4..26932f0c0 100644 --- a/test/bootstrap_fields_test.rb +++ b/test/bootstrap_fields_test.rb @@ -106,8 +106,8 @@ class BootstrapFieldsTest < ActionView::TestCase
+
error for test
-
error for test
HTML diff --git a/test/bootstrap_selects_test.rb b/test/bootstrap_selects_test.rb index 2ccfa7c8c..2e2e24d89 100644 --- a/test/bootstrap_selects_test.rb +++ b/test/bootstrap_selects_test.rb @@ -293,8 +293,8 @@ def options_range(start: 1, stop: 31, selected: nil, months: false) +
error for test
-
error for test
HTML @@ -395,8 +395,8 @@ def options_range(start: 1, stop: 31, selected: nil, months: false) +
error for test
-
error for test
HTML @@ -510,8 +510,8 @@ def options_range(start: 1, stop: 31, selected: nil, months: false) +
error for test
-
error for test
HTML From 9d73dd565ba5dc0e71446133ae4b1b444db5d864 Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Wed, 7 Mar 2018 19:33:38 -0800 Subject: [PATCH 15/17] Update CHANGELOG. --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e77c6f62..8b3f7c71a 100644 --- a/CHANGELOG.md +++ b/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 From 6f27cefe7fe603f71149b128007d876d21a46c11 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Mon, 28 May 2018 17:23:21 -0700 Subject: [PATCH 16/17] 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 %{

Bar

}.html_safe end expected = <<-HTML.strip_heredoc

Bar

can't be blank, is too short (minimum is 5 characters)
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. --- test/bootstrap_form_group_test.rb | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/test/bootstrap_form_group_test.rb b/test/bootstrap_form_group_test.rb index 37bd7b95a..70f74a7ec 100644 --- a/test/bootstrap_form_group_test.rb +++ b/test/bootstrap_form_group_test.rb @@ -374,25 +374,6 @@ class BootstrapFormGroupTest < ActionView::TestCase assert_equivalent_xml expected, output end - # NOTE: Removing this test case as 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 - # %{

Bar

}.html_safe - # end - # - # expected = <<-HTML.strip_heredoc - #
- #

Bar

- #
can't be blank, is too short (minimum is 5 characters)
- #
- # HTML - # assert_equivalent_xml expected, output - # end - - # It could be argued that this test replaces the commented-out test above. test 'upgrade doc for form_group renders the "error" class and message corrrectly when object is invalid' do @user.email = nil assert @user.invalid? From 7c6298993aadd8770f778e12c8a1cac4cbc2730e Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Mon, 28 May 2018 17:52:24 -0700 Subject: [PATCH 17/17] Error message must be sibling of input --- test/bootstrap_form_group_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bootstrap_form_group_test.rb b/test/bootstrap_form_group_test.rb index 70f74a7ec..7d25beacf 100644 --- a/test/bootstrap_form_group_test.rb +++ b/test/bootstrap_form_group_test.rb @@ -199,8 +199,8 @@ class BootstrapFormGroupTest < ActionView::TestCase
+ This is required
- This is required HTML assert_equivalent_xml expected, @horizontal_builder.text_field(:email, help: "This is required")