Use option[:id] for label's *for* attribute

[duleorlovic]

If provided, use html option id to specify for attribute on label
for (text, email...) fields, checkbox, radio and select.
Solves #213, #221, #342, #349 issues and supercede #221 and #343 pull
requests.

Partially solves #251 (only the part regarding the id.

[lcreid]

Hi. Thanks for your contribution!

As you may have read about in #361, we got a bit behind in handling PRs. Unfortunately, that means we've got several different solutions for some issues now. We're still discussing how to deal with the overlapping/duplicate PRs, and we welcome your comments on that topic.

For now, note that much of this PR is also covered by PR #369 (which was PR #346 last year). @mattbrictson your thoughts?

[duleorlovic]

OK, I will check my use cases against master when #369 is merged.

[lcreid]

@duleorlovic I'm reopening this because it has work we can use, but please see my review comments which I'll post shortly.

[lcreid]

There is some very small overlap between this and #369, but I'll deal with any conflicts or redundancy where I finish #369. This fixes an important issue, especially for Rails 5.1, which doesn't generate DOM IDs by default and therefore the use of id: something is almost obligatory.

[lcreid]

I think this will be redundant when #369 is merged.

[lcreid]

@duleorlovic Again, I apologize for suggesting that this PR might not be needed. We're trying to make #369 do absolutely one thing only, so this PR is needed to fix the id issue. If the challenge is the merge conflict, let me know. Maybe I can help?

[duleorlovic]

Sure, I can rebase and fix conflicts... I will do it tomorrow, after I use master on some of my projects...

[duleorlovic]

I rebased and forced push so there are no conflicts now...

[mattbrictson]

Looks great to me 👍

@lcreid is this good to merge?

[lcreid]

Nice work! Very clean and elegant solution to an obscure problem.

[lcreid]

@duleorlovic @mattbrictson I think there's one more issue that this PR could address. Should I open a new issue, or should we include it here?

Basically, for text fields, at least, the label's for attribute doesn't pick up the user-specified ID. You can show the issue by adding this test to test/bootstrap_form_test.rb:

  test "bootstrap_form_for does not clobber custom options" do
    expected = <<-HTML.strip_heredoc
      <form accept-charset="UTF-8" action="/users" class="new_user" id="new_user" method="post" role="form">
        <input name="utf8" type="hidden" value="&#x2713;" />
        <div class="form-group">
          <label for="ID">Email</label>
          <input class="form-control" id="ID" name="NAME" type="text" />
        </div>
      </form>
    HTML
    assert_equivalent_xml expected, bootstrap_form_for(@user) { |f| f.text_field :email, name: 'NAME', id: "ID" }
  end

[mattbrictson]

I think it is best to merge PRs sooner rather than later, so I'll merge this one now and we can open a new issue detailing the problem for text fields. Could you open an issue, @lcreid ? Thanks

[lcreid]

Sounds good. I'll make "open a new issue" my default action going forward.