Config option default_form_class does not work as expected

[hisenb3rg]

I'm upgrading an application to Rails 4, and during this upgrade the SimpleForm gem is also upgraded from 2.1 to 3.1.

This application uses custom class for forms built with SimpleForm. Its initializer for SF looks like this:

SimpleForm.setup do |config|
  ...
  config.form_class = 'my_form_class'
  ...
end

Running the app like that produces depraction message suggestion to use default_form_class instead.

But using default_form_class in the initializer doesn't get the same result: no form gets configured 'my_form_class', but they get 'simple_form'. Looks like this setting is not working at all, or am I missing someting?

[georgeguimaraes]

Please use the mailing list or StackOverflow for questions/help, where a wider community will be able to help you. We reserve the issues tracker for issues only.

[hisenb3rg]

@georgeguimaraes I was trying to be polite and clear. This is a bug and you should reopen the issue.

[georgeguimaraes]

Do you use html: { :class } in the simple_form_for call? If you do, this setting will override default_form_class. I created a new app with simple_form 3.1.0 and it worked as expected.

Can you please provide a sample application that reproduces the error?

[hisenb3rg]

Thanks for reopening!

I've set up a minimal fresh rails-4 application here.

So I see two possible outcomes. Either:

• The default_form_class doesn't work as expected.
• The default_form_class does work as the author intended, but the deprecation of form_class is faulty. Some users (me) need the old behaviour of config.form_class. The deprecation directs them to use config.default_form_class which doesn't behave the same ((a) it is overriden when class is given and b) it is only added to the existing simple_form class). So in this case the only option left is to use the old config.form_class and bare the deprecation message.

In any case I think a line or two in the README about this would really help.

[hisenb3rg]

Hey guys, anyone got a chance to look at this?

[hisenb3rg]

@georgeguimaraes Hey, got any chance to look at this?

[rafaelfranca]

Sorry, we didn't have time to look at it yet.

On Fri, Mar 20, 2015, 06:59 Uroš Jurglič [email protected] wrote:

@georgeguimaraes https://github.com/georgeguimaraes Hey, got any chance
to look at this?

—
Reply to this email directly or view it on GitHub
#1217 (comment)
.

[caiotarifa]

👍

I had the same problem that @jurglic reported, because of that I kept the config.form_class setting.

[hisenb3rg]

@georgeguimaraes, @rafaelfranca If you can give me some input on what would be acceptable correct behaviour, I'd be happy to prepare a PR for it.

[bquorning]

I have SimpleForm 3.0.2 configured with config.form_class = nil, because I don’t want any class added by default. Just setting config.default_form_class = nil isn’t enough, as @@form_class defaults to :simple_form.

Sorry for piggy-backing on your open issue, @jurglic, but I believe our problems are closely related.

[jimherz]

I have the same problem @jurglic reported. We want the form_class to be added both when html: { :class } is passed to simple_form_for and when it is not. This was the behavior prior to 3.1. It is not possible to get this behavior without using config.form_class. See here:

https://github.com/plataformatec/simple_form/blob/master/lib/simple_form/action_view_extensions/form_helper.rb#L19-L23

To get the prior behavior, we are forced to have this confusing configuration:

  config.default_form_class = nil
  ActiveSupport::Deprecation.silence do
    config.form_class = 'form'
  end

I say confusing, because this is not how I would expect these two config options to work if config.form_class is truly deprecated in favor of config.default_form_class

[bquorning]

Ping @rmm5t

[rmm5t]

Ping @rmm5t

Sorry, I overlooked this.

The form_class deprecation and reasons for the new behavior behind default_form_class was documented in #1109. The meat of the suggested changes and discussion start here: #1109 (comment)

[rmm5t]

We want the form_class to be added both when html: { :class } is passed to simple_form_for and when it is not. This was the behavior prior to 3.1. It is not possible to get this behavior without using config.form_class.

Sorry. This was the decision we made at the time. The problem before was that there were no way to get rid of the underlying config.form_class setting.

It is not possible to get this behavior without using config.form_class.

Sure there is. You just have to be explicit in your html: { :class } override by including all the classes you want there.

In my opinion, if you're setting html: { :class }, that's all that should show up on the form element. Anything else breaks the principle of least surprise. I understand it breaks existing behavior of concatenating config.form_class with everything, but I believe that old behavior to be flawed and more error-prone.

[feliperenan]

I don't know if it's an issue yet since it have a long time so I'm closing it. Feel free to reopen if necessary.

[allenwu1973]

This is still an issue, and it's just bad default value

lib/simple_form.rb

  # DEPRECATED: You can define the class to be used on all forms. Default is
  # simple_form.
  mattr_reader :form_class
  @@form_class = :simple_form

  # You can define the default class to be used on all forms. Can be overriden
  # with `html: { :class }`. Defaults to none.
  mattr_accessor :default_form_class
  @@default_form_class = nil

it should be default_form_class = :simple_form and form_class = nil
or just wait for form_class eventually get removed.

[koki-miyazaki]

Sure there is. You just have to be explicit in your html: { :class } override by including all the classes you want there.

In my app, I've been using form_class as a shared class setting, which I don't want to be overridden by { html: { class: '' } } option

-  config.form_class = 'the-shared-class1 the-shared-class2'

so I tried adding classes to the default class .simple_form

.simple_form {
  @extend .the-shared-class1;
  @extend .the-shared-class2;
}

this is not clean way but works

[attenzione]

To get the prior behavior, we are forced to have this confusing configuration:

  config.default_form_class = nil
  ActiveSupport::Deprecation.silence do
    config.form_class = 'form'
  end

Since SimpleForm 5.3.0 the code above no longer works. You need to use a custom deprecator:

SimpleForm.deprecator.silence do
  config.form_class = 'form'
end