Merge pull request #13122 from andrielfn/fix-collection-label-class

Fix the merge of a label class defined inside the block

actionview/CHANGELOG.md

+*   Label tags generated by collection helpers only inherit the `:index` and
+    `:namespace` from the input, because only these attributes modifies the
+    `for` attribute of the label. Also, the input attributes don't have
+    precedence over the label attributes anymore.
+
+    Before:
+
+        collection = [[1, true, { class: 'foo' }]]
+        f.collection_check_boxes :options, collection, :second, :first do |b|
+          b.label(class: 'my_custom_class')
+        end
+
+        # => <label class="foo" for="user_active_true">1</label>
+
+    After:
+
+        collection = [[1, true, { class: 'foo' }]]
+        f.collection_check_boxes :options, collection, :second, :first do |b|
+          b.label(class: 'my_custom_class')
+        end
+
+        # => <label class="my_custom_class" for="user_active_true">1</label>
+
+    *Andriel Nuernberg*
+
 *   Fixed a long-standing bug in `json_escape` that causes quotation marks to be stripped.
     This method also escapes the \u2028 and \u2029 unicode newline characters which are
     treated as \n in JavaScript. This matches the behaviour of the AS::JSON encoder. (The

actionview/lib/action_view/helpers/tags/collection_helpers.rb

@@ -18,7 +18,7 @@ def initialize(template_object, object_name, method_name, object,
           end
 
           def label(label_html_options={}, &block)
-            html_options = label_html_options.merge(@input_html_options)
+            html_options = @input_html_options.slice(:index, :namespace).merge(label_html_options)
             @template_object.label(@object_name, @sanitized_attribute_name, @text, html_options, &block)
           end
         end

actionview/lib/action_view/helpers/tags/label.rb

@@ -30,7 +30,6 @@ def render(&block)
           add_default_name_and_id_for_value(tag_value, name_and_id)
           options.delete("index")
           options.delete("namespace")
-          options.delete("multiple")
           options["for"] = name_and_id["id"] unless options.key?("for")
 
           if block_given?

actionview/test/template/form_collections_helper_test.rb

@@ -84,6 +84,24 @@ def with_collection_check_boxes(*args, &block)
     assert_select 'input[type=radio][value=false].bar#user_active_false'
   end
 
+  test 'collection radio sets the label class defined inside the block' do
+    collection = [[1, true, {class: 'foo'}], [0, false, {class: 'bar'}]]
+    with_collection_radio_buttons :user, :active, collection, :second, :first do |b|
+      b.label(class: "collection_radio_buttons")
+    end
+
+    assert_select 'label.collection_radio_buttons[for=user_active_true]'
+    assert_select 'label.collection_radio_buttons[for=user_active_false]'
+  end
+
+  test 'collection radio does not include the input class in the respective label' do
+    collection = [[1, true, {class: 'foo'}], [0, false, {class: 'bar'}]]
+    with_collection_radio_buttons :user, :active, collection, :second, :first
+
+    assert_no_select 'label.foo[for=user_active_true]'
+    assert_no_select 'label.bar[for=user_active_false]'
+  end
+
   test 'collection radio does not wrap input inside the label' do
     with_collection_radio_buttons :user, :active, [true, false], :to_s, :to_s
 
@@ -215,6 +233,24 @@ def with_collection_check_boxes(*args, &block)
     assert_select 'input[type=checkbox][value=2].bar'
   end
 
+  test 'collection check boxes sets the label class defined inside the block' do
+    collection = [[1, 'Category 1', {class: 'foo'}], [2, 'Category 2', {class: 'bar'}]]
+    with_collection_check_boxes :user, :active, collection, :second, :first do |b|
+      b.label(class: 'collection_check_boxes')
+    end
+
+    assert_select 'label.collection_check_boxes[for=user_active_category_1]'
+    assert_select 'label.collection_check_boxes[for=user_active_category_2]'
+  end
+
+  test 'collection check boxes does not include the input class in the respective label' do
+    collection = [[1, 'Category 1', {class: 'foo'}], [2, 'Category 2', {class: 'bar'}]]
+    with_collection_check_boxes :user, :active, collection, :second, :first
+
+    assert_no_select 'label.foo[for=user_active_category_1]'
+    assert_no_select 'label.bar[for=user_active_category_2]'
+  end
+
   test 'collection check boxes accepts selected values as :checked option' do
     collection = (1..3).map{|i| [i, "Category #{i}"] }
     with_collection_check_boxes :user, :category_ids, collection, :first, :last, :checked => [1, 3]

actionview/test/template/form_helper_test.rb

@@ -1300,6 +1300,24 @@ def post.active; false; end
     assert_dom_equal expected, output_buffer
   end
 
+  def test_form_with_index_and_with_collection_radio_buttons
+    post = Post.new
+    def post.active; false; end
+
+    form_for(post, index: '1') do |f|
+      concat f.collection_radio_buttons(:active, [true, false], :to_s, :to_s)
+    end
+
+    expected = whole_form("/posts", "new_post", "new_post") do
+      "<input id='post_1_active_true' name='post[1][active]' type='radio' value='true' />" +
+      "<label for='post_1_active_true'>true</label>" +
+      "<input checked='checked' id='post_1_active_false' name='post[1][active]' type='radio' value='false' />" +
+      "<label for='post_1_active_false'>false</label>"
+    end
+
+    assert_dom_equal expected, output_buffer
+  end
+
   def test_form_for_with_collection_check_boxes
     post = Post.new
     def post.tag_ids; [1, 3]; end
@@ -1397,6 +1415,24 @@ def post.tag_ids; [1]; end
     assert_dom_equal expected, output_buffer
   end
 
+  def test_form_with_index_and_with_collection_check_boxes
+    post = Post.new
+    def post.tag_ids; [1]; end
+    collection = [[1, "Tag 1"]]
+
+    form_for(post, index: '1') do |f|
+      concat f.collection_check_boxes(:tag_ids, collection, :first, :last)
+    end
+
+    expected = whole_form("/posts", "new_post", "new_post") do
+      "<input checked='checked' id='post_1_tag_ids_1' name='post[1][tag_ids][]' type='checkbox' value='1' />" +
+      "<label for='post_1_tag_ids_1'>Tag 1</label>" +
+      "<input name='post[tag_ids][]' type='hidden' value='' />"
+    end
+
+    assert_dom_equal expected, output_buffer
+  end
+
   def test_form_for_with_file_field_generate_multipart
     Post.send :attr_accessor, :file