From 6b78c36b1860b066964dbcffa8010cdd35f59619 Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 13:09:59 +0000 Subject: [PATCH 01/10] Clean up Metrics/AbcSize offences --- .rubocop_todo.yml | 40 +-- lib/bootstrap_form/form_builder.rb | 322 +++++++++++++--------- lib/bootstrap_form/helpers/bootstrap.rb | 21 +- lib/bootstrap_form/inputs/check_box.rb | 21 +- lib/bootstrap_form/inputs/radio_button.rb | 89 +++--- 5 files changed, 282 insertions(+), 211 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c1a785fe9..6ae81ed3d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -2,7 +2,7 @@ inherit_from: .rubocop.yml # This configuration was generated by # `rubocop --auto-gen-config` -# on 2018-11-01 19:28:56 +0000 using RuboCop version 0.60.0. +# on 2019-02-19 13:05:41 +0000 using RuboCop version 0.64.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -16,18 +16,14 @@ Layout/IndentHeredoc: Exclude: - 'Dangerfile' -# Offense count: 9 -Metrics/AbcSize: - Max: 69 - # Offense count: 2 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 445 + Max: 348 -# Offense count: 7 +# Offense count: 1 Metrics/CyclomaticComplexity: - Max: 18 + Max: 7 # Offense count: 1 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. @@ -35,20 +31,16 @@ Metrics/CyclomaticComplexity: Metrics/LineLength: Max: 137 -# Offense count: 11 +# Offense count: 6 # Configuration parameters: CountComments, ExcludedMethods. Metrics/MethodLength: - Max: 45 + Max: 19 -# Offense count: 2 +# Offense count: 3 # Configuration parameters: CountKeywordArgs. Metrics/ParameterLists: Max: 8 -# Offense count: 5 -Metrics/PerceivedComplexity: - Max: 20 - # Offense count: 2 # Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist, MethodDefinitionMacros. # NamePrefix: is_, has_, have_ @@ -67,38 +59,26 @@ Rails/FilePath: Exclude: - 'demo/config/environments/development.rb' -# Offense count: 23 +# Offense count: 6 Rails/OutputSafety: Exclude: - - 'bootstrap_error_rendering_test.rb' - 'lib/bootstrap_form/form_builder.rb' - 'lib/bootstrap_form/helpers/bootstrap.rb' - - 'test/bootstrap_checkbox_test.rb' - - 'test/bootstrap_form_group_test.rb' - - 'test/bootstrap_other_components_test.rb' - - 'test/bootstrap_radio_button_test.rb' # Offense count: 1 Style/CaseEquality: Exclude: - 'test/test_helper.rb' -# Offense count: 6 +# Offense count: 5 # Configuration parameters: MinBodyLength. Style/GuardClause: Exclude: - 'lib/bootstrap_form/form_builder.rb' - 'lib/bootstrap_form/helpers/bootstrap.rb' -# Offense count: 2 +# Offense count: 1 # Cop supports --auto-correct. Style/IfUnlessModifier: Exclude: - 'demo/config/application.rb' - - 'lib/bootstrap_form/helper.rb' - -# Offense count: 2 -Style/MixinUsage: - Exclude: - - 'demo/bin/setup' - - 'demo/bin/update' diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index b59180970..a8350b1cd 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -60,46 +60,22 @@ def form_group(*args, &block) options = args.extract_options! name = args.first - options[:class] = ["form-group", options[:class]].compact.join(" ") - options[:class] << " row" if get_group_layout(options[:layout]) == :horizontal && - !options[:class].split.include?("form-row") - options[:class] << " form-inline" if field_inline_override?(options[:layout]) - options[:class] << " #{feedback_class}" if options[:icon] + options[:class] = form_group_classes(options) content_tag(:div, options.except(:append, :id, :label, :help, :icon, :input_group_class, :label_col, :control_col, :add_control_col_class, :layout, :prepend)) do - label = generate_label(options[:id], name, options[:label], options[:label_col], options[:layout]) if options[:label] - control = capture(&block) - - help = options[:help] - help_text = generate_help(name, help).to_s - - if get_group_layout(options[:layout]) == :horizontal - control_class = options[:control_col] || control_col - control_class = [control_class, options[:add_control_col_class]].compact.join(" ") if options[:add_control_col_class] - unless options[:label] - control_offset = offset_col(options[:label_col] || @label_col) - control_class = "#{control_class} #{control_offset}" - end - control = content_tag(:div, control + help_text, class: control_class) - concat(label).concat(control) - else - concat(label).concat(control).concat(help_text) - end + form_group_content( + generate_label(options[:id], name, options[:label], options[:label_col], options[:layout]), + generate_help(name, options[:help]), options, &block + ) end end def fields_for_with_bootstrap(record_name, record_object=nil, fields_options={}, &block) - if record_object.is_a?(Hash) && record_object.extractable_options? - fields_options = record_object + fields_options = fields_for_options(record_object, fields_options) + record_object.is_a?(Hash) && record_object.extractable_options? && record_object = nil - end - fields_options[:layout] ||= options[:layout] - fields_options[:label_col] = fields_options[:label_col].present? ? (fields_options[:label_col]).to_s : options[:label_col] - fields_options[:control_col] ||= options[:control_col] - fields_options[:inline_errors] ||= options[:inline_errors] - fields_options[:label_errors] ||= options[:label_errors] fields_for_without_bootstrap(record_name, record_object, fields_options, &block) end @@ -111,6 +87,44 @@ def fields_for_with_bootstrap(record_name, record_object=nil, fields_options={}, private + def form_group_content(label, help_text, options, &block) + if group_layout_horizontal?(options[:layout]) + concat(label).concat(content_tag(:div, capture(&block) + help_text, class: form_group_control_class(options))) + else + concat(label).concat(capture(&block)).concat(help_text) + end + end + + def form_group_control_class(options) + classes = [options[:control_col] || control_col] + classes << options[:add_control_col_class] if options[:add_control_col_class] + classes << offset_col(options[:label_col] || @label_col) unless options[:label] + classes.flatten.compact + end + + def form_group_classes(options) + classes = ["form-group", options[:class].try(:split)].flatten.compact + classes << "row" if group_layout_horizontal?(options[:layout]) && classes.exclude?("form-row") + classes << "form-inline" if field_inline_override?(options[:layout]) + classes << feedback_class if options[:icon] + classes + end + + def group_layout_horizontal?(layout) + get_group_layout(layout) == :horizontal + end + + def fields_for_options(record_object, fields_options) + field_options = fields_options + record_object.is_a?(Hash) && record_object.extractable_options? && + field_options = record_object + %i[layout control_col inline_errors label_errors].each do |option| + field_options[option] ||= options[option] + end + field_options[:label_col] = field_options[:label_col].present? ? (field_options[:label_col]).to_s : options[:label_col] + field_options + end + def layout_default?(field_layout=nil) [:default, nil].include? layout_in_effect(field_layout) end @@ -195,74 +209,92 @@ def required_attribute?(obj, attribute) has_presence_validator end - # # TODO: Refactor this method and remove the rubocop:disable - def form_group_builder(method, options, html_options=nil) # rubocop:disable Metrics/MethodLength + def form_group_builder(method, options, html_options=nil) options.symbolize_keys! + options = convert_form_tag_options(method, options) if acts_like_form_tag + wrapper_options = options[:wrapper] == false + css_options = form_group_css_options(method, html_options.try(:symbolize_keys!), options) - wrapper_class = options.delete(:wrapper_class) - wrapper_options = options.delete(:wrapper) + unless options[:skip_label] + options[:required] = form_group_required(options) if options.key?(:skip_required) + end - html_options.symbolize_keys! if html_options + form_group_options = form_group_opts(options, css_options) - # Add control_class; allow it to be overridden by :control_class option - css_options = html_options || options - control_classes = css_options.delete(:control_class) { control_class } - css_options[:class] = [control_classes, css_options[:class]].compact.join(" ") - css_options[:class] << " is-invalid" if has_error?(method) + options.except!( + :help, :icon, :label_col, :control_col, :add_control_col_class, :layout, :skip_label, :label, :label_class, + :hide_label, :skip_required, :label_as_placeholder, :wrapper_class, :wrapper + ) - options = convert_form_tag_options(method, options) if acts_like_form_tag + if wrapper_options + yield + else + form_group(method, form_group_options) do + yield + end + end + end - help = options.delete(:help) - icon = options.delete(:icon) - label_col = options.delete(:label_col) - control_col = options.delete(:control_col) - add_control_col_class = options.delete(:add_control_col_class) - layout = get_group_layout(options.delete(:layout)) + def form_group_opts(options, css_options) + wrapper_options = options[:wrapper] form_group_options = { id: options[:id], - help: help, - icon: icon, - label_col: label_col, - control_col: control_col, - add_control_col_class: add_control_col_class, - layout: layout, - class: wrapper_class + help: options[:help], + icon: options[:icon], + label_col: options[:label_col], + control_col: options[:control_col], + add_control_col_class: options[:add_control_col_class], + layout: get_group_layout(options[:layout]), + class: options[:wrapper_class] } form_group_options.merge!(wrapper_options) if wrapper_options.is_a?(Hash) + form_group_options[:label] = form_group_label(options, css_options) unless options[:skip_label] + form_group_options + end - unless options.delete(:skip_label) - if options[:label].is_a?(Hash) - label_text = options[:label].delete(:text) - label_class = options[:label].delete(:class) - options.delete(:label) - end - label_class ||= options.delete(:label_class) - label_class = hide_class if options.delete(:hide_label) || options[:label_as_placeholder] + def form_group_label(options, css_options) + hash = { + text: form_group_label_text(options[:label]), + class: form_group_label_class(options), + required: options[:required] + }.merge(css_options[:id].present? ? { for: css_options[:id] } : {}) + hash + end - label_text ||= options.delete(:label) if options[:label].is_a?(String) + def form_group_label_text(label) + text = label[:text] if label.is_a?(Hash) + text ||= label if label.is_a?(String) + text + end - if options.key?(:skip_required) - warn "`:skip_required` is deprecated, use `:required: false` instead" - options[:required] = options.delete(:skip_required) ? false : :default - end + def form_group_label_class(options) + return hide_class if options[:hide_label] || options[:label_as_placeholder] - form_group_options[:label] = { - text: label_text, - class: label_class, - required: options[:required] - }.merge(css_options[:id].present? ? { for: css_options[:id] } : {}) + classes = options[:label][:class] if options[:label].is_a?(Hash) + classes ||= options[:label_class] + classes + end - css_options[:placeholder] = label_text || object.class.human_attribute_name(method) if options.delete(:label_as_placeholder) + def form_group_required(options) + if options.key?(:skip_required) + warn "`:skip_required` is deprecated, use `:required: false` instead" + options[:skip_required] ? false : :default end + end - if wrapper_options == false - yield - else - form_group(method, form_group_options) do - yield - end - end + def form_group_css_options(method, html_options, options) + css_options = html_options || options + # Add control_class; allow it to be overridden by :control_class option + control_classes = css_options.delete(:control_class) { control_class } + css_options[:class] = [control_classes, css_options[:class]].compact.join(" ") + css_options[:class] << " is-invalid" if has_error?(method) + css_options[:placeholder] = form_group_placeholder(options, method) if options[:label_as_placeholder] + css_options + end + + def form_group_placeholder(options, method) + form_group_label_text(options[:label]) || object.class.human_attribute_name(method) end def convert_form_tag_options(method, options={}) @@ -274,19 +306,22 @@ def convert_form_tag_options(method, options={}) end def generate_label(id, name, options, custom_label_col, group_layout) + return if options.blank? + # 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]] - if layout_horizontal?(group_layout) - classes << "col-form-label" - classes << (custom_label_col || label_col) - elsif layout_inline?(group_layout) - classes << "mr-sm-2" - end + options[:class] = label_classes(name, options, custom_label_col, group_layout) + options.delete(:class) if options[:class].none? + + label(name, label_text(name, options), options.except(:text)) + end + + def label_classes(name, options, custom_label_col, group_layout) + classes = [options[:class], label_layout_classes(custom_label_col, group_layout)] case options.delete(:required) when true @@ -295,16 +330,23 @@ def generate_label(id, name, options, custom_label_col, group_layout) classes << "required" if required_attribute?(object, name) end - options[:class] = classes.compact.join(" ").strip - options.delete(:class) if options[:class].empty? + classes << "text-danger" if label_errors && has_error?(name) + classes.flatten.compact + end + + def label_layout_classes(custom_label_col, group_layout) + if layout_horizontal?(group_layout) + ["col-form-label", (custom_label_col || label_col)] + elsif layout_inline?(group_layout) + ["mr-sm-2"] + end + end + def label_text(name, options) if label_errors && has_error?(name) - error_messages = get_error_messages(name) - label_text = (options[:text] || object.class.human_attribute_name(name)).to_s.concat(" #{error_messages}") - options[:class] = [options[:class], "text-danger"].compact.join(" ") - label(name, label_text, options.except(:text)) + (options[:text] || object.class.human_attribute_name(name)).to_s.concat(" #{get_error_messages(name)}") else - label(name, options[:text], options.except(:text)) + options[:text] end end @@ -319,6 +361,8 @@ def generate_error(name) help_tag = :div content_tag(help_tag, help_text, class: help_klass) + else + "" end end @@ -338,59 +382,69 @@ def get_error_messages(name) def inputs_collection(name, collection, value, text, options={}) options[:inline] ||= layout_inline?(options[:layout]) + form_group_builder(name, options) do inputs = "" 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) - if (checked = input_options[:checked]) - input_options[:checked] = checked == input_value || - Array(checked).try(:include?, input_value) || - checked == obj || - Array(checked).try(:include?, obj) - end - - input_options.delete(:class) - inputs << yield(name, input_value, input_options.merge(error_message: i == collection.size - 1)) + input_options = form_group_collection_input_options(options, text, obj, i, input_value, collection) + inputs << yield(name, input_value, input_options) end inputs.html_safe end end + def form_group_collection_input_options(options, text, obj, index, input_value, collection) + input_options = options.merge(label: text.respond_to?(:call) ? text.call(obj) : obj.send(text)) + if (checked = input_options[:checked]) + input_options[:checked] = form_group_collection_input_checked?(checked, obj, input_value) + end + + input_options[:error_message] = index == collection.size - 1 + input_options.except!(:class) + input_options + end + + def form_group_collection_input_checked?(checked, obj, input_value) + checked == input_value || Array(checked).try(:include?, input_value) || + checked == obj || Array(checked).try(:include?, obj) + end + def get_help_text_by_i18n_key(name) - if object - - partial_scope = if object.class.respond_to?(:model_name) - object.class.model_name.name - else - object.class.name - end - - underscored_scope = "activerecord.help.#{partial_scope.underscore}" - downcased_scope = "activerecord.help.#{partial_scope.downcase}" - # First check for a subkey :html, as it is also accepted by i18n, and the - # simple check for name would return an hash instead of a string (both - # with .presence returning true!) - help_text = I18n.t("#{name}.html", scope: underscored_scope, default: "").html_safe.presence - help_text ||= if (text = I18n.t("#{name}.html", scope: downcased_scope, default: "").html_safe.presence) - warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" - text - end - help_text ||= I18n.t(name, scope: underscored_scope, default: "").presence - help_text ||= if (text = I18n.t(name, scope: downcased_scope, default: "").presence) - warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" - text - end - help_text ||= I18n.t("#{name}_html", scope: underscored_scope, default: "").html_safe.presence - help_text ||= if (text = I18n.t("#{name}_html", scope: downcased_scope, default: "").html_safe.presence) - warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" - text + return unless object + + partial_scope = if object.class.respond_to?(:model_name) + object.class.model_name.name + else + object.class.name end - help_text + + # First check for a subkey :html, as it is also accepted by i18n, and the + # simple check for name would return an hash instead of a string (both + # with .presence returning true!) + help_text = nil + ["#{name}.html", name, "#{name}_html"].each do |scope| + break if help_text + + help_text = scoped_help_text(scope, partial_scope) end + help_text + end + + def scoped_help_text(name, partial_scope) + underscored_scope = "activerecord.help.#{partial_scope.underscore}" + downcased_scope = "activerecord.help.#{partial_scope.downcase}" + + help_text = I18n.t(name, scope: underscored_scope, default: "").html_safe.presence + + help_text ||= if (text = I18n.t(name, scope: downcased_scope, default: "").html_safe.presence) + warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" + text + end + + help_text end end end diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index 82458d2a7..da680fe74 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -63,7 +63,7 @@ def static_control(*args) static_options = options.merge( readonly: true, - control_class: [options[:control_class], static_class].compact.join(" ") + control_class: [options[:control_class], static_class].compact ) static_options[:value] = object.send(name) unless static_options.key?(:value) @@ -80,14 +80,13 @@ def custom_control(*args, &block) def prepend_and_append_input(name, options, &block) options = options.extract!(:prepend, :append, :input_group_class) - input_group_class = ["input-group", options[:input_group_class]].compact.join(" ") input = capture(&block) || "".html_safe - input = content_tag(:div, input_group_content(options[:prepend]), class: "input-group-prepend") + input if options[:prepend] - input << content_tag(:div, input_group_content(options[:append]), class: "input-group-append") if options[:append] - input << generate_error(name) - input = content_tag(:div, input, class: input_group_class) unless options.empty? + input = prepend_input(options) + input + append_input(options) + input += generate_error(name) + options.present? && + input = content_tag(:div, input.html_safe, class: ["input-group", options[:input_group_class]].compact) input end @@ -108,6 +107,16 @@ def static_class private + def append_input(options) + html = content_tag(:div, input_group_content(options[:append]), class: "input-group-append") if options[:append] + (html || "").html_safe + end + + def prepend_input(options) + html = content_tag(:div, input_group_content(options[:prepend]), class: "input-group-prepend") if options[:prepend] + (html || "").html_safe + end + def setup_css_class(the_class, options={}) unless options.key? :class if (extra_class = options.delete(:extra_class)) diff --git a/lib/bootstrap_form/inputs/check_box.rb b/lib/bootstrap_form/inputs/check_box.rb index 364e11f38..9a79b0e22 100644 --- a/lib/bootstrap_form/inputs/check_box.rb +++ b/lib/bootstrap_form/inputs/check_box.rb @@ -27,8 +27,6 @@ def check_box_with_bootstrap(name, options={}, checked_value="1", unchecked_valu private def check_box_label(name, options, checked_value, &block) - content = block_given? ? capture(&block) : options[:label] - description = content || (object && object.class.human_attribute_name(name)) || name.to_s.humanize label_name = if options[:multiple] check_box_value(name, checked_value) else @@ -36,7 +34,12 @@ def check_box_label(name, options, checked_value, &block) end label_options = { class: check_box_label_class(options) } label_options[:for] = options[:id] if options[:id].present? - label(label_name, description, label_options) + label(label_name, check_box_description(name, options, &block), label_options) + end + + def check_box_description(name, options, &block) + content = block_given? ? capture(&block) : options[:label] + content || (object && object.class.human_attribute_name(name)) || name.to_s.humanize end def check_box_value(name, value) @@ -65,9 +68,7 @@ def check_box_label_class(options) def check_box_wrapper_class(options) classes = [] if options[:custom] - classes << "custom-control" - classes << (options[:custom] == :switch ? "custom-switch" : "custom-checkbox") - classes << "custom-control-inline" if layout_inline?(options[:inline]) + classes << custom_check_box_wrapper_class(options) else classes << "form-check" classes << "form-check-inline" if layout_inline?(options[:inline]) @@ -75,6 +76,14 @@ def check_box_wrapper_class(options) classes << options[:wrapper_class] if options[:wrapper_class].present? classes.flatten.compact end + + def custom_check_box_wrapper_class(options) + classes = [] + classes << "custom-control" + classes << (options[:custom] == :switch ? "custom-switch" : "custom-checkbox") + classes << "custom-control-inline" if layout_inline?(options[:inline]) + classes + end end end end diff --git a/lib/bootstrap_form/inputs/radio_button.rb b/lib/bootstrap_form/inputs/radio_button.rb index 7a6dedbc3..83ea53db8 100644 --- a/lib/bootstrap_form/inputs/radio_button.rb +++ b/lib/bootstrap_form/inputs/radio_button.rb @@ -6,45 +6,17 @@ module RadioButton extend ActiveSupport::Concern include Base - # rubocop:disable Metrics/BlockLength included do def radio_button_with_bootstrap(name, value, *args) options = args.extract_options!.symbolize_keys! - radio_options = options.except(:label, :label_class, :error_message, :help, - :inline, :custom, :hide_label, :skip_label, - :wrapper_class) - radio_classes = [options[:class]] - radio_classes << "position-static" if options[:skip_label] || options[:hide_label] - radio_classes << "is-invalid" if has_error?(name) - - label_classes = [options[:label_class]] - label_classes << hide_class if options[:hide_label] - - if options[:custom] - radio_options[:class] = radio_classes.prepend("custom-control-input").compact.join(" ") - wrapper_class = ["custom-control", "custom-radio"] - wrapper_class.append("custom-control-inline") if layout_inline?(options[:inline]) - label_class = label_classes.prepend("custom-control-label").compact.join(" ") - else - radio_options[:class] = radio_classes.prepend("form-check-input").compact.join(" ") - wrapper_class = ["form-check"] - wrapper_class.append("form-check-inline") if layout_inline?(options[:inline]) - wrapper_class.append("disabled") if options[:disabled] - label_class = label_classes.prepend("form-check-label").compact.join(" ") - end - radio_html = radio_button_without_bootstrap(name, value, radio_options) - - label_options = { value: value, class: label_class } - label_options[:for] = options[:id] if options[:id].present? + radio_button_options = options.except(:class, :label, :label_class, :error_message, :help, + :inline, :custom, :hide_label, :skip_label, :wrapper_class) - wrapper_class.append(options[:wrapper_class]) if options[:wrapper_class] + radio_button_options[:class] = radio_button_classes(name, options) - content_tag(:div, class: wrapper_class.compact.join(" ")) do - html = if options[:skip_label] - radio_html - else - radio_html.concat(label(name, options[:label], label_options)) - end + content_tag(:div, class: radio_button_wrapper_class(options)) do + html = radio_button_without_bootstrap(name, value, radio_button_options) + html.concat(radio_button_label(name, value, options)) unless options[:skip_label] html.concat(generate_error(name)) if options[:error_message] html end @@ -52,7 +24,54 @@ def radio_button_with_bootstrap(name, value, *args) bootstrap_alias :radio_button end - # rubocop:enable Metrics/BlockLength + + private + + def radio_button_label(name, value, options) + label_options = { value: value, class: radio_button_label_class(options) } + label_options[:for] = options[:id] if options[:id].present? + label(name, options[:label], label_options) + end + + def radio_button_classes(name, options) + classes = [options[:class]] + classes << (options[:custom] ? "custom-control-input" : "form-check-input") + classes << "is-invalid" if has_error?(name) + classes << "position-static" if options[:skip_label] || options[:hide_label] + classes.flatten.compact + end + + def radio_button_label_class(options) + classes = [] + classes << (options[:custom] ? "custom-control-label" : "form-check-label") + classes << options[:label_class] + classes << hide_class if options[:hide_label] + classes.flatten.compact + end + + def radio_button_wrapper_class(options) + classes = [] + classes << if options[:custom] + custom_radio_button_wrapper_class(options) + else + standard_radio_button_wrapper_class(options) + end + classes << options[:wrapper_class] if options[:wrapper_class].present? + classes.flatten.compact + end + + def standard_radio_button_wrapper_class(options) + classes = ["form-check"] + classes << "form-check-inline" if layout_inline?(options[:inline]) + classes << "disabled" if options[:disabled] + classes + end + + def custom_radio_button_wrapper_class(options) + classes = ["custom-control", "custom-radio"] + classes << "custom-control-inline" if layout_inline?(options[:inline]) + classes + end end end end From bf3855b41e17b4e6c6099634caa5a96c253c48dc Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 13:14:28 +0000 Subject: [PATCH 02/10] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac1fd86d4..bb1248af8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Bugfixes * Your contribution here! +* [#522](https://github.com/bootstrap-ruby/bootstrap_form/pull/522): Clean up Metrics/AbcSize offences - [@simmerz](https://github.com/simmerz) ## [4.1.0][] (2019-01-19) From 8a339f918ac571447fef00cde60b674a5ffbdbc0 Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 14:13:55 +0000 Subject: [PATCH 03/10] Clean up Metrics/MethodLength offences --- .rubocop_todo.yml | 14 +-- demo/app/helpers/bootstrap_helper.rb | 26 +++-- lib/bootstrap_form.rb | 2 + lib/bootstrap_form/form_builder.rb | 138 ++--------------------- lib/bootstrap_form/form_group.rb | 62 ++++++++++ lib/bootstrap_form/form_group_builder.rb | 95 ++++++++++++++++ lib/bootstrap_form/inputs/base.rb | 10 +- lib/bootstrap_form/inputs/file_field.rb | 19 ++-- 8 files changed, 198 insertions(+), 168 deletions(-) create mode 100644 lib/bootstrap_form/form_group.rb create mode 100644 lib/bootstrap_form/form_group_builder.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6ae81ed3d..ab498cb34 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -2,7 +2,7 @@ inherit_from: .rubocop.yml # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-02-19 13:05:41 +0000 using RuboCop version 0.64.0. +# on 2019-02-19 14:11:20 +0000 using RuboCop version 0.64.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -19,7 +19,7 @@ Layout/IndentHeredoc: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 348 + Max: 244 # Offense count: 1 Metrics/CyclomaticComplexity: @@ -31,17 +31,12 @@ Metrics/CyclomaticComplexity: Metrics/LineLength: Max: 137 -# Offense count: 6 -# Configuration parameters: CountComments, ExcludedMethods. -Metrics/MethodLength: - Max: 19 - # Offense count: 3 # Configuration parameters: CountKeywordArgs. Metrics/ParameterLists: Max: 8 -# Offense count: 2 +# Offense count: 3 # Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist, MethodDefinitionMacros. # NamePrefix: is_, has_, have_ # NamePrefixBlacklist: is_, has_, have_ @@ -70,11 +65,10 @@ Style/CaseEquality: Exclude: - 'test/test_helper.rb' -# Offense count: 5 +# Offense count: 4 # Configuration parameters: MinBodyLength. Style/GuardClause: Exclude: - - 'lib/bootstrap_form/form_builder.rb' - 'lib/bootstrap_form/helpers/bootstrap.rb' # Offense count: 1 diff --git a/demo/app/helpers/bootstrap_helper.rb b/demo/app/helpers/bootstrap_helper.rb index 153cf1c15..2060a8c0d 100644 --- a/demo/app/helpers/bootstrap_helper.rb +++ b/demo/app/helpers/bootstrap_helper.rb @@ -3,19 +3,25 @@ def form_with_source(&block) form_html = capture(&block) content_tag(:div, class: "example") do - codemirror = content_tag(:div, class: "code", style: "display: none") do - content_tag(:textarea, class: "codemirror") do - HtmlBeautifier.beautify(form_html.strip.gsub(">", ">\n").gsub("<", "\n<")) - end - end - - toggle = content_tag(:button, class: "toggle btn btn-sm btn-info") do - "Show Source Code" - end - concat(form_html) concat(toggle) concat(codemirror) end end + + private + + def codemirror(form_html) + content_tag(:div, class: "code", style: "display: none") do + content_tag(:textarea, class: "codemirror") do + HtmlBeautifier.beautify(form_html.strip.gsub(">", ">\n").gsub("<", "\n<")) + end + end + end + + def toggle + content_tag(:button, class: "toggle btn btn-sm btn-info") do + "Show Source Code" + end + end end diff --git a/lib/bootstrap_form.rb b/lib/bootstrap_form.rb index 58c3970cf..18efcacae 100644 --- a/lib/bootstrap_form.rb +++ b/lib/bootstrap_form.rb @@ -14,6 +14,8 @@ module BootstrapForm eager_autoload do autoload :FormBuilder + autoload :FormGroupBuilder + autoload :FormGroup autoload :Inputs autoload :Helpers end diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index a8350b1cd..555a8d02d 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -7,6 +7,9 @@ class FormBuilder < ActionView::Helpers::FormBuilder include BootstrapForm::Helpers::Bootstrap + include BootstrapForm::FormGroupBuilder + include BootstrapForm::FormGroup + include BootstrapForm::Inputs::Base include BootstrapForm::Inputs::CheckBox include BootstrapForm::Inputs::CollectionCheckBoxes @@ -56,22 +59,6 @@ def initialize(object_name, object, template, options) super end - def form_group(*args, &block) - options = args.extract_options! - name = args.first - - options[:class] = form_group_classes(options) - - content_tag(:div, options.except(:append, :id, :label, :help, :icon, - :input_group_class, :label_col, :control_col, - :add_control_col_class, :layout, :prepend)) do - form_group_content( - generate_label(options[:id], name, options[:label], options[:label_col], options[:layout]), - generate_help(name, options[:help]), options, &block - ) - end - end - def fields_for_with_bootstrap(record_name, record_object=nil, fields_options={}, &block) fields_options = fields_for_options(record_object, fields_options) record_object.is_a?(Hash) && record_object.extractable_options? && @@ -87,33 +74,6 @@ def fields_for_with_bootstrap(record_name, record_object=nil, fields_options={}, private - def form_group_content(label, help_text, options, &block) - if group_layout_horizontal?(options[:layout]) - concat(label).concat(content_tag(:div, capture(&block) + help_text, class: form_group_control_class(options))) - else - concat(label).concat(capture(&block)).concat(help_text) - end - end - - def form_group_control_class(options) - classes = [options[:control_col] || control_col] - classes << options[:add_control_col_class] if options[:add_control_col_class] - classes << offset_col(options[:label_col] || @label_col) unless options[:label] - classes.flatten.compact - end - - def form_group_classes(options) - classes = ["form-group", options[:class].try(:split)].flatten.compact - classes << "row" if group_layout_horizontal?(options[:layout]) && classes.exclude?("form-row") - classes << "form-inline" if field_inline_override?(options[:layout]) - classes << feedback_class if options[:icon] - classes - end - - def group_layout_horizontal?(layout) - get_group_layout(layout) == :horizontal - end - def fields_for_options(record_object, fields_options) field_options = fields_options record_object.is_a?(Hash) && record_object.extractable_options? && @@ -196,6 +156,10 @@ def required_attribute?(obj, attribute) [] end + has_presence_validator(target_validators) + end + + def has_presence_validator(target_validators) has_presence_validator = target_validators.include?( ActiveModel::Validations::PresenceValidator ) @@ -209,94 +173,6 @@ def required_attribute?(obj, attribute) has_presence_validator end - def form_group_builder(method, options, html_options=nil) - options.symbolize_keys! - options = convert_form_tag_options(method, options) if acts_like_form_tag - wrapper_options = options[:wrapper] == false - css_options = form_group_css_options(method, html_options.try(:symbolize_keys!), options) - - unless options[:skip_label] - options[:required] = form_group_required(options) if options.key?(:skip_required) - end - - form_group_options = form_group_opts(options, css_options) - - options.except!( - :help, :icon, :label_col, :control_col, :add_control_col_class, :layout, :skip_label, :label, :label_class, - :hide_label, :skip_required, :label_as_placeholder, :wrapper_class, :wrapper - ) - - if wrapper_options - yield - else - form_group(method, form_group_options) do - yield - end - end - end - - def form_group_opts(options, css_options) - wrapper_options = options[:wrapper] - form_group_options = { - id: options[:id], - help: options[:help], - icon: options[:icon], - label_col: options[:label_col], - control_col: options[:control_col], - add_control_col_class: options[:add_control_col_class], - layout: get_group_layout(options[:layout]), - class: options[:wrapper_class] - } - - form_group_options.merge!(wrapper_options) if wrapper_options.is_a?(Hash) - form_group_options[:label] = form_group_label(options, css_options) unless options[:skip_label] - form_group_options - end - - def form_group_label(options, css_options) - hash = { - text: form_group_label_text(options[:label]), - class: form_group_label_class(options), - required: options[:required] - }.merge(css_options[:id].present? ? { for: css_options[:id] } : {}) - hash - end - - def form_group_label_text(label) - text = label[:text] if label.is_a?(Hash) - text ||= label if label.is_a?(String) - text - end - - def form_group_label_class(options) - return hide_class if options[:hide_label] || options[:label_as_placeholder] - - classes = options[:label][:class] if options[:label].is_a?(Hash) - classes ||= options[:label_class] - classes - end - - def form_group_required(options) - if options.key?(:skip_required) - warn "`:skip_required` is deprecated, use `:required: false` instead" - options[:skip_required] ? false : :default - end - end - - def form_group_css_options(method, html_options, options) - css_options = html_options || options - # Add control_class; allow it to be overridden by :control_class option - control_classes = css_options.delete(:control_class) { control_class } - css_options[:class] = [control_classes, css_options[:class]].compact.join(" ") - css_options[:class] << " is-invalid" if has_error?(method) - css_options[:placeholder] = form_group_placeholder(options, method) if options[:label_as_placeholder] - css_options - end - - def form_group_placeholder(options, method) - form_group_label_text(options[:label]) || object.class.human_attribute_name(method) - end - def convert_form_tag_options(method, options={}) unless @options[:skip_default_ids] options[:name] ||= method diff --git a/lib/bootstrap_form/form_group.rb b/lib/bootstrap_form/form_group.rb new file mode 100644 index 000000000..aeefc8b1b --- /dev/null +++ b/lib/bootstrap_form/form_group.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module BootstrapForm + module FormGroup + extend ActiveSupport::Concern + + def form_group(*args, &block) + options = args.extract_options! + name = args.first + + options[:class] = form_group_classes(options) + + content_tag(:div, options.except(:append, :id, :label, :help, :icon, + :input_group_class, :label_col, :control_col, + :add_control_col_class, :layout, :prepend)) do + form_group_content( + generate_label(options[:id], name, options[:label], options[:label_col], options[:layout]), + generate_help(name, options[:help]), options, &block + ) + end + end + + private + + def form_group_content_tag(name, field_name, without_field_name, options, html_options) + html_class = control_specific_class(field_name) + html_class = "#{html_class} form-inline" if @layout == :horizontal && options[:skip_inline].blank? + content_tag(:div, class: html_class) do + input_with_error(name) do + send(without_field_name, name, options, html_options) + end + end + end + + def form_group_content(label, help_text, options, &block) + if group_layout_horizontal?(options[:layout]) + concat(label).concat(content_tag(:div, capture(&block) + help_text, class: form_group_control_class(options))) + else + concat(label).concat(capture(&block)).concat(help_text) + end + end + + def form_group_control_class(options) + classes = [options[:control_col] || control_col] + classes << options[:add_control_col_class] if options[:add_control_col_class] + classes << offset_col(options[:label_col] || @label_col) unless options[:label] + classes.flatten.compact + end + + def form_group_classes(options) + classes = ["form-group", options[:class].try(:split)].flatten.compact + classes << "row" if group_layout_horizontal?(options[:layout]) && classes.exclude?("form-row") + classes << "form-inline" if field_inline_override?(options[:layout]) + classes << feedback_class if options[:icon] + classes + end + + def group_layout_horizontal?(layout) + get_group_layout(layout) == :horizontal + end + end +end diff --git a/lib/bootstrap_form/form_group_builder.rb b/lib/bootstrap_form/form_group_builder.rb new file mode 100644 index 000000000..5ec92ec7b --- /dev/null +++ b/lib/bootstrap_form/form_group_builder.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module BootstrapForm + module FormGroupBuilder + extend ActiveSupport::Concern + + private + + def form_group_builder(method, options, html_options=nil) + no_wrapper = options[:wrapper] == false + + options = form_group_builder_options(options, method) + + form_group_options = form_group_opts(options, form_group_css_options(method, html_options.try(:symbolize_keys!), options)) + + options.except!( + :help, :icon, :label_col, :control_col, :add_control_col_class, :layout, :skip_label, :label, :label_class, + :hide_label, :skip_required, :label_as_placeholder, :wrapper_class, :wrapper + ) + + if no_wrapper + yield + else + form_group(method, form_group_options) { yield } + end + end + + def form_group_builder_options(options, method) + options.symbolize_keys! + options = convert_form_tag_options(method, options) if acts_like_form_tag + unless options[:skip_label] + options[:required] = form_group_required(options) if options.key?(:skip_required) + end + options + end + + def form_group_opts(options, css_options) + wrapper_options = options[:wrapper] + form_group_options = { + id: options[:id], help: options[:help], icon: options[:icon], + label_col: options[:label_col], control_col: options[:control_col], + add_control_col_class: options[:add_control_col_class], + layout: get_group_layout(options[:layout]), class: options[:wrapper_class] + } + + form_group_options.merge!(wrapper_options) if wrapper_options.is_a?(Hash) + form_group_options[:label] = form_group_label(options, css_options) unless options[:skip_label] + form_group_options + end + + def form_group_label(options, css_options) + hash = { + text: form_group_label_text(options[:label]), + class: form_group_label_class(options), + required: options[:required] + }.merge(css_options[:id].present? ? { for: css_options[:id] } : {}) + hash + end + + def form_group_label_text(label) + text = label[:text] if label.is_a?(Hash) + text ||= label if label.is_a?(String) + text + end + + def form_group_label_class(options) + return hide_class if options[:hide_label] || options[:label_as_placeholder] + + classes = options[:label][:class] if options[:label].is_a?(Hash) + classes ||= options[:label_class] + classes + end + + def form_group_required(options) + return unless options.key?(:skip_required) + + warn "`:skip_required` is deprecated, use `:required: false` instead" + options[:skip_required] ? false : :default + end + + def form_group_css_options(method, html_options, options) + css_options = html_options || options + # Add control_class; allow it to be overridden by :control_class option + control_classes = css_options.delete(:control_class) { control_class } + css_options[:class] = [control_classes, css_options[:class]].compact.join(" ") + css_options[:class] << " is-invalid" if has_error?(method) + css_options[:placeholder] = form_group_placeholder(options, method) if options[:label_as_placeholder] + css_options + end + + def form_group_placeholder(options, method) + form_group_label_text(options[:label]) || object.class.human_attribute_name(method) + end + end +end diff --git a/lib/bootstrap_form/inputs/base.rb b/lib/bootstrap_form/inputs/base.rb index 5bb07e6f6..d84c4e9b3 100644 --- a/lib/bootstrap_form/inputs/base.rb +++ b/lib/bootstrap_form/inputs/base.rb @@ -5,7 +5,6 @@ module Inputs module Base extend ActiveSupport::Concern - # rubocop:disable Metrics/BlockLength class_methods do def bootstrap_field(field_name) define_method "#{field_name}_with_bootstrap" do |name, options={}| @@ -24,13 +23,7 @@ def bootstrap_select_group(field_name) without_field_name = "#{field_name}_without_bootstrap" define_method(with_field_name) do |name, options={}, html_options={}| form_group_builder(name, options, html_options) do - html_class = control_specific_class(field_name) - html_class = "#{html_class} form-inline" if @layout == :horizontal && options[:skip_inline].blank? - content_tag(:div, class: html_class) do - input_with_error(name) do - send(without_field_name, name, options, html_options) - end - end + form_group_content_tag(name, field_name, without_field_name, options, html_options) end end @@ -42,7 +35,6 @@ def bootstrap_alias(field_name) alias_method field_name, "#{field_name}_with_bootstrap".to_sym end end - # rubocop:enable Metrics/BlockLength end end end diff --git a/lib/bootstrap_form/inputs/file_field.rb b/lib/bootstrap_form/inputs/file_field.rb index 79e2ac254..8e5dea119 100644 --- a/lib/bootstrap_form/inputs/file_field.rb +++ b/lib/bootstrap_form/inputs/file_field.rb @@ -12,14 +12,7 @@ def file_field_with_bootstrap(name, options={}) form_group_builder(name, options) 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? - - input = file_field_without_bootstrap(name, options) - placeholder_label = label(name, placeholder, placeholder_opts) - concat(input) - concat(placeholder_label) + file_filed_input(name, options) end end end @@ -27,6 +20,16 @@ def file_field_with_bootstrap(name, options={}) bootstrap_alias :file_field end + + private + + def file_filed_input(name, options) + placeholder = options.delete(:placeholder) || "Choose file" + placeholder_opts = { class: "custom-file-label" } + placeholder_opts[:for] = options[:id] if options[:id].present? + + file_field_without_bootstrap(name, options) + label(name, placeholder, placeholder_opts) + end end end end From 086240b882e9054f32b159e18d20752e98a2bb58 Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 14:15:53 +0000 Subject: [PATCH 04/10] Adjust changelog to be more accurate --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb1248af8..934c477fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Bugfixes * Your contribution here! -* [#522](https://github.com/bootstrap-ruby/bootstrap_form/pull/522): Clean up Metrics/AbcSize offences - [@simmerz](https://github.com/simmerz) +* [#522](https://github.com/bootstrap-ruby/bootstrap_form/pull/522): Clean up rubocop offences - [@simmerz](https://github.com/simmerz) ## [4.1.0][] (2019-01-19) From 52b6f81ac0f412a7203e31517f231709c987b9b9 Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 14:33:30 +0000 Subject: [PATCH 05/10] Clean up Metrics/CyclomaticComplexity --- .rubocop_todo.yml | 11 +---------- test/test_helper.rb | 44 ++++++++++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index ab498cb34..58ec4e8ca 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -2,7 +2,7 @@ inherit_from: .rubocop.yml # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-02-19 14:11:20 +0000 using RuboCop version 0.64.0. +# on 2019-02-19 14:32:34 +0000 using RuboCop version 0.64.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -21,10 +21,6 @@ Layout/IndentHeredoc: Metrics/ClassLength: Max: 244 -# Offense count: 1 -Metrics/CyclomaticComplexity: - Max: 7 - # Offense count: 1 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https @@ -60,11 +56,6 @@ Rails/OutputSafety: - 'lib/bootstrap_form/form_builder.rb' - 'lib/bootstrap_form/helpers/bootstrap.rb' -# Offense count: 1 -Style/CaseEquality: - Exclude: - - 'test/test_helper.rb' - # Offense count: 4 # Configuration parameters: MinBodyLength. Style/GuardClause: diff --git a/test/test_helper.rb b/test/test_helper.rb index b36e25486..21d50f667 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -61,26 +61,10 @@ def assert_equivalent_xml(expected, actual) actual_xml = Nokogiri::XML("\n#{actual}\n") { |config| config.default_xml.noblanks } ignored_attributes = %w[style data-disable-with] - equivalent = EquivalentXml.equivalent?(expected_xml, actual_xml, - ignore_attr_values: ignored_attributes) do |a, b, result| - if result === false && b.is_a?(Nokogiri::XML::Element) - if b.attr("name") == "utf8" - # Handle wrapped utf8 hidden field for Rails 4.2+ - result = EquivalentXml.equivalent?(a.child, b) - end - if b.delete("data-disable-with") - # Remove data-disable-with for Rails 5+ - # Workaround because ignoring in EquivalentXml doesn't work - result = EquivalentXml.equivalent?(a, b) - end - if a.attr("type") == "datetime" && b.attr("type") == "datetime-local" - a.delete("type") - b.delete("type") - # Handle new datetime type for Rails 5+ - result = EquivalentXml.equivalent?(a, b) - end - end - result + equivalent = EquivalentXml.equivalent?( + expected_xml, actual_xml, ignore_attr_values: ignored_attributes + ) do |a, b, result| + equivalent_xml?(a, b, result) end assert equivalent, lambda { @@ -91,4 +75,24 @@ def assert_equivalent_xml(expected, actual) ).to_s(:color) } end + + private + + def equivalent_xml?(before, after, result) + return result if result != false || !after.is_a?(Nokogiri::XML::Element) + + if after.attr("name") == "utf8" + # Handle wrapped utf8 hidden field for Rails 4.2+ + before = before.child + end + + after.delete("data-disable-with") + + if before.attr("type") == "datetime" && after.attr("type") == "datetime-local" + before.delete("type") + after.delete("type") + end + + EquivalentXml.equivalent?(before, after) + end end From ba6fbfb92b10b2ae367a44887b0d083990e6ad4f Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 14:50:19 +0000 Subject: [PATCH 06/10] clean up Rails/FilePath, Rails/OutputSafety and Style/GuardClause --- .rubocop_todo.yml | 19 ------- demo/config/environments/development.rb | 2 +- lib/bootstrap_form/form_builder.rb | 40 ++------------ lib/bootstrap_form/helpers/bootstrap.rb | 55 +++++++++---------- lib/bootstrap_form/inputs.rb | 1 + .../inputs/collection_check_boxes.rb | 1 + .../inputs/collection_radio_buttons.rb | 1 + .../inputs/inputs_collection.rb | 41 ++++++++++++++ 8 files changed, 78 insertions(+), 82 deletions(-) create mode 100644 lib/bootstrap_form/inputs/inputs_collection.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 58ec4e8ca..e4b126520 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -43,25 +43,6 @@ Naming/PredicateName: - 'spec/**/*' - 'lib/bootstrap_form/form_builder.rb' -# Offense count: 1 -# Configuration parameters: EnforcedStyle. -# SupportedStyles: slashes, arguments -Rails/FilePath: - Exclude: - - 'demo/config/environments/development.rb' - -# Offense count: 6 -Rails/OutputSafety: - Exclude: - - 'lib/bootstrap_form/form_builder.rb' - - 'lib/bootstrap_form/helpers/bootstrap.rb' - -# Offense count: 4 -# Configuration parameters: MinBodyLength. -Style/GuardClause: - Exclude: - - 'lib/bootstrap_form/helpers/bootstrap.rb' - # Offense count: 1 # Cop supports --auto-correct. Style/IfUnlessModifier: diff --git a/demo/config/environments/development.rb b/demo/config/environments/development.rb index 796409740..0883932fa 100644 --- a/demo/config/environments/development.rb +++ b/demo/config/environments/development.rb @@ -16,7 +16,7 @@ # Enable/disable caching. By default caching is disabled. # Run rails dev:cache to toggle caching. - if Rails.root.join("tmp/caching-dev.txt").exist? + if Rails.root.join("tmp", "caching-dev.txt").exist? config.action_controller.perform_caching = true config.cache_store = :memory_store diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 555a8d02d..66c93aa53 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -256,38 +256,6 @@ def get_error_messages(name) object.errors[name].join(", ") end - def inputs_collection(name, collection, value, text, options={}) - options[:inline] ||= layout_inline?(options[:layout]) - - form_group_builder(name, options) do - inputs = "" - - collection.each_with_index do |obj, i| - input_value = value.respond_to?(:call) ? value.call(obj) : obj.send(value) - input_options = form_group_collection_input_options(options, text, obj, i, input_value, collection) - inputs << yield(name, input_value, input_options) - end - - inputs.html_safe - end - end - - def form_group_collection_input_options(options, text, obj, index, input_value, collection) - input_options = options.merge(label: text.respond_to?(:call) ? text.call(obj) : obj.send(text)) - if (checked = input_options[:checked]) - input_options[:checked] = form_group_collection_input_checked?(checked, obj, input_value) - end - - input_options[:error_message] = index == collection.size - 1 - input_options.except!(:class) - input_options - end - - def form_group_collection_input_checked?(checked, obj, input_value) - checked == input_value || Array(checked).try(:include?, input_value) || - checked == obj || Array(checked).try(:include?, obj) - end - def get_help_text_by_i18n_key(name) return unless object @@ -313,14 +281,18 @@ def scoped_help_text(name, partial_scope) underscored_scope = "activerecord.help.#{partial_scope.underscore}" downcased_scope = "activerecord.help.#{partial_scope.downcase}" - help_text = I18n.t(name, scope: underscored_scope, default: "").html_safe.presence + help_text = translated_help_text(name, underscored_scope).presence - help_text ||= if (text = I18n.t(name, scope: downcased_scope, default: "").html_safe.presence) + help_text ||= if (text = translated_help_text(name, downcased_scope).presence) warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" text end help_text end + + def translated_help_text(name, scope) + ActiveSupport::SafeBuffer.new I18n.t(name, scope: scope, default: "") + end end end diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index da680fe74..f512c21c9 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -24,35 +24,34 @@ def primary(name=nil, options={}, &block) def alert_message(title, options={}) css = options[:class] || "alert alert-danger" + return unless object.respond_to?(:errors) && object.errors.full_messages.any? - if object.respond_to?(:errors) && object.errors.full_messages.any? - content_tag :div, class: css do - concat content_tag :p, title - concat error_summary unless options[:error_summary] == false - end + content_tag :div, class: css do + concat content_tag :p, title + concat error_summary unless options[:error_summary] == false end end def error_summary - if object.errors.any? - content_tag :ul, class: "rails-bootstrap-forms-error-summary" do - object.errors.full_messages.each do |error| - concat content_tag(:li, error) - end + return unless object.errors.any? + + content_tag :ul, class: "rails-bootstrap-forms-error-summary" do + object.errors.full_messages.each do |error| + concat content_tag(:li, error) end end end def errors_on(name, options={}) - if has_error?(name) - hide_attribute_name = options[:hide_attribute_name] || false - - content_tag :div, class: "alert alert-danger" do - if hide_attribute_name - object.errors[name].join(", ") - else - object.errors.full_messages_for(name).join(", ") - end + return unless has_error?(name) + + hide_attribute_name = options[:hide_attribute_name] || false + + content_tag :div, class: "alert alert-danger" do + if hide_attribute_name + object.errors[name].join(", ") + else + object.errors.full_messages_for(name).join(", ") end end end @@ -81,12 +80,12 @@ def custom_control(*args, &block) def prepend_and_append_input(name, options, &block) options = options.extract!(:prepend, :append, :input_group_class) - input = capture(&block) || "".html_safe + input = ActiveSupport::SafeBuffer.new(capture(&block) || "") input = prepend_input(options) + input + append_input(options) input += generate_error(name) options.present? && - input = content_tag(:div, input.html_safe, class: ["input-group", options[:input_group_class]].compact) + input = content_tag(:div, input, class: ["input-group", options[:input_group_class]].compact) input end @@ -109,21 +108,21 @@ def static_class def append_input(options) html = content_tag(:div, input_group_content(options[:append]), class: "input-group-append") if options[:append] - (html || "").html_safe + ActiveSupport::SafeBuffer.new(html || "") end def prepend_input(options) html = content_tag(:div, input_group_content(options[:prepend]), class: "input-group-prepend") if options[:prepend] - (html || "").html_safe + ActiveSupport::SafeBuffer.new(html || "") end def setup_css_class(the_class, options={}) - unless options.key? :class - if (extra_class = options.delete(:extra_class)) - the_class = "#{the_class} #{extra_class}" - end - options[:class] = the_class + return if options.key? :class + + if (extra_class = options.delete(:extra_class)) + the_class = "#{the_class} #{extra_class}" end + options[:class] = the_class end end end diff --git a/lib/bootstrap_form/inputs.rb b/lib/bootstrap_form/inputs.rb index 6e324029c..08211db3f 100644 --- a/lib/bootstrap_form/inputs.rb +++ b/lib/bootstrap_form/inputs.rb @@ -5,6 +5,7 @@ module Inputs extend ActiveSupport::Autoload autoload :Base + autoload :InputsCollection autoload :CheckBox autoload :CollectionCheckBoxes autoload :CollectionRadioButtons diff --git a/lib/bootstrap_form/inputs/collection_check_boxes.rb b/lib/bootstrap_form/inputs/collection_check_boxes.rb index 360c142dd..2298e0860 100644 --- a/lib/bootstrap_form/inputs/collection_check_boxes.rb +++ b/lib/bootstrap_form/inputs/collection_check_boxes.rb @@ -5,6 +5,7 @@ module Inputs module CollectionCheckBoxes extend ActiveSupport::Concern include Base + include InputsCollection included do def collection_check_boxes_with_bootstrap(*args) diff --git a/lib/bootstrap_form/inputs/collection_radio_buttons.rb b/lib/bootstrap_form/inputs/collection_radio_buttons.rb index fe9238015..be07db35e 100644 --- a/lib/bootstrap_form/inputs/collection_radio_buttons.rb +++ b/lib/bootstrap_form/inputs/collection_radio_buttons.rb @@ -5,6 +5,7 @@ module Inputs module CollectionRadioButtons extend ActiveSupport::Concern include Base + include InputsCollection included do def collection_radio_buttons_with_bootstrap(*args) diff --git a/lib/bootstrap_form/inputs/inputs_collection.rb b/lib/bootstrap_form/inputs/inputs_collection.rb new file mode 100644 index 000000000..b80219e9f --- /dev/null +++ b/lib/bootstrap_form/inputs/inputs_collection.rb @@ -0,0 +1,41 @@ +module BootstrapForm + module Inputs + module InputsCollection + extend ActiveSupport::Concern + + private + + def inputs_collection(name, collection, value, text, options={}) + options[:inline] ||= layout_inline?(options[:layout]) + + form_group_builder(name, options) do + inputs = ActiveSupport::SafeBuffer.new + + collection.each_with_index do |obj, i| + input_value = value.respond_to?(:call) ? value.call(obj) : obj.send(value) + input_options = form_group_collection_input_options(options, text, obj, i, input_value, collection) + inputs << yield(name, input_value, input_options) + end + + inputs + end + end + + def form_group_collection_input_options(options, text, obj, index, input_value, collection) + input_options = options.merge(label: text.respond_to?(:call) ? text.call(obj) : obj.send(text)) + if (checked = input_options[:checked]) + input_options[:checked] = form_group_collection_input_checked?(checked, obj, input_value) + end + + input_options[:error_message] = index == collection.size - 1 + input_options.except!(:class) + input_options + end + + def form_group_collection_input_checked?(checked, obj, input_value) + checked == input_value || Array(checked).try(:include?, input_value) || + checked == obj || Array(checked).try(:include?, obj) + end + end + end +end From 5048fe90e0898efe090a2c6f4c85d18649ae56c5 Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 15:05:50 +0000 Subject: [PATCH 07/10] Clean up Layout/IndentHeredoc Metrics/LineLength Metrics/ParameterLists Naming/PredicateName Style/IfUnlessModifier --- .rubocop_todo.yml | 36 ------------------- Dangerfile | 10 +++--- demo/config/application.rb | 5 ++- lib/bootstrap_form/form_builder.rb | 18 +++++----- lib/bootstrap_form/form_group_builder.rb | 2 +- lib/bootstrap_form/helpers/bootstrap.rb | 2 +- lib/bootstrap_form/inputs/check_box.rb | 2 +- .../inputs/collection_select.rb | 2 ++ .../inputs/grouped_collection_select.rb | 2 ++ .../inputs/inputs_collection.rb | 2 ++ lib/bootstrap_form/inputs/radio_button.rb | 2 +- test/bootstrap_other_components_test.rb | 8 +++-- 12 files changed, 32 insertions(+), 59 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e4b126520..bf559c519 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -8,43 +8,7 @@ inherit_from: .rubocop.yml # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: auto_detection, squiggly, active_support, powerpack, unindent -Layout/IndentHeredoc: - Exclude: - - 'Dangerfile' - # Offense count: 2 # Configuration parameters: CountComments. Metrics/ClassLength: Max: 244 - -# Offense count: 1 -# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 137 - -# Offense count: 3 -# Configuration parameters: CountKeywordArgs. -Metrics/ParameterLists: - Max: 8 - -# Offense count: 3 -# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist, MethodDefinitionMacros. -# NamePrefix: is_, has_, have_ -# NamePrefixBlacklist: is_, has_, have_ -# NameWhitelist: is_a? -# MethodDefinitionMacros: define_method, define_singleton_method -Naming/PredicateName: - Exclude: - - 'spec/**/*' - - 'lib/bootstrap_form/form_builder.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Style/IfUnlessModifier: - Exclude: - - 'demo/config/application.rb' diff --git a/Dangerfile b/Dangerfile index 5a33daa58..2ecc4157c 100644 --- a/Dangerfile +++ b/Dangerfile @@ -29,12 +29,12 @@ end # Have you updated CHANGELOG.md? # ------------------------------------------------------------------------------ if !has_changelog_changes && has_lib_changes - markdown <<-MARKDOWN -Here's an example of a CHANGELOG.md entry (place it immediately under the `* Your contribution here!` line): + markdown <<-MARKDOWN.strip_heredoc + Here's an example of a CHANGELOG.md entry (place it immediately under the `* Your contribution here!` line): -```markdown -* [##{pr_number}](#{pr_url}): #{github.pr_title} - [@#{github.pr_author}](https://github.com/#{github.pr_author}). -``` + ```markdown + * [##{pr_number}](#{pr_url}): #{github.pr_title} - [@#{github.pr_author}](https://github.com/#{github.pr_author}). + ``` MARKDOWN warn("Please update CHANGELOG.md with a description of your changes. "\ "If this PR is not a user-facing change (e.g. just refactoring), "\ diff --git a/demo/config/application.rb b/demo/config/application.rb index b548d5883..0120167b5 100644 --- a/demo/config/application.rb +++ b/demo/config/application.rb @@ -9,9 +9,8 @@ module Dummy class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. - if config.respond_to?(:load_defaults) - config.load_defaults [Rails::VERSION::MAJOR, Rails::VERSION::MINOR].join(".") - end + config.respond_to?(:load_defaults) && + config.load_defaults([Rails::VERSION::MAJOR, Rails::VERSION::MINOR].join(".")) config.secret_key_base = "ignore" if config.respond_to?(:secret_key_base) diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 66c93aa53..2c50a2274 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -141,7 +141,7 @@ def control_specific_class(method) "rails-bootstrap-forms-#{method.to_s.tr('_', '-')}" end - def has_error?(name) + def error?(name) object.respond_to?(:errors) && !(name.nil? || object.errors[name].empty?) end @@ -156,10 +156,10 @@ def required_attribute?(obj, attribute) [] end - has_presence_validator(target_validators) + presence_validator?(target_validators) end - def has_presence_validator(target_validators) + def presence_validator?(target_validators) has_presence_validator = target_validators.include?( ActiveModel::Validations::PresenceValidator ) @@ -206,7 +206,7 @@ def label_classes(name, options, custom_label_col, group_layout) classes << "required" if required_attribute?(object, name) end - classes << "text-danger" if label_errors && has_error?(name) + classes << "text-danger" if label_errors && error?(name) classes.flatten.compact end @@ -219,19 +219,19 @@ def label_layout_classes(custom_label_col, group_layout) end def label_text(name, options) - if label_errors && has_error?(name) + if label_errors && error?(name) (options[:text] || object.class.human_attribute_name(name)).to_s.concat(" #{get_error_messages(name)}") else options[:text] end end - def has_inline_error?(name) - has_error?(name) && inline_errors + def inline_error?(name) + error?(name) && inline_errors end def generate_error(name) - if has_inline_error?(name) + if inline_error?(name) help_text = get_error_messages(name) help_klass = "invalid-feedback" help_tag = :div @@ -243,7 +243,7 @@ def generate_error(name) end def generate_help(name, help_text) - return if help_text == false || has_inline_error?(name) + return if help_text == false || inline_error?(name) help_klass ||= "form-text text-muted" help_text ||= get_help_text_by_i18n_key(name) diff --git a/lib/bootstrap_form/form_group_builder.rb b/lib/bootstrap_form/form_group_builder.rb index 5ec92ec7b..da94961b9 100644 --- a/lib/bootstrap_form/form_group_builder.rb +++ b/lib/bootstrap_form/form_group_builder.rb @@ -83,7 +83,7 @@ def form_group_css_options(method, html_options, options) # Add control_class; allow it to be overridden by :control_class option control_classes = css_options.delete(:control_class) { control_class } css_options[:class] = [control_classes, css_options[:class]].compact.join(" ") - css_options[:class] << " is-invalid" if has_error?(method) + css_options[:class] << " is-invalid" if error?(method) css_options[:placeholder] = form_group_placeholder(options, method) if options[:label_as_placeholder] css_options end diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index f512c21c9..e3bf9ba75 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -43,7 +43,7 @@ def error_summary end def errors_on(name, options={}) - return unless has_error?(name) + return unless error?(name) hide_attribute_name = options[:hide_attribute_name] || false diff --git a/lib/bootstrap_form/inputs/check_box.rb b/lib/bootstrap_form/inputs/check_box.rb index 9a79b0e22..68e6466d8 100644 --- a/lib/bootstrap_form/inputs/check_box.rb +++ b/lib/bootstrap_form/inputs/check_box.rb @@ -52,7 +52,7 @@ def check_box_value(name, value) def check_box_classes(name, options) classes = [options[:class]] classes << (options[:custom] ? "custom-control-input" : "form-check-input") - classes << "is-invalid" if has_error?(name) + classes << "is-invalid" if error?(name) classes << "position-static" if options[:skip_label] || options[:hide_label] classes.flatten.compact end diff --git a/lib/bootstrap_form/inputs/collection_select.rb b/lib/bootstrap_form/inputs/collection_select.rb index 2d9bdb853..4d2a249f6 100644 --- a/lib/bootstrap_form/inputs/collection_select.rb +++ b/lib/bootstrap_form/inputs/collection_select.rb @@ -7,6 +7,7 @@ module CollectionSelect include Base included do + # rubocop:disable Metrics/ParameterLists def collection_select_with_bootstrap(method, collection, value_method, text_method, options={}, html_options={}) form_group_builder(method, options, html_options) do input_with_error(method) do @@ -14,6 +15,7 @@ def collection_select_with_bootstrap(method, collection, value_method, text_meth end end end + # rubocop:enable Metrics/ParameterLists bootstrap_alias :collection_select end diff --git a/lib/bootstrap_form/inputs/grouped_collection_select.rb b/lib/bootstrap_form/inputs/grouped_collection_select.rb index ab4f5b77e..e7ac8141d 100644 --- a/lib/bootstrap_form/inputs/grouped_collection_select.rb +++ b/lib/bootstrap_form/inputs/grouped_collection_select.rb @@ -7,6 +7,7 @@ module GroupedCollectionSelect include Base included do + # rubocop:disable Metrics/ParameterLists def grouped_collection_select_with_bootstrap(method, collection, group_method, group_label_method, option_key_method, option_value_method, options={}, html_options={}) @@ -18,6 +19,7 @@ def grouped_collection_select_with_bootstrap(method, collection, group_method, end end end + # rubocop:enable Metrics/ParameterLists bootstrap_alias :grouped_collection_select end diff --git a/lib/bootstrap_form/inputs/inputs_collection.rb b/lib/bootstrap_form/inputs/inputs_collection.rb index b80219e9f..617d64d36 100644 --- a/lib/bootstrap_form/inputs/inputs_collection.rb +++ b/lib/bootstrap_form/inputs/inputs_collection.rb @@ -21,6 +21,7 @@ def inputs_collection(name, collection, value, text, options={}) end end + # rubocop:disable Metrics/ParameterLists def form_group_collection_input_options(options, text, obj, index, input_value, collection) input_options = options.merge(label: text.respond_to?(:call) ? text.call(obj) : obj.send(text)) if (checked = input_options[:checked]) @@ -31,6 +32,7 @@ def form_group_collection_input_options(options, text, obj, index, input_value, input_options.except!(:class) input_options end + # rubocop:enable Metrics/ParameterLists def form_group_collection_input_checked?(checked, obj, input_value) checked == input_value || Array(checked).try(:include?, input_value) || diff --git a/lib/bootstrap_form/inputs/radio_button.rb b/lib/bootstrap_form/inputs/radio_button.rb index 83ea53db8..a3bad9a08 100644 --- a/lib/bootstrap_form/inputs/radio_button.rb +++ b/lib/bootstrap_form/inputs/radio_button.rb @@ -36,7 +36,7 @@ def radio_button_label(name, value, options) def radio_button_classes(name, options) classes = [options[:class]] classes << (options[:custom] ? "custom-control-input" : "form-check-input") - classes << "is-invalid" if has_error?(name) + classes << "is-invalid" if error?(name) classes << "position-static" if options[:skip_label] || options[:hide_label] classes.flatten.compact end diff --git a/test/bootstrap_other_components_test.rb b/test/bootstrap_other_components_test.rb index f5cb82b74..06a6ce3f9 100644 --- a/test/bootstrap_other_components_test.rb +++ b/test/bootstrap_other_components_test.rb @@ -132,13 +132,17 @@ class BootstrapOtherComponentsTest < ActionView::TestCase end test "regular button uses proper css classes" do - expected = %q() + expected = <<-HTML.strip_heredoc + + HTML assert_equivalent_xml expected, @builder.button("I'm HTML! in a button!".html_safe) end test "regular button can have extra css classes" do - expected = %q() + expected = <<-HTML.strip_heredoc + + HTML assert_equivalent_xml expected, @builder.button("I'm HTML! in a button!".html_safe, extra_class: "test-button") end From 445abb33d031d73a484ec1ce46baa4011d8aee1c Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Tue, 19 Feb 2019 15:47:08 +0000 Subject: [PATCH 08/10] Refactor and clean up .rubocop_todo.yml --- .rubocop_todo.yml | 6 +- lib/bootstrap_form.rb | 2 + lib/bootstrap_form/components.rb | 17 ++ lib/bootstrap_form/components/hints.rb | 60 +++++++ lib/bootstrap_form/components/labels.rb | 56 ++++++ lib/bootstrap_form/components/layout.rb | 39 +++++ lib/bootstrap_form/components/validation.rb | 63 +++++++ lib/bootstrap_form/form_builder.rb | 183 +------------------- lib/bootstrap_form/form_group_builder.rb | 8 + 9 files changed, 249 insertions(+), 185 deletions(-) create mode 100644 lib/bootstrap_form/components.rb create mode 100644 lib/bootstrap_form/components/hints.rb create mode 100644 lib/bootstrap_form/components/labels.rb create mode 100644 lib/bootstrap_form/components/layout.rb create mode 100644 lib/bootstrap_form/components/validation.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bf559c519..3466f57fb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -2,13 +2,13 @@ inherit_from: .rubocop.yml # This configuration was generated by # `rubocop --auto-gen-config` -# on 2019-02-19 14:32:34 +0000 using RuboCop version 0.64.0. +# on 2019-02-19 15:47:19 +0000 using RuboCop version 0.64.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 2 +# Offense count: 1 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 244 + Max: 111 diff --git a/lib/bootstrap_form.rb b/lib/bootstrap_form.rb index 18efcacae..84b948c58 100644 --- a/lib/bootstrap_form.rb +++ b/lib/bootstrap_form.rb @@ -16,12 +16,14 @@ module BootstrapForm autoload :FormBuilder autoload :FormGroupBuilder autoload :FormGroup + autoload :Components autoload :Inputs autoload :Helpers end def self.eager_load! super + BootstrapForm::Components.eager_load! BootstrapForm::Helpers.eager_load! BootstrapForm::Inputs.eager_load! end diff --git a/lib/bootstrap_form/components.rb b/lib/bootstrap_form/components.rb new file mode 100644 index 000000000..4d86db638 --- /dev/null +++ b/lib/bootstrap_form/components.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module BootstrapForm + module Components + extend ActiveSupport::Autoload + + autoload :Hints + autoload :Labels + autoload :Layout + autoload :Validation + + include Hints + include Labels + include Layout + include Validation + end +end diff --git a/lib/bootstrap_form/components/hints.rb b/lib/bootstrap_form/components/hints.rb new file mode 100644 index 000000000..93d10aac6 --- /dev/null +++ b/lib/bootstrap_form/components/hints.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module BootstrapForm + module Components + module Hints + extend ActiveSupport::Concern + + private + + def generate_help(name, help_text) + return if help_text == false || inline_error?(name) + + help_klass ||= "form-text text-muted" + help_text ||= get_help_text_by_i18n_key(name) + help_tag ||= :small + + content_tag(help_tag, help_text, class: help_klass) if help_text.present? + end + + def get_help_text_by_i18n_key(name) + return unless object + + partial_scope = if object.class.respond_to?(:model_name) + object.class.model_name.name + else + object.class.name + end + + # First check for a subkey :html, as it is also accepted by i18n, and the + # simple check for name would return an hash instead of a string (both + # with .presence returning true!) + help_text = nil + ["#{name}.html", name, "#{name}_html"].each do |scope| + break if help_text + + help_text = scoped_help_text(scope, partial_scope) + end + help_text + end + + def scoped_help_text(name, partial_scope) + underscored_scope = "activerecord.help.#{partial_scope.underscore}" + downcased_scope = "activerecord.help.#{partial_scope.downcase}" + + help_text = translated_help_text(name, underscored_scope).presence + + help_text ||= if (text = translated_help_text(name, downcased_scope).presence) + warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" + text + end + + help_text + end + + def translated_help_text(name, scope) + ActiveSupport::SafeBuffer.new I18n.t(name, scope: scope, default: "") + end + end + end +end diff --git a/lib/bootstrap_form/components/labels.rb b/lib/bootstrap_form/components/labels.rb new file mode 100644 index 000000000..dd213600f --- /dev/null +++ b/lib/bootstrap_form/components/labels.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module BootstrapForm + module Components + module Labels + extend ActiveSupport::Concern + + private + + def generate_label(id, name, options, custom_label_col, group_layout) + return if options.blank? + + # 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 + + options[:class] = label_classes(name, options, custom_label_col, group_layout) + options.delete(:class) if options[:class].none? + + label(name, label_text(name, options), options.except(:text)) + end + + def label_classes(name, options, custom_label_col, group_layout) + classes = [options[:class], label_layout_classes(custom_label_col, group_layout)] + + case options.delete(:required) + when true + classes << "required" + when nil, :default + classes << "required" if required_attribute?(object, name) + end + + classes << "text-danger" if label_errors && error?(name) + classes.flatten.compact + end + + def label_layout_classes(custom_label_col, group_layout) + if layout_horizontal?(group_layout) + ["col-form-label", (custom_label_col || label_col)] + elsif layout_inline?(group_layout) + ["mr-sm-2"] + end + end + + 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)}") + else + options[:text] + end + end + end + end +end diff --git a/lib/bootstrap_form/components/layout.rb b/lib/bootstrap_form/components/layout.rb new file mode 100644 index 000000000..2a58007cc --- /dev/null +++ b/lib/bootstrap_form/components/layout.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module BootstrapForm + module Components + module Layout + extend ActiveSupport::Concern + + private + + def layout_default?(field_layout=nil) + [:default, nil].include? layout_in_effect(field_layout) + end + + def layout_horizontal?(field_layout=nil) + layout_in_effect(field_layout) == :horizontal + end + + def layout_inline?(field_layout=nil) + layout_in_effect(field_layout) == :inline + end + + def field_inline_override?(field_layout=nil) + field_layout == :inline && layout != :inline + end + + # true and false should only come from check_box and radio_button, + # and those don't have a :horizontal layout + def layout_in_effect(field_layout) + field_layout = :inline if field_layout == true + field_layout = :default if field_layout == false + field_layout || layout + end + + def get_group_layout(group_layout) + group_layout || layout + end + end + end +end diff --git a/lib/bootstrap_form/components/validation.rb b/lib/bootstrap_form/components/validation.rb new file mode 100644 index 000000000..dbf13ec19 --- /dev/null +++ b/lib/bootstrap_form/components/validation.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module BootstrapForm + module Components + module Validation + extend ActiveSupport::Concern + + private + + def error?(name) + object.respond_to?(:errors) && !(name.nil? || object.errors[name].empty?) + end + + def required_attribute?(obj, attribute) + return false unless obj && attribute + + target = obj.class == Class ? obj : obj.class + + target_validators = if target.respond_to? :validators_on + target.validators_on(attribute).map(&:class) + else + [] + end + + presence_validator?(target_validators) + end + + def presence_validator?(target_validators) + has_presence_validator = target_validators.include?( + ActiveModel::Validations::PresenceValidator + ) + + if defined? ActiveRecord::Validations::PresenceValidator + has_presence_validator |= target_validators.include?( + ActiveRecord::Validations::PresenceValidator + ) + end + + has_presence_validator + end + + def inline_error?(name) + error?(name) && inline_errors + end + + def generate_error(name) + if inline_error?(name) + help_text = get_error_messages(name) + help_klass = "invalid-feedback" + help_tag = :div + + content_tag(help_tag, help_text, class: help_klass) + else + "" + end + end + + def get_error_messages(name) + object.errors[name].join(", ") + end + end + end +end diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb index 2c50a2274..1f983b697 100644 --- a/lib/bootstrap_form/form_builder.rb +++ b/lib/bootstrap_form/form_builder.rb @@ -9,6 +9,7 @@ class FormBuilder < ActionView::Helpers::FormBuilder include BootstrapForm::FormGroupBuilder include BootstrapForm::FormGroup + include BootstrapForm::Components include BootstrapForm::Inputs::Base include BootstrapForm::Inputs::CheckBox @@ -85,34 +86,6 @@ def fields_for_options(record_object, fields_options) field_options end - def layout_default?(field_layout=nil) - [:default, nil].include? layout_in_effect(field_layout) - end - - def layout_horizontal?(field_layout=nil) - layout_in_effect(field_layout) == :horizontal - end - - def layout_inline?(field_layout=nil) - layout_in_effect(field_layout) == :inline - end - - def field_inline_override?(field_layout=nil) - field_layout == :inline && layout != :inline - end - - # true and false should only come from check_box and radio_button, - # and those don't have a :horizontal layout - def layout_in_effect(field_layout) - field_layout = :inline if field_layout == true - field_layout = :default if field_layout == false - field_layout || layout - end - - def get_group_layout(group_layout) - group_layout || layout - end - def default_label_col "col-sm-2" end @@ -140,159 +113,5 @@ def feedback_class def control_specific_class(method) "rails-bootstrap-forms-#{method.to_s.tr('_', '-')}" end - - def error?(name) - object.respond_to?(:errors) && !(name.nil? || object.errors[name].empty?) - end - - def required_attribute?(obj, attribute) - return false unless obj && attribute - - target = obj.class == Class ? obj : obj.class - - target_validators = if target.respond_to? :validators_on - target.validators_on(attribute).map(&:class) - else - [] - end - - presence_validator?(target_validators) - end - - def presence_validator?(target_validators) - has_presence_validator = target_validators.include?( - ActiveModel::Validations::PresenceValidator - ) - - if defined? ActiveRecord::Validations::PresenceValidator - has_presence_validator |= target_validators.include?( - ActiveRecord::Validations::PresenceValidator - ) - end - - has_presence_validator - end - - def convert_form_tag_options(method, options={}) - 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) - return if options.blank? - - # 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 - - options[:class] = label_classes(name, options, custom_label_col, group_layout) - options.delete(:class) if options[:class].none? - - label(name, label_text(name, options), options.except(:text)) - end - - def label_classes(name, options, custom_label_col, group_layout) - classes = [options[:class], label_layout_classes(custom_label_col, group_layout)] - - case options.delete(:required) - when true - classes << "required" - when nil, :default - classes << "required" if required_attribute?(object, name) - end - - classes << "text-danger" if label_errors && error?(name) - classes.flatten.compact - end - - def label_layout_classes(custom_label_col, group_layout) - if layout_horizontal?(group_layout) - ["col-form-label", (custom_label_col || label_col)] - elsif layout_inline?(group_layout) - ["mr-sm-2"] - end - end - - 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)}") - else - options[:text] - end - end - - def inline_error?(name) - error?(name) && inline_errors - end - - def generate_error(name) - if inline_error?(name) - help_text = get_error_messages(name) - help_klass = "invalid-feedback" - help_tag = :div - - content_tag(help_tag, help_text, class: help_klass) - else - "" - end - end - - def generate_help(name, help_text) - return if help_text == false || inline_error?(name) - - help_klass ||= "form-text text-muted" - help_text ||= get_help_text_by_i18n_key(name) - help_tag ||= :small - - content_tag(help_tag, help_text, class: help_klass) if help_text.present? - end - - def get_error_messages(name) - object.errors[name].join(", ") - end - - def get_help_text_by_i18n_key(name) - return unless object - - partial_scope = if object.class.respond_to?(:model_name) - object.class.model_name.name - else - object.class.name - end - - # First check for a subkey :html, as it is also accepted by i18n, and the - # simple check for name would return an hash instead of a string (both - # with .presence returning true!) - help_text = nil - ["#{name}.html", name, "#{name}_html"].each do |scope| - break if help_text - - help_text = scoped_help_text(scope, partial_scope) - end - help_text - end - - def scoped_help_text(name, partial_scope) - underscored_scope = "activerecord.help.#{partial_scope.underscore}" - downcased_scope = "activerecord.help.#{partial_scope.downcase}" - - help_text = translated_help_text(name, underscored_scope).presence - - help_text ||= if (text = translated_help_text(name, downcased_scope).presence) - warn "I18n key '#{downcased_scope}.#{name}' is deprecated, use '#{underscored_scope}.#{name}' instead" - text - end - - help_text - end - - def translated_help_text(name, scope) - ActiveSupport::SafeBuffer.new I18n.t(name, scope: scope, default: "") - end end end diff --git a/lib/bootstrap_form/form_group_builder.rb b/lib/bootstrap_form/form_group_builder.rb index da94961b9..4407266c0 100644 --- a/lib/bootstrap_form/form_group_builder.rb +++ b/lib/bootstrap_form/form_group_builder.rb @@ -34,6 +34,14 @@ def form_group_builder_options(options, method) options end + def convert_form_tag_options(method, options={}) + unless @options[:skip_default_ids] + options[:name] ||= method + options[:id] ||= method + end + options + end + def form_group_opts(options, css_options) wrapper_options = options[:wrapper] form_group_options = { From b55734324490a0a1db65398ef0ec284683a0b65c Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Wed, 20 Feb 2019 08:23:25 +0000 Subject: [PATCH 09/10] Rubocop refactoring and remove .rubocop_todo.yml --- .rubocop_todo.yml | 14 -- Rakefile | 6 +- bootstrap_deferred_builder_test.rb | 15 --- bootstrap_error_rendering_test.rb | 141 -------------------- lib/bootstrap_form/components/validation.rb | 16 +-- test/test_helper.rb | 18 +-- 6 files changed, 18 insertions(+), 192 deletions(-) delete mode 100644 .rubocop_todo.yml delete mode 100644 bootstrap_deferred_builder_test.rb delete mode 100644 bootstrap_error_rendering_test.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml deleted file mode 100644 index 3466f57fb..000000000 --- a/.rubocop_todo.yml +++ /dev/null @@ -1,14 +0,0 @@ -inherit_from: .rubocop.yml - -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2019-02-19 15:47:19 +0000 using RuboCop version 0.64.0. -# The point is for the user to remove these configuration records -# one by one as the offenses are removed from the code base. -# Note that changes in the inspected code, or installation of new -# versions of RuboCop, may require this file to be generated again. - -# Offense count: 1 -# Configuration parameters: CountComments. -Metrics/ClassLength: - Max: 111 diff --git a/Rakefile b/Rakefile index 90e3ba3c3..4971c766e 100644 --- a/Rakefile +++ b/Rakefile @@ -31,9 +31,7 @@ task "release:rubygem_push" do Rake.application.invoke_task("chandler:push") end -desc 'Run RuboCop checks -- see .rubocop_todo.yml for outstanding offenses' -RuboCop::RakeTask.new(:rubocop) do |task| - task.options = %w[--config .rubocop_todo.yml] -end +desc 'Run RuboCop checks' +RuboCop::RakeTask.new(:rubocop) task default: %i[test rubocop] diff --git a/bootstrap_deferred_builder_test.rb b/bootstrap_deferred_builder_test.rb deleted file mode 100644 index 21c337393..000000000 --- a/bootstrap_deferred_builder_test.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -require_relative "./test_helper" - -class BootstrapDeferredBuilderTest < ActionView::TestCase - include BootstrapForm::ActionViewExtensions::FormHelper - - setup :setup_test_fixture - - test "create a deferred builder for a radio button" do - assert what_we_got = BootstrapForm::DeferredBuilder::RadioButton.new(:email, @builder) - assert what_we_got.is_a?(BootstrapForm::DeferredBuilder::RadioButton) - assert_equal '
', what_we_got.to_s - end -end diff --git a/bootstrap_error_rendering_test.rb b/bootstrap_error_rendering_test.rb deleted file mode 100644 index 12f8f1147..000000000 --- a/bootstrap_error_rendering_test.rb +++ /dev/null @@ -1,141 +0,0 @@ -# frozen_string_literal: true - -require "test_helper" - -class BootstrapErrorRenderingTest < ActionView::TestCase - include BootstrapForm::ActionViewExtensions::FormHelper - - def setup - setup_test_fixture - end - - test "Default error message" do - @user.email = nil - @user.valid? - - expected = <<-HTML -
- -
- - -
can't be blank, is too short (minimum is 5 characters)
-
-
- HTML - - assert_equivalent_xml expected, bootstrap_form_for(@user) { |f| f.text_field :email } - end - - test "form_group error message" do - @user.email = nil - @user.valid? - - output = @builder.form_group :email do - '

Bar

'.html_safe - end - - expected = <<-HTML -
-

Bar

-
can't be blank, is too short (minimum is 5 characters)
-
- HTML - - assert_equivalent_xml expected, output - end - - test "check_box wrapped in form_group error message" do - @user.errors.add :terms, "an error for testing" - - output = bootstrap_form_for(@user) do |f| - f.form_group(:terms) do - f.check_box(:terms, label: "I agree to the terms") - end - end - - expected = <<-HTML -
- -
-
- - - -
an error for testing
-
-
-
- HTML - - assert_equivalent_xml expected, output - end - - test "radio_button wrapped in form_group error message" do - @user.errors.add :misc, "an error for testing" - - output = bootstrap_form_for(@user) do |f| - f.form_group(:misc) do - f.radio_button(:misc, "1", label: "This is a radio button") - end - end - - expected = <<-HTML -
- -
-
- - -
an error for testing
-
-
-
- HTML - - assert_equivalent_xml expected, output - end - - test "bare check_box error message" do - @user.errors.add :terms, "an error for testing" - - output = bootstrap_form_for(@user) do |f| - f.check_box(:terms, label: "I agree to the terms") - end - - expected = <<-HTML -
- -
- - - -
an error for testing
-
-
- HTML - - assert_equivalent_xml expected, output - end - - test "bare radio_button error message" do - @user.errors.add :misc, "an error for testing" - - output = bootstrap_form_for(@user) do |f| - f.radio_button(:misc, "1", label: "This is a radio button") - end - - expected = <<-HTML -
- -
- - -
an error for testing
-
-
- HTML - - assert_equivalent_xml expected, output - end -end diff --git a/lib/bootstrap_form/components/validation.rb b/lib/bootstrap_form/components/validation.rb index dbf13ec19..1c7facfc4 100644 --- a/lib/bootstrap_form/components/validation.rb +++ b/lib/bootstrap_form/components/validation.rb @@ -44,15 +44,13 @@ def inline_error?(name) end def generate_error(name) - if inline_error?(name) - help_text = get_error_messages(name) - help_klass = "invalid-feedback" - help_tag = :div - - content_tag(help_tag, help_text, class: help_klass) - else - "" - end + return unless inline_error?(name) + + help_text = get_error_messages(name) + help_klass = "invalid-feedback" + help_tag = :div + + content_tag(help_tag, help_text, class: help_klass) end def get_error_messages(name) diff --git a/test/test_helper.rb b/test/test_helper.rb index 21d50f667..846467fee 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -78,21 +78,21 @@ def assert_equivalent_xml(expected, actual) private - def equivalent_xml?(before, after, result) - return result if result != false || !after.is_a?(Nokogiri::XML::Element) + def equivalent_xml?(expected, real, result) + return result if result != false || !real.is_a?(Nokogiri::XML::Element) - if after.attr("name") == "utf8" + if real.attr("name") == "utf8" # Handle wrapped utf8 hidden field for Rails 4.2+ - before = before.child + expected = expected.child end - after.delete("data-disable-with") + real.delete("data-disable-with") - if before.attr("type") == "datetime" && after.attr("type") == "datetime-local" - before.delete("type") - after.delete("type") + if expected.attr("type") == "datetime" && real.attr("type") == "datetime-local" + expected.delete("type") + real.delete("type") end - EquivalentXml.equivalent?(before, after) + EquivalentXml.equivalent?(expected, real) end end From 3df32823752a8ea680c40edbcc05983214d9c44e Mon Sep 17 00:00:00 2001 From: Tomislav Simnett Date: Thu, 21 Feb 2019 11:54:32 +0000 Subject: [PATCH 10/10] Feedback updates --- lib/bootstrap_form/helpers/bootstrap.rb | 6 +++--- lib/bootstrap_form/inputs/collection_select.rb | 1 + lib/bootstrap_form/inputs/file_field.rb | 4 ++-- lib/bootstrap_form/inputs/grouped_collection_select.rb | 1 + lib/bootstrap_form/inputs/inputs_collection.rb | 1 + 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/bootstrap_form/helpers/bootstrap.rb b/lib/bootstrap_form/helpers/bootstrap.rb index e3bf9ba75..df2ce9a40 100644 --- a/lib/bootstrap_form/helpers/bootstrap.rb +++ b/lib/bootstrap_form/helpers/bootstrap.rb @@ -80,7 +80,7 @@ def custom_control(*args, &block) def prepend_and_append_input(name, options, &block) options = options.extract!(:prepend, :append, :input_group_class) - input = ActiveSupport::SafeBuffer.new(capture(&block) || "") + input = capture(&block) || ActiveSupport::SafeBuffer.new input = prepend_input(options) + input + append_input(options) input += generate_error(name) @@ -108,12 +108,12 @@ def static_class def append_input(options) html = content_tag(:div, input_group_content(options[:append]), class: "input-group-append") if options[:append] - ActiveSupport::SafeBuffer.new(html || "") + html || ActiveSupport::SafeBuffer.new end def prepend_input(options) html = content_tag(:div, input_group_content(options[:prepend]), class: "input-group-prepend") if options[:prepend] - ActiveSupport::SafeBuffer.new(html || "") + html || ActiveSupport::SafeBuffer.new end def setup_css_class(the_class, options={}) diff --git a/lib/bootstrap_form/inputs/collection_select.rb b/lib/bootstrap_form/inputs/collection_select.rb index 4d2a249f6..b5ac1472c 100644 --- a/lib/bootstrap_form/inputs/collection_select.rb +++ b/lib/bootstrap_form/inputs/collection_select.rb @@ -7,6 +7,7 @@ module CollectionSelect include Base included do + # Disabling Metrics/ParameterLists because the upstream Rails method has the same parameters # rubocop:disable Metrics/ParameterLists def collection_select_with_bootstrap(method, collection, value_method, text_method, options={}, html_options={}) form_group_builder(method, options, html_options) do diff --git a/lib/bootstrap_form/inputs/file_field.rb b/lib/bootstrap_form/inputs/file_field.rb index 8e5dea119..67d0c73b5 100644 --- a/lib/bootstrap_form/inputs/file_field.rb +++ b/lib/bootstrap_form/inputs/file_field.rb @@ -12,7 +12,7 @@ def file_field_with_bootstrap(name, options={}) form_group_builder(name, options) do content_tag(:div, class: "custom-file") do input_with_error(name) do - file_filed_input(name, options) + file_field_input(name, options) end end end @@ -23,7 +23,7 @@ def file_field_with_bootstrap(name, options={}) private - def file_filed_input(name, options) + def file_field_input(name, options) placeholder = options.delete(:placeholder) || "Choose file" placeholder_opts = { class: "custom-file-label" } placeholder_opts[:for] = options[:id] if options[:id].present? diff --git a/lib/bootstrap_form/inputs/grouped_collection_select.rb b/lib/bootstrap_form/inputs/grouped_collection_select.rb index e7ac8141d..7ed37d347 100644 --- a/lib/bootstrap_form/inputs/grouped_collection_select.rb +++ b/lib/bootstrap_form/inputs/grouped_collection_select.rb @@ -7,6 +7,7 @@ module GroupedCollectionSelect include Base included do + # Disabling Metrics/ParameterLists because the upstream Rails method has the same parameters # rubocop:disable Metrics/ParameterLists def grouped_collection_select_with_bootstrap(method, collection, group_method, group_label_method, option_key_method, diff --git a/lib/bootstrap_form/inputs/inputs_collection.rb b/lib/bootstrap_form/inputs/inputs_collection.rb index 617d64d36..e6fc4c637 100644 --- a/lib/bootstrap_form/inputs/inputs_collection.rb +++ b/lib/bootstrap_form/inputs/inputs_collection.rb @@ -21,6 +21,7 @@ def inputs_collection(name, collection, value, text, options={}) end end + # FIXME: Find a way to reduce the parameter list size # rubocop:disable Metrics/ParameterLists def form_group_collection_input_options(options, text, obj, index, input_value, collection) input_options = options.merge(label: text.respond_to?(:call) ? text.call(obj) : obj.send(text))