Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

allows custom field_error_proc #899

Merged
merged 3 commits into from

4 participants

@tokland

As per request at #896

@tokland

It says that the build failed but it's unrelated to this commit: "Bundler could not find compatible versions for gem "bundler".

@justinfrench

Looks great! @sobrinho any thoughts?

@sobrinho
Collaborator

Looks great for me! :)

@sobrinho
Collaborator

@tokland can you update the changelog?

@tokland

Is the CHANGELOG ok?

@justinfrench

Merging, will revert if the build still fails and can't be easily resolved.

@justinfrench justinfrench merged commit 9a41fec into justinfrench:master
@bughit

this overrides the default for rails built-in form helpers, is that the intent?

@tokland

@bughit Note that the ensure restores the old value (the patch does not change that behavior, it was already there)

@bughit

@@field_error_proc is the name used by ActionView::Base

the inclusion of Formtastic::Helpers::FormHelper clobbers it, the original rails value is lost

@bughit

according to #39, the intent is not to alter rails native behavior, but this change does, no?

@tokland

Oops, you're right, I wasn't aware that this module was included in ActionView::Base, we should use a different class variable name.

@bughit

using the following work around (in an initializer):

module Formtastic::Helpers::FormHelper

  @@_field_error_proc = @@field_error_proc
  remove_class_variable :@@field_error_proc

protected

  def with_custom_field_error_proc(&block)
    default_field_error_proc = ::ActionView::Base.field_error_proc
    ::ActionView::Base.field_error_proc = @@_field_error_proc
    yield
  ensure
    ::ActionView::Base.field_error_proc = default_field_error_proc
  end

end
@justinfrench

@bughit @tokland It's great that there's a work-around, but I'd love to see either a patch for this or a roll-back of the feature, as Formtastic was deliberately designed so that it shouldn't have any impact on forms that aren't wrapped in a semantic_form_for block, and this is a regression. At the very least, could someone start or re-open an issue with everything you know so far?

@tokland

I'll prepare a patch tomorrow, it's just changing the name of the field and adding an spec. I don't think I have rights to reopen issues, can't you do it?

@justinfrench

I've re-opened #39, but at this point a new issue is probably more relevant, anyway, looking forward to your patch, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
4 CHANGELOG
@@ -1,3 +1,7 @@
+HEAD
+
+* Allow to use a custom ActionView::Base.field_error_proc
+
2.2.1
* Rails 4 compatibilty updates
View
1  formtastic.gemspec
@@ -36,4 +36,5 @@ Gem::Specification.new do |s|
s.add_development_dependency(%q<ammeter>, ["0.2.5"])
s.add_development_dependency(%q<appraisal>)
s.add_development_dependency(%q<rake>)
+ s.add_development_dependency(%q<ruby-debug19>)
end
View
16 lib/formtastic/helpers/form_helper.rb
@@ -58,6 +58,12 @@ module FormHelper
@@default_form_class = 'formtastic'
mattr_accessor :default_form_class
+ # Allows to set a custom field_error_proc wrapper. By default this wrapper
+ # is disabled since `formtastic` already adds an error class to the LI tag
+ # containing the input. Change this from `config/initializers/formtastic.rb`.
+ @@field_error_proc = proc { |html_tag, instance_tag| html_tag }
+ mattr_accessor :field_error_proc
+
# Wrapper around Rails' own `form_for` helper to set the `:builder` option to
# `Formtastic::FormBuilder` and to set some class names on the `<form>` tag such as
# `formtastic` and the downcased and underscored model name (eg `post`).
@@ -178,17 +184,9 @@ def semantic_fields_for(record_name, record_object = nil, options = {}, &block)
protected
- # Override the default ActiveRecordHelper behaviour of wrapping the input.
- # This gets taken care of semantically by adding an error class to the LI tag
- # containing the input.
- # @private
- FIELD_ERROR_PROC = proc do |html_tag, instance_tag|
- html_tag
- end
-
def with_custom_field_error_proc(&block)
default_field_error_proc = ::ActionView::Base.field_error_proc
- ::ActionView::Base.field_error_proc = FIELD_ERROR_PROC
+ ::ActionView::Base.field_error_proc = @@field_error_proc
yield
ensure
::ActionView::Base.field_error_proc = default_field_error_proc
View
23 spec/helpers/form_helper_spec.rb
@@ -135,6 +135,29 @@
end
end
+ describe ActionView::Base.field_error_proc do
+ it 'is set to no-op wrapper by default' do
+ semantic_form_for(@new_post, :url => '/hello') do |builder|
+ ::ActionView::Base.field_error_proc.call("html", nil).should == "html"
+ end
+ end
+
+ it 'is set to the configured custom field_error_proc' do
+ field_error_proc = mock()
+ Formtastic::Helpers::FormHelper.field_error_proc = field_error_proc
+ semantic_form_for(@new_post, :url => '/hello') do |builder|
+ ::ActionView::Base.field_error_proc.should == field_error_proc
+ end
+ end
+
+ it 'is restored to its original value after the form is rendered' do
+ lambda do
+ Formtastic::Helpers::FormHelper.field_error_proc = proc {}
+ semantic_form_for(@new_post, :url => '/hello') { |builder| }
+ end.should_not change(::ActionView::Base, :field_error_proc)
+ end
+ end
+
describe "with :builder option" do
it "yields an instance of the given builder" do
class MyAwesomeCustomBuilder < Formtastic::FormBuilder
Something went wrong with that request. Please try again.