allows custom field_error_proc #899

Merged
merged 3 commits into from Dec 17, 2012

Projects

None yet

4 participants

@tokland
tokland commented Nov 8, 2012

As per request at #896

@tokland
tokland commented Nov 8, 2012

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

@justinfrench
Owner

Looks great! @sobrinho any thoughts?

@sobrinho
Collaborator

Looks great for me! :)

@sobrinho
Collaborator

@tokland can you update the changelog?

@tokland
tokland commented Dec 13, 2012

Is the CHANGELOG ok?

@justinfrench
Owner

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

@justinfrench justinfrench merged commit 9a41fec into justinfrench:master Dec 17, 2012

1 check failed

Details default The Travis build failed
@bughit
bughit commented Sep 24, 2013

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

@tokland
tokland commented Sep 24, 2013

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

@bughit
bughit commented Sep 24, 2013

@@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
bughit commented Sep 24, 2013

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

@tokland
tokland commented Sep 24, 2013

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
bughit commented Sep 24, 2013

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
Owner

@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
tokland commented Sep 24, 2013

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
Owner

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