From a187460347dfb9bbdc0df4e8ad58c42adec03e2a Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Sat, 1 Oct 2022 13:01:04 -0700 Subject: [PATCH 1/2] Don't use `concat` when not necessary --- lib/bootstrap_form/components/labels.rb | 2 +- lib/bootstrap_form/components/validation.rb | 2 +- lib/bootstrap_form/form_group.rb | 13 +++--- lib/bootstrap_form/helpers/bootstrap.rb | 11 +++-- lib/bootstrap_form/inputs/check_box.rb | 4 +- lib/bootstrap_form/inputs/radio_button.rb | 4 +- test/bootstrap_form_group_test.rb | 12 +++--- test/bootstrap_form_test.rb | 48 ++++++++++----------- 8 files changed, 49 insertions(+), 47 deletions(-) diff --git a/lib/bootstrap_form/components/labels.rb b/lib/bootstrap_form/components/labels.rb index a66ec2760..3a2dbc782 100644 --- a/lib/bootstrap_form/components/labels.rb +++ b/lib/bootstrap_form/components/labels.rb @@ -46,7 +46,7 @@ def label_layout_classes(custom_label_col, group_layout) def label_text(name, options) if label_errors && error?(name) - (options[:text] || object.class.human_attribute_name(name)).to_s.concat(" #{get_error_messages(name)}") + (options[:text] || object.class.human_attribute_name(name)).to_s + " #{get_error_messages(name)}" else options[:text] || object&.class.try(:human_attribute_name, name) end diff --git a/lib/bootstrap_form/components/validation.rb b/lib/bootstrap_form/components/validation.rb index 221149b22..aa142c703 100644 --- a/lib/bootstrap_form/components/validation.rb +++ b/lib/bootstrap_form/components/validation.rb @@ -69,7 +69,7 @@ def get_error_messages(name) next unless a.is_a?(ActiveRecord::Reflection::BelongsToReflection) next unless a.foreign_key == name.to_s - messages.concat object.errors[association_name] + messages << object.errors[association_name] end messages.join(", ") end diff --git a/lib/bootstrap_form/form_group.rb b/lib/bootstrap_form/form_group.rb index 7e8ea4105..30aee017a 100644 --- a/lib/bootstrap_form/form_group.rb +++ b/lib/bootstrap_form/form_group.rb @@ -33,14 +33,17 @@ def form_group_content_tag(name, field_name, without_field_name, options, html_o end def form_group_content(label, help_text, options, &block) + label ||= ActiveSupport::SafeBuffer.new if group_layout_horizontal?(options[:layout]) - concat(label) << tag.div(capture(&block) + help_text, class: form_group_control_class(options)) + label + tag.div(capture(&block) + help_text, class: form_group_control_class(options)) else + content = ActiveSupport::SafeBuffer.new # Floating labels need to be rendered after the field - concat(label) unless options[:floating] - concat(capture(&block)) - concat(label) if options[:floating] - concat(help_text) if help_text + content << label unless options[:floating] + content << capture(&block) + content << label if options[:floating] + content << help_text if help_text + content end end diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index e2a68714a..11e8778ff 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -30,8 +30,7 @@ def alert_message(title, options={}) if options[:error_summary] == false title else - concat tag.p title - concat error_summary + tag.p(title) + error_summary end end end @@ -39,9 +38,9 @@ def alert_message(title, options={}) def error_summary return unless object.errors.any? - tag.ul class: "rails-bootstrap-forms-error-summary" do - object.errors.full_messages.each do |error| - concat tag.li(error) + tag.ul(class: "rails-bootstrap-forms-error-summary") do + object.errors.full_messages.reduce(ActiveSupport::SafeBuffer.new) do |acc, error| + acc << tag.li(error) end end end @@ -88,7 +87,7 @@ def prepend_and_append_input(name, options, &block) input = capture(&block) || ActiveSupport::SafeBuffer.new input = attach_input(options, :prepend) + input + attach_input(options, :append) - input += generate_error(name) + input << generate_error(name) options.present? && input = tag.div(input, class: ["input-group", options[:input_group_class]].compact) input diff --git a/lib/bootstrap_form/inputs/check_box.rb b/lib/bootstrap_form/inputs/check_box.rb index 7ca5424b4..8ad106904 100644 --- a/lib/bootstrap_form/inputs/check_box.rb +++ b/lib/bootstrap_form/inputs/check_box.rb @@ -15,8 +15,8 @@ def check_box_with_bootstrap(name, options={}, checked_value="1", unchecked_valu tag.div(class: check_box_wrapper_class(options), **options[:wrapper].to_h.except(:class)) do html = check_box_without_bootstrap(name, check_box_options, checked_value, unchecked_value) - html.concat(check_box_label(name, options, checked_value, &block)) unless options[:skip_label] - html.concat(generate_error(name)) if options[:error_message] + html << check_box_label(name, options, checked_value, &block) unless options[:skip_label] + html << generate_error(name) if options[:error_message] html end end diff --git a/lib/bootstrap_form/inputs/radio_button.rb b/lib/bootstrap_form/inputs/radio_button.rb index 55627b4a6..45583a272 100644 --- a/lib/bootstrap_form/inputs/radio_button.rb +++ b/lib/bootstrap_form/inputs/radio_button.rb @@ -13,8 +13,8 @@ def radio_button_with_bootstrap(name, value, *args) wrapper_attributes[:class] = radio_button_wrapper_class(options) tag.div(**wrapper_attributes) do html = radio_button_without_bootstrap(name, value, radio_button_options(name, options)) - html.concat(radio_button_label(name, value, options)) unless options[:skip_label] - html.concat(generate_error(name)) if options[:error_message] + html << radio_button_label(name, value, options) unless options[:skip_label] + html << generate_error(name) if options[:error_message] html end end diff --git a/test/bootstrap_form_group_test.rb b/test/bootstrap_form_group_test.rb index fa127b582..5d85b0c18 100644 --- a/test/bootstrap_form_group_test.rb +++ b/test/bootstrap_form_group_test.rb @@ -393,8 +393,8 @@ class BootstrapFormGroupTest < ActionView::TestCase output = @builder.form_group :email do html = '

Bar

'.html_safe unless @user.errors[:email].empty? - html.concat(tag.div(@user.errors[:email].join(", "), class: "invalid-feedback", - style: "display: block;")) + html << tag.div(@user.errors[:email].join(", "), class: "invalid-feedback", + style: "display: block;") end html end @@ -413,9 +413,9 @@ class BootstrapFormGroupTest < ActionView::TestCase 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)) + concat(f.radio_button(:misc, "primary school")) + concat(f.radio_button(:misc, "high school")) + concat(f.radio_button(:misc, "university", error_message: true)) end end @@ -528,7 +528,7 @@ class BootstrapFormGroupTest < ActionView::TestCase "Hallo" end - output += @horizontal_builder.text_field(:email) + output << @horizontal_builder.text_field(:email) expected = <<~HTML
diff --git a/test/bootstrap_form_test.rb b/test/bootstrap_form_test.rb index d54ffc77e..d032484ad 100644 --- a/test/bootstrap_form_test.rb +++ b/test/bootstrap_form_test.rb @@ -56,10 +56,10 @@ class BootstrapFormTest < ActionView::TestCase collection = [Address.new(id: 1, street: "Foo"), Address.new(id: 2, street: "Bar")] actual = bootstrap_form_for(@user) do |f| - f.email_field(:email, layout: :horizontal) - .concat(f.check_box(:terms, label: "I agree to the terms")) - .concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :horizontal)) - .concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :horizontal)) + concat(f.email_field(:email, layout: :horizontal)) + concat(f.check_box(:terms, label: "I agree to the terms")) + concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :horizontal)) + concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :horizontal)) end assert_equivalent_xml expected, actual @@ -102,10 +102,10 @@ class BootstrapFormTest < ActionView::TestCase collection = [Address.new(id: 1, street: "Foo"), Address.new(id: 2, street: "Bar")] actual = bootstrap_form_for(@user) do |f| - f.email_field(:email, layout: :inline) - .concat(f.check_box(:terms, label: "I agree to the terms", inline: true)) - .concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :inline)) - .concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :inline)) + concat(f.email_field(:email, layout: :inline)) + concat(f.check_box(:terms, label: "I agree to the terms", inline: true)) + concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :inline)) + concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :inline)) end assert_equivalent_xml expected, actual @@ -164,10 +164,10 @@ class BootstrapFormTest < ActionView::TestCase collection = [Address.new(id: 1, street: "Foo"), Address.new(id: 2, street: "Bar")] actual = bootstrap_form_for(@user, layout: :inline) do |f| - f.email_field(:email) - .concat(f.check_box(:terms, label: "I agree to the terms")) - .concat(f.collection_radio_buttons(:misc, collection, :id, :street)) - .concat(f.select(:status, [["activated", 1], ["blocked", 2]])) + concat(f.email_field(:email)) + concat(f.check_box(:terms, label: "I agree to the terms")) + concat(f.collection_radio_buttons(:misc, collection, :id, :street)) + concat(f.select(:status, [["activated", 1], ["blocked", 2]])) end assert_equivalent_xml expected, actual @@ -215,10 +215,10 @@ class BootstrapFormTest < ActionView::TestCase collection = [Address.new(id: 1, street: "Foo"), Address.new(id: 2, street: "Bar")] actual = bootstrap_form_for(@user, layout: :horizontal) do |f| - f.email_field(:email) - .concat(f.check_box(:terms, label: "I agree to the terms")) - .concat(f.collection_radio_buttons(:misc, collection, :id, :street)) - .concat(f.select(:status, [["activated", 1], ["blocked", 2]])) + concat(f.email_field(:email)) + concat(f.check_box(:terms, label: "I agree to the terms")) + concat(f.collection_radio_buttons(:misc, collection, :id, :street)) + concat(f.select(:status, [["activated", 1], ["blocked", 2]])) end assert_equivalent_xml expected, actual @@ -260,10 +260,10 @@ class BootstrapFormTest < ActionView::TestCase collection = [Address.new(id: 1, street: "Foo"), Address.new(id: 2, street: "Bar")] actual = bootstrap_form_for(@user, layout: :horizontal) do |f| - f.email_field(:email, layout: :default) - .concat(f.check_box(:terms, label: "I agree to the terms")) - .concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :default)) - .concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :default)) + concat(f.email_field(:email, layout: :default)) + concat(f.check_box(:terms, label: "I agree to the terms")) + concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :default)) + concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :default)) end assert_equivalent_xml expected, actual @@ -306,10 +306,10 @@ class BootstrapFormTest < ActionView::TestCase collection = [Address.new(id: 1, street: "Foo"), Address.new(id: 2, street: "Bar")] actual = bootstrap_form_for(@user, layout: :horizontal) do |f| - f.email_field(:email, layout: :inline) - .concat(f.check_box(:terms, label: "I agree to the terms", inline: true)) - .concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :inline)) - .concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :inline)) + concat(f.email_field(:email, layout: :inline)) + concat(f.check_box(:terms, label: "I agree to the terms", inline: true)) + concat(f.collection_radio_buttons(:misc, collection, :id, :street, layout: :inline)) + concat(f.select(:status, [["activated", 1], ["blocked", 2]], layout: :inline)) end assert_equivalent_xml expected, actual From 75a305d55f50bd9865f2f5178e86244f8ed4e94d Mon Sep 17 00:00:00 2001 From: Larry Reid Date: Sat, 1 Oct 2022 13:13:14 -0700 Subject: [PATCH 2/2] Silence Rubocop --- lib/bootstrap_form/form_group.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bootstrap_form/form_group.rb b/lib/bootstrap_form/form_group.rb index 30aee017a..e89707881 100644 --- a/lib/bootstrap_form/form_group.rb +++ b/lib/bootstrap_form/form_group.rb @@ -32,7 +32,7 @@ def form_group_content_tag(name, field_name, without_field_name, options, html_o end end - def form_group_content(label, help_text, options, &block) + def form_group_content(label, help_text, options, &block) # rubocop:disable Metrics/AbcSize label ||= ActiveSupport::SafeBuffer.new if group_layout_horizontal?(options[:layout]) label + tag.div(capture(&block) + help_text, class: form_group_control_class(options))