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

If provided, use option `id` to specify `for` attribute on label for (text, email...) fields, checkbox, radio and select. Solves bootstrap-ruby#342 issue and supercede bootstrap-ruby#221 and bootstrap-ruby#343 pull requests. Partially solves bootstrap-ruby#251 (only the part regarding the id).
lcreid · Jun 4, 2018 · 65e9d04 · 65e9d04
1 parent b08a5f9
commit 65e9d04
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,9 @@ In addition to these necessary markup changes, the bootstrap_form API itself has
 ### Bugfixes
 
 * Your contribution here!
+* [#357](https://github.com/bootstrap-ruby/bootstrap_form/pull/357) if provided,
+  use html option `id` to specify `for` attribute on label
+  [@duleorlovic](https://github.com/duleorlovic)
 
 ## [2.7.0][] (2017-04-21)
 

diff --git a/lib/bootstrap_form/form_builder.rb b/lib/bootstrap_form/form_builder.rb
@@ -144,10 +144,10 @@ def check_box_with_bootstrap(name, options = {}, checked_value = "1", unchecked_
         disabled_class = " disabled" if options[:disabled]
         if options[:inline]
           label_class = " #{label_class}" if label_class
-          label(label_name, html, class: "form-check-inline#{disabled_class}#{label_class}")
+          label(label_name, html, { class: "form-check-inline#{disabled_class}#{label_class}" }.merge(options[:id].present? ? { for: options[:id] } : {}))
         else
           content_tag(:div, class: "form-check#{disabled_class}") do
-            label(label_name, html, class: ["form-check-label", label_class].compact.join(" "))
+            label(label_name, html, { class: ["form-check-label", label_class].compact.join(" ") }.merge(options[:id].present? ? { for: options[:id] } : {}))
           end
         end
       end
@@ -179,10 +179,10 @@ def radio_button_with_bootstrap(name, value, *args)
       else
         if options[:inline]
           label_class = " #{label_class}" if label_class
-          label(name, html, class: "radio-inline#{disabled_class}#{label_class}", value: value)
+          label(name, html, { class: "radio-inline#{disabled_class}#{label_class}", value: value }.merge(options[:id].present? ? { for: options[:id] } : {}))
         else
           content_tag(:div, class: "radio#{disabled_class}") do
-            label(name, html, value: value, class: label_class)
+            label(name, html, { value: value, class: label_class }.merge(options[:id].present? ? { for: options[:id] } : {}))
           end
         end
       end
@@ -368,7 +368,7 @@ def form_group_builder(method, options, html_options = nil)
           text: label_text,
           class: label_class,
           skip_required: options.delete(:skip_required)
-        }
+        }.merge(css_options[:id].present? ? { for: css_options[:id] } : {})
       end
 
       form_group(method, form_group_options) do

diff --git a/test/bootstrap_checkbox_test.rb b/test/bootstrap_checkbox_test.rb
@@ -70,6 +70,19 @@ class BootstrapCheckboxTest < ActionView::TestCase
     assert_equivalent_xml expected, @builder.check_box(:terms, label_class: 'btn')
   end
 
+  test "check_box 'id' attribute is used to specify label 'for' attribute" do
+    expected = <<-HTML.strip_heredoc
+      <div class="form-check">
+        <label class="form-check-label" for="custom_id">
+          <input name="user[terms]" type="hidden" value="0" />
+          <input class="form-check-input" id="custom_id" name="user[terms]" type="checkbox" value="1" />
+          Terms
+        </label>
+      </div>
+    HTML
+    assert_equivalent_xml expected, @builder.check_box(:terms, id: 'custom_id')
+  end
+
   test "check_box responds to checked_value and unchecked_value arguments" do
     expected = <<-HTML.strip_heredoc
       <div class="form-check">

diff --git a/test/bootstrap_fields_test.rb b/test/bootstrap_fields_test.rb
@@ -176,6 +176,16 @@ class BootstrapFieldsTest < ActionView::TestCase
     assert_equivalent_xml expected, @builder.text_field(:email)
   end
 
+  test "field 'id' attribute is used to specify label 'for' attribute" do
+    expected = <<-HTML.strip_heredoc
+      <div class="form-group">
+        <label class="required" for="custom_id">Email</label>
+        <input class="form-control" id="custom_id" name="user[email]" type="text" value="steve@example.com" />
+      </div>
+    HTML
+    assert_equivalent_xml expected, @builder.text_field(:email, id: :custom_id)
+  end
+
   test "time fields are wrapped correctly" do
     expected = <<-HTML.strip_heredoc
       <div class="form-group">

diff --git a/test/bootstrap_radio_button_test.rb b/test/bootstrap_radio_button_test.rb
@@ -41,6 +41,18 @@ class BootstrapRadioButtonTest < ActionView::TestCase
     assert_equivalent_xml expected, @builder.radio_button(:misc, '1', label: 'This is a radio button', label_class: 'btn')
   end
 
+  test "radio_button 'id' attribute is used to specify label 'for' attribute" do
+    expected = <<-HTML.strip_heredoc
+      <div class="radio">
+        <label for="custom_id">
+          <input id="custom_id" name="user[misc]" type="radio" value="1" />
+          This is a radio button
+        </label>
+      </div>
+    HTML
+    assert_equivalent_xml expected, @builder.radio_button(:misc, '1', label: 'This is a radio button', id: 'custom_id')
+  end
+
   test "radio_button inline label is set correctly" do
     expected = <<-HTML.strip_heredoc
       <label class="radio-inline" for="user_misc_1">

diff --git a/test/bootstrap_selects_test.rb b/test/bootstrap_selects_test.rb
@@ -79,6 +79,20 @@ def options_range(start: 1, stop: 31, selected: nil, months: false)
     assert_equivalent_xml expected, @builder.select(:status, [['activated', 1], ['blocked', 2]], { prompt: "Please Select" }, class: "my-select")
   end
 
+  test "select 'id' attribute is used to specify label 'for' attribute" do
+    expected = <<-HTML.strip_heredoc
+      <div class="form-group">
+        <label for="custom_id">Status</label>
+        <select class="form-control" id="custom_id" name="user[status]">
+          <option value="">Please Select</option>
+          <option value="1">activated</option>
+          <option value="2">blocked</option>
+        </select>
+      </div>
+    HTML
+    assert_equivalent_xml expected, @builder.select(:status, [['activated', 1], ['blocked', 2]], { prompt: "Please Select" }, id: "custom_id")
+  end
+
   test 'selects with addons are wrapped correctly' do
     expected = <<-HTML.strip_heredoc
       <div class="form-group">