Fix name and id for checkbox/radio

[bikerduweb]

When using bootstrap_form_tag, "name", "id" and "for" properties for check_box and radio_button are not properly generated, because there is no objet_name to refer to.

This patch fix that to restore proper naming of checkboxes and radio buttons.

[mattbrictson]

Thanks for the PR! It has been a while since I've touched this code, so I will need your help to be able to fully review and merge this.

Could you explain in more detail the bug you are fixing, and what changes were necessary?

Also, I'll need you to fix the test failure and add a CHANGELOG entry before this can be merged. Thanks!

[bikerduweb]

In current version of bootstrap_form_tag, name, id and for properties are not properly generated just because there is no object name to refer to.
Exemple from your test:
<input id="_misc" name="[misc]" type="checkbox" value="1" />
=> Here we see that something is missing in the id, ie should be objectname_misc and not just "_misc", and same for name which should be objectname[misc] instead of [misc]... I say should because it would be like that if we had an object... But when we don't have any object, then the proper way to generate all this in rails is id="misc" and name="misc" ...
So my patch make sure that name and id are generated the rails way

The second patch allow to be able to pass "for" parameter to the label in a control group. In the current version, there is no way to specify a specific for parameter ... It also fix the way for parameter is generated in a control group when a specific id have been given.

I'm not fluent in english so it's hard to turn all this in proper english. I've fixed the tests

[mattbrictson]

Thanks for the explanation! I'll review this week. In the meantime, can you add an entry to the CHANGELOG file as part of this PR?

[bikerduweb]

Matt, I've added the changelog, but I'm not sure if it's good english ;-) I also tried to reference a few tickets that seems linked to the fix in this PR. So it should Fix #266, Fix #193, Fix #241 and Fix #213

[dan-klasson]

I've tried this PR and I'm still getting name="[foo]"

[bikerduweb]

Sorry to hear that Dan. I use my patch in production, and it work properly so I'm not sure why it isn't working for you. There is also a test that validate the name format. Maybe you could try to debug your code or provide an example of what you do exactly ?

[bikerduweb]

If you need, here is a live example:

    <%= bootstrap_form_tag url:'/', html: { role: 'form' } do |form| %>
      <%= form.text_field :email_id, placeholder: 'Mail ID' %>
    <% end %>

it converts to

<form role="form" action="/" accept-charset="UTF-8" method="post">
<input name="utf8" type="hidden" value="✓">
<input type="hidden" name="authenticity_token" value="eNOQVLssnDo+DWNMvsFFjx+Nuos9qXUPK857Pr2LXHmAfUqlDPPAM7R5NobLCLyJAyZCloBYaXcOf9E0IIVqBQ==">
<div class="form-group">
  <label class="sr-only control-label" for="email_id">Email</label>
  <div class="input-group">
    <input placeholder="Mail ID" class="form-control" name="email_id" id="email_id" type="text"> 
  </div>
</div>
</form>

[dan-klasson]

@veilleperso

This is what I did:

1. cloned the repo

2. fetched the PR: git fetch origin pull/251/muweb

3. git checkout muweb

4. added to Gemfile: gem 'bootstrap_form', path: '~/rails-bootstrap-forms'

5. Added this:

   <%= bootstrap_form_tag url: listing_path(@Listing) do |f| %>
   <%= f.select :foo, options_for_select([1, 2]) %>
   <% end %>

But got this output:

<form role="form" action="/listings/1" accept-charset="UTF-8" method="post"><input name="utf8" type="hidden" value="✓">
  <div class="form-group">
    <label class="control-label" for="foo">Foo</label>
    <select class="form-control" name="[foo]" id="_foo">
      <option value="1">1</option>
      <option value="2">2</option>
    </select>
  </div>
</form>

[bikerduweb]

I see: i just fixed the generation of name and id for checkbox/radio (as stated in the tile), and that extend to any input field as they work the same. But I didn't take care of select tag which use a different code. I will look into it.

[dan-klasson]

Also, using the documented way for radio buttons, makes the html come out like this:

<input id="rent_sale_{:value=>&quot;foo&quot;}" name="rent_sale" type="radio" value="{:value=>&quot;foo&quot;}">

[bikerduweb]

I think that for radio_button the proper way to do it is not use :value => "foo" , but to pass the value as you would with bootstrap_form ... So your bug seems to be much more related to the way you build your code, as you seem to have copied the exemple of a text_field to a radio_button, but they are not built the same way ...
For the proper way to use radio_button, please use the same way as you do in a classic boostrap_form / rails form

[dan-klasson]

@veilleperso Select works now as well. Thank you

[dan-klasson]

Same problem with hidden fields though

[bikerduweb]

hidden_field are not processed by rails-bootstrap-form, I can't find any specific code related to hidden field, so I guess the problem is that hidden field are delegated to rails without specific processing by rails-bootstrap-form like for all other fields ...
I don't have to time to fix that because it's not related to my patch. Once hidden_field are handled by rails-bootstrap-form, then I think that the current patch will work like for any other input field.

But feel free to address this specific issue if you need it for your own use.

[mattbrictson]

@veilleperso @dan-klasson Thanks for all your help on this! Would you say this is ready for merge?

Also, if we are going to leave hidden_field for a future PR, could you please create a GitHub issue in the meantime that describes the problem? That way we won't lose track of it.

[bikerduweb]

@mattbrictson no need for a PR, i had a quick look this morning and i guess this last patch should fix the hidden_field tag... I also added a test for it.

I guess now everything is ready to merge.

[dan-klasson]

Still having the problem with the hidden field. But can leave for later.

[bikerduweb]

Sorry Dan, but i don't see any problem on hidden field

    <%= bootstrap_form_tag url: '/' do |form| %>
      <%= form.hidden_field :email, value: 'hello@yahoo.com' %>
    <% end %>

gives me

<form role="form" action="/" accept-charset="UTF-8" method="post">
<input name="utf8" type="hidden" value="✓">
<input type="hidden" name="authenticity_token" value="4IAdC35B25rLlhGIb06N2ccqU9FNPnFfHyTirSu3wuDs53eNzGk7EIiqcIqt2cMJ8ftyYJixYV0BEFuW4uMUFQ==">
<input value="hello@yahoo.com" id="email" name="email" type="hidden">
</form>

[dan-klasson]

@veilleperso hidden fields with form_for is not working for me. Neither is multiselect.

[bikerduweb]

Sorry to hear that @dan-klasson but it's outside of the scope of this patch: I only patched a few things for bootstrap_form_for only, mainly for my own needs at first...
You now speak of problems with form_for, feel free to investigate on that subject if there are bugs there, and maybe patch them, as I didn't saw any bug on using form_for + hidden_field ... For multiselect, I don't use them, sorry.

BTW "not working" is a pretty vague concept, so it's hard to help you ;-) It would probably help if instead you show us some code that "don't work" ?

[dan-klasson]

@veilleperso No worries. Not sure how to use hidden field with form_for and bootstrap form. f.hidden_field throws a nilclass exception. hidden_field produces bar[] instead of foo[bar]. Multiselect has the same issue as select before you fixed it.

I'll review your code changes and submit my own PR.

[bikerduweb]

@dan-klasson Here is an exemple of use from code in production, where "Invoice" is an activerecord object with a field customer_id

    <%= form_for(Invoice.first, url: '/') do |form| %>
      <%= form.hidden_field :customer_id, id: 'bill_customer_id'  %>
    <% end %>

The resulting html is:

<form class="edit_invoice" id="edit_invoice_6" action="/" accept-charset="UTF-8" method="post">
<input name="utf8" type="hidden" value="&#x2713;" />
<input type="hidden" name="_method" value="patch" /><input type="hidden" name="authenticity_token" value="4X1M4x18fTj17LUSr+j2ZGULbIDCnTGw3MUJ3e3p9HJRSyHt7WsSvVGdEOLAY+TPI9BWVGm1CgqE6XS7ZMtXPQ==" />
<input id="bill_customer_id" type="hidden" value="5" name="invoice[customer_id]" />
</form>

As you can see the hidden field is properly generated, using the id I manually specified, and properly building the name field

So I guess that the problem is much more in the way you write your code, than in the gem or my patch. Again, my patch didn't concern form_for, only bootstrap_form_for.

[mattbrictson]

@veilleperso @dan-klasson I lost track of the discussion here. Have all the issues been resolved and is this PR good to merge? Let me know, thanks!

[lcreid]

Thank you for your interest in improving bootstrap_form. bootstrap_form has been substantially changed since this PR was submitted. The upgrade to Bootstrap 4 introduced many changes that may have addressed this PR, or made it not applicable. Therefore, we're closing this PR.

If you still believe this PR is needed, please re-open it and update it to be mergeable with the current master. Our apologies for not responding sooner.