add custom_control

[ThomasSevestre]

static_control wraps the displayed value in a p tag.
This is a problem in some edge use case, when you want to display an ul element.
It leads to invalid HTML generation that is silently changed by erb / haml / slim.

This view (slim) :

= f.static_control(:my_attribute) do
  ul
    li abc
    li def

Generate something like :

...
<p></p>
<ul><li>abc</li><li>def</li><ul>
<p></p>
...

The new custom_control helper generates expected view :

...
<p>
<ul><li>abc</li><li>def</li><ul>
</p>
...

[mattbrictson]

It seems like this PR is a different way of accomplishing the same thing as #301. Am I right?

@ThomasSevestre @antpaw which do you think is the better solution?

[antpaw]

I don't like the name of the method, i prefer to just remove the private attribute form form_group_builder, i think it will just work they same way custom_control does but with way less code and overhead.

[ThomasSevestre]

It all depends on your will to publish form_group_builder in the gem API. If you want to be able to change form_group_builder behavior without breaking the API, then you should keep it private and use a proxy method.

[antpaw]

How does removing this method from private breaks the API? I think its not a major change, it will be the same as a new method was added?

[ThomasSevestre]

If the method becomes public, future changes to the method will break the API.
It's not my gem and I don't known if it is ok or not.

[mattbrictson]

As the de facto maintainer of this gem, I am concerned about breaking changes, but I am also concerned about support and usability of the API. For me, the method name form_group_builder is not very intuitive and the difference between form_group and form_group_builder is subtle and could confuse people.

I would prefer we make a new public method with a good, self-explanatory name. How we structure the implementation is less important: it could even just be a public alias of form_group_builder.

[mattbrictson]

Would this accomplish the same thing? Do we need the extract_options! and capture?

def custom_control(name, options={}, &block)
   form_group_builder(name, options, &block)
end

[ThomasSevestre]

extract_options! is not needed. I've copied static_control to have the same ergonomy. It allows this kind of things:

= custom_control label: 'MyLabel' do
  ...

capture is not needed since it is already done in form_group

[mattbrictson]

I see... So in order to support the example you described (custom_control label: 'MyLabel'), then we do need extract_options!. Right?

capture is not needed since it is already done in form_group

Agreed. Can you update the PR to remove it?

[mattbrictson]

@ThomasSevestre I'd still like merge this PR if you are interested. Could you review the latest code comments and provide an update? Thanks

[ThomasSevestre]

@mattbrictson Sorry for my late reply, I've removed capturecall and kept extract_options!

[mattbrictson]

Thanks! 🙇