Skip to content

Commit

Permalink
Skip valid_class check if no class defined
Browse files Browse the repository at this point in the history
#1656

Previously, we would check for validity all the time, even if the outcome of the check added no class. This meant that there was no way to opt out of the check in cases where it was expensive (if the file is stored in S3, in our example).

Now, we only check for validity if there is a valid_class defined to be added if the field is valid.
  • Loading branch information
TALlama committed Apr 23, 2019
1 parent 4dd9261 commit c825741
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
11 changes: 8 additions & 3 deletions lib/simple_form/wrappers/root.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,16 @@ def html_classes(input, options)
css += SimpleForm.additional_classes_for(:wrapper) do
input.additional_classes + [input.input_class]
end
css << (options[:wrapper_error_class] || @defaults[:error_class]) if input.has_errors?
css << (options[:wrapper_hint_class] || @defaults[:hint_class]) if input.has_hint?
css << (options[:wrapper_valid_class] || @defaults[:valid_class]) if input.valid?
css << html_class(:error_class, options) { input.has_errors? }
css << html_class(:hint_class, options) { input.has_hint? }
css << html_class(:valid_class, options) { input.valid? }
css.compact
end

def html_class(key, options)
css = (options[:"wrapper_#{key}"] || @defaults[key])
css if css && yield
end
end
end
end
9 changes: 9 additions & 0 deletions test/form_builder/wrapper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ class WrapperTest < ActionView::TestCase
assert_no_select 'input.is-invalid'
end

test 'wrapper does not determine if valid class is needed when it is set to nil' do
def @user.name; raise BOOM; end
with_form_for @user, :name, as: :file, wrapper: custom_wrapper_with_input_valid_class(valid_class: nil)
assert_select 'div'
assert_select 'input'
assert_no_select 'div.field_with_errors'
assert_no_select 'input.is-invalid'
end

test 'wrapper adds hint class for attribute with a hint' do
with_form_for @user, :name, hint: 'hint'
assert_select 'div.field_with_hint'
Expand Down
4 changes: 2 additions & 2 deletions test/support/misc_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ def custom_wrapper_with_input_error_class
end
end

def custom_wrapper_with_input_valid_class
SimpleForm.build tag: :div, class: "custom_wrapper", valid_class: :field_without_errors do |b|
def custom_wrapper_with_input_valid_class(valid_class: :field_without_errors)
SimpleForm.build tag: :div, class: "custom_wrapper", valid_class: valid_class do |b|
b.use :label
b.use :input, class: 'inline-class', valid_class: 'is-valid'
end
Expand Down

0 comments on commit c825741

Please sign in to comment.