From 67681cb1031f41a610e94b9339f0a1b6f2a99a39 Mon Sep 17 00:00:00 2001 From: Tim Cooper Date: Fri, 6 Jan 2012 14:32:25 +1000 Subject: Always treat the object passed to content_tag_for as an array. --- actionpack/lib/action_view/helpers/record_tag_helper.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/record_tag_helper.rb b/actionpack/lib/action_view/helpers/record_tag_helper.rb index b351302d01..980fa8130b 100644 --- a/actionpack/lib/action_view/helpers/record_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/record_tag_helper.rb @@ -81,13 +81,9 @@ module ActionView #
  • ... # def content_tag_for(tag_name, single_or_multiple_records, prefix = nil, options = nil, &block) - if single_or_multiple_records.respond_to?(:to_ary) - single_or_multiple_records.to_ary.map do |single_record| - capture { content_tag_for_single_record(tag_name, single_record, prefix, options, &block) } - end.join("\n").html_safe - else - content_tag_for_single_record(tag_name, single_or_multiple_records, prefix, options, &block) - end + Array.wrap(single_or_multiple_records).map do |single_record| + capture { content_tag_for_single_record(tag_name, single_record, prefix, options, &block) } + end.join("\n").html_safe end private -- cgit v1.2.3 From f4551ca7ac75d1b500d5c20a8571dcd43bda2526 Mon Sep 17 00:00:00 2001 From: Tim Cooper Date: Sat, 7 Jan 2012 16:45:22 +1000 Subject: Remove capture. --- actionpack/lib/action_view/helpers/record_tag_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/record_tag_helper.rb b/actionpack/lib/action_view/helpers/record_tag_helper.rb index 980fa8130b..8e667fe30b 100644 --- a/actionpack/lib/action_view/helpers/record_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/record_tag_helper.rb @@ -82,7 +82,7 @@ module ActionView # def content_tag_for(tag_name, single_or_multiple_records, prefix = nil, options = nil, &block) Array.wrap(single_or_multiple_records).map do |single_record| - capture { content_tag_for_single_record(tag_name, single_record, prefix, options, &block) } + content_tag_for_single_record tag_name, single_record, prefix, options, &block end.join("\n").html_safe end -- cgit v1.2.3 From 69512e1a0e5adf1bcfbda7aee7d75ea2b183fb50 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 22:47:28 -0200 Subject: Add test for content tag with prefix and extra html options --- actionpack/test/template/record_tag_helper_test.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index ec777d15c4..247704928e 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -33,8 +33,8 @@ class RecordTagHelperTest < ActionView::TestCase end def test_content_tag_for - expected = %(
  • ) - actual = content_tag_for(:li, @post, :class => 'bar') { } + expected = %(
  • ) + actual = content_tag_for(:li, @post) { } assert_dom_equal expected, actual end @@ -44,9 +44,15 @@ class RecordTagHelperTest < ActionView::TestCase assert_dom_equal expected, actual end - def test_content_tag_for_with_extra_html_tags + def test_content_tag_for_with_extra_html_options expected = %() - actual = content_tag_for(:tr, @post, {:class => "bar", :style => "background-color: #f0f0f0"}) { } + actual = content_tag_for(:tr, @post, :class => "bar", :style => "background-color: #f0f0f0") { } + assert_dom_equal expected, actual + end + + def test_content_tag_for_with_prefix_and_extra_html_options + expected = %() + actual = content_tag_for(:tr, @post, :archived, :class => "bar", :style => "background-color: #f0f0f0") { } assert_dom_equal expected, actual end -- cgit v1.2.3 From 5c997ccadd0014577c4b67d92fdeeb53bdad9637 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 22:49:00 -0200 Subject: No need for concat as well --- actionpack/test/template/record_tag_helper_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index 247704928e..a349f803de 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -80,7 +80,7 @@ class RecordTagHelperTest < ActionView::TestCase post_1 = Post.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } post_2 = Post.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true } expected = %(
  • Hello!
    • \n
  • World!
    • ) - actual = content_tag_for(:li, [post_1, post_2]) { |post| concat post.body } + actual = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual end @@ -88,19 +88,19 @@ class RecordTagHelperTest < ActionView::TestCase post_1 = Post.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } post_2 = Post.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true } expected = %(
    Hello!
    \n
    World!
    ) - actual = div_for([post_1, post_2]) { |post| concat post.body } + actual = div_for([post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual end def test_content_tag_for_single_record_is_html_safe - result = div_for(@post, :class => "bar") { concat @post.body } + result = div_for(@post, :class => "bar") { @post.body } assert result.html_safe? end def test_content_tag_for_collection_is_html_safe post_1 = Post.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } post_2 = Post.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true } - result = content_tag_for(:li, [post_1, post_2]) { |post| concat post.body } + result = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert result.html_safe? end end -- cgit v1.2.3 From 53381be007785369f5f3c41a19693a2ec0d43e31 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 22:51:27 -0200 Subject: Mimic AR models yielding when building new records, avoid using tap --- actionpack/test/template/record_tag_helper_test.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index a349f803de..97dfb473d2 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -10,6 +10,8 @@ class Post @id = nil @body = nil super + + yield self if block_given? end def id @@ -77,16 +79,16 @@ class RecordTagHelperTest < ActionView::TestCase end def test_content_tag_for_collection - post_1 = Post.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } - post_2 = Post.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true } + post_1 = Post.new { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } + post_2 = Post.new { |post| post.id = 102; post.body = "World!"; post.persisted = true } expected = %(
  • Hello!
    • \n
  • World!
    • ) actual = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual end def test_div_for_collection - post_1 = Post.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } - post_2 = Post.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true } + post_1 = Post.new { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } + post_2 = Post.new { |post| post.id = 102; post.body = "World!"; post.persisted = true } expected = %(
    Hello!
    \n
    World!
    ) actual = div_for([post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual @@ -98,8 +100,8 @@ class RecordTagHelperTest < ActionView::TestCase end def test_content_tag_for_collection_is_html_safe - post_1 = Post.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } - post_2 = Post.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true } + post_1 = Post.new { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } + post_2 = Post.new { |post| post.id = 102; post.body = "World!"; post.persisted = true } result = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert result.html_safe? end -- cgit v1.2.3 From 22ffc3dceaa83bcbfcfa541cc992346e6d428fbf Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 23:04:18 -0200 Subject: Cleanup persisted setup for model --- actionpack/test/template/record_tag_helper_test.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index 97dfb473d2..127e1b6bdd 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -11,6 +11,7 @@ class Post @body = nil super + @persisted = true yield self if block_given? end @@ -31,7 +32,6 @@ class RecordTagHelperTest < ActionView::TestCase def setup super @post = Post.new - @post.persisted = true end def test_content_tag_for @@ -79,16 +79,16 @@ class RecordTagHelperTest < ActionView::TestCase end def test_content_tag_for_collection - post_1 = Post.new { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } - post_2 = Post.new { |post| post.id = 102; post.body = "World!"; post.persisted = true } + post_1 = Post.new { |post| post.id = 101; post.body = "Hello!" } + post_2 = Post.new { |post| post.id = 102; post.body = "World!" } expected = %(
  • Hello!
    • \n
  • World!
    • ) actual = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual end def test_div_for_collection - post_1 = Post.new { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } - post_2 = Post.new { |post| post.id = 102; post.body = "World!"; post.persisted = true } + post_1 = Post.new { |post| post.id = 101; post.body = "Hello!" } + post_2 = Post.new { |post| post.id = 102; post.body = "World!" } expected = %(
    Hello!
    \n
    World!
    ) actual = div_for([post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual @@ -100,8 +100,8 @@ class RecordTagHelperTest < ActionView::TestCase end def test_content_tag_for_collection_is_html_safe - post_1 = Post.new { |post| post.id = 101; post.body = "Hello!"; post.persisted = true } - post_2 = Post.new { |post| post.id = 102; post.body = "World!"; post.persisted = true } + post_1 = Post.new { |post| post.id = 101; post.body = "Hello!" } + post_2 = Post.new { |post| post.id = 102; post.body = "World!" } result = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert result.html_safe? end -- cgit v1.2.3 From ba77ff3c1b583fdacf733404bbf4508042b45656 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 23:05:18 -0200 Subject: Do not mutate options hash --- actionpack/lib/action_view/helpers/record_tag_helper.rb | 4 ++-- actionpack/test/template/record_tag_helper_test.rb | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/record_tag_helper.rb b/actionpack/lib/action_view/helpers/record_tag_helper.rb index 8e667fe30b..8dcead64fb 100644 --- a/actionpack/lib/action_view/helpers/record_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/record_tag_helper.rb @@ -92,8 +92,8 @@ module ActionView # for each record. def content_tag_for_single_record(tag_name, record, prefix, options, &block) options, prefix = prefix, nil if prefix.is_a?(Hash) - options ||= {} - options.merge!({ :class => "#{dom_class(record, prefix)} #{options[:class]}".strip, :id => dom_id(record, prefix) }) + options = options ? options.dup : {} + options.merge!(:class => "#{dom_class(record, prefix)} #{options[:class]}".strip, :id => dom_id(record, prefix)) if block.arity == 0 content_tag(tag_name, capture(&block), options) else diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index 127e1b6bdd..e7e489b96d 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -105,4 +105,10 @@ class RecordTagHelperTest < ActionView::TestCase result = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert result.html_safe? end + + def test_content_tag_for_does_not_change_options_hash + options = { :class => "important" } + result = content_tag_for(:li, @post, options) { } + assert_equal({ :class => "important" }, options) + end end -- cgit v1.2.3 From dcad38542039154156d79510dbdb1be4a6cfbf76 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 23:11:55 -0200 Subject: Refactor content tag to not detect options Hash always Only check for options and prefix arguments order once when running content_tag_for with a collection. --- actionpack/lib/action_view/helpers/record_tag_helper.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/record_tag_helper.rb b/actionpack/lib/action_view/helpers/record_tag_helper.rb index 8dcead64fb..4cd5fa2ad1 100644 --- a/actionpack/lib/action_view/helpers/record_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/record_tag_helper.rb @@ -81,6 +81,8 @@ module ActionView #
  • ... # def content_tag_for(tag_name, single_or_multiple_records, prefix = nil, options = nil, &block) + options, prefix = prefix, nil if prefix.is_a?(Hash) + Array.wrap(single_or_multiple_records).map do |single_record| content_tag_for_single_record tag_name, single_record, prefix, options, &block end.join("\n").html_safe @@ -91,14 +93,11 @@ module ActionView # Called by content_tag_for internally to render a content tag # for each record. def content_tag_for_single_record(tag_name, record, prefix, options, &block) - options, prefix = prefix, nil if prefix.is_a?(Hash) options = options ? options.dup : {} - options.merge!(:class => "#{dom_class(record, prefix)} #{options[:class]}".strip, :id => dom_id(record, prefix)) - if block.arity == 0 - content_tag(tag_name, capture(&block), options) - else - content_tag(tag_name, capture(record, &block), options) - end + options.merge!(:class => "#{dom_class(record, prefix)} #{options[:class]}".rstrip, :id => dom_id(record, prefix)) + + content = block.arity == 0 ? capture(&block) : capture(record, &block) + content_tag(tag_name, content, options) end end end -- cgit v1.2.3 From b64b7d09c0331e04304c4b6c49f5c314f414475f Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Tue, 17 Jan 2012 23:24:17 -0200 Subject: Cleanup Post model, no need to require fake_models --- actionpack/test/template/record_tag_helper_test.rb | 23 ++++++---------------- 1 file changed, 6 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index e7e489b96d..8a9a150910 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -1,27 +1,16 @@ require 'abstract_unit' -require 'controller/fake_models' class Post extend ActiveModel::Naming include ActiveModel::Conversion - attr_writer :id, :body + attr_accessor :id, :body def initialize - @id = nil - @body = nil - super + @id = 45 + @body = "What a wonderful world!" - @persisted = true yield self if block_given? end - - def id - @id || 45 - end - - def body - super || @body || "What a wonderful world!" - end end class RecordTagHelperTest < ActionView::TestCase @@ -59,7 +48,7 @@ class RecordTagHelperTest < ActionView::TestCase end def test_block_not_in_erb_multiple_calls - expected = %(
    #{@post.body}
    ) + expected = %(
    What a wonderful world!
    ) actual = div_for(@post, :class => "bar") { @post.body } assert_dom_equal expected, actual actual = div_for(@post, :class => "bar") { @post.body } @@ -67,13 +56,13 @@ class RecordTagHelperTest < ActionView::TestCase end def test_block_works_with_content_tag_for_in_erb - expected = %(#{@post.body}) + expected = %(What a wonderful world!) actual = render_erb("<%= content_tag_for(:tr, @post) do %><%= @post.body %><% end %>") assert_dom_equal expected, actual end def test_div_for_in_erb - expected = %(
    #{@post.body}
    ) + expected = %(
    What a wonderful world!
    ) actual = render_erb("<%= div_for(@post, :class => 'bar') do %><%= @post.body %><% end %>") assert_dom_equal expected, actual end -- cgit v1.2.3 From 8470fc9902b97a5341d3afc5cab1a361d21e52de Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 18 Jan 2012 00:26:17 -0200 Subject: Fix errors when running entire suite due to class name collision The Post class is created everywhere in the test suite, and due to that when applying the Array() logic to refactor content_tag_for, some other change to the Post class was breaking record tag tests. The solution is to rename the class to not collide with others already defined in the test suite. --- .../lib/action_view/helpers/record_tag_helper.rb | 4 +- actionpack/test/template/record_tag_helper_test.rb | 46 +++++++++++----------- 2 files changed, 25 insertions(+), 25 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/record_tag_helper.rb b/actionpack/lib/action_view/helpers/record_tag_helper.rb index 4cd5fa2ad1..1a15459406 100644 --- a/actionpack/lib/action_view/helpers/record_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/record_tag_helper.rb @@ -83,8 +83,8 @@ module ActionView def content_tag_for(tag_name, single_or_multiple_records, prefix = nil, options = nil, &block) options, prefix = prefix, nil if prefix.is_a?(Hash) - Array.wrap(single_or_multiple_records).map do |single_record| - content_tag_for_single_record tag_name, single_record, prefix, options, &block + Array(single_or_multiple_records).map do |single_record| + content_tag_for_single_record(tag_name, single_record, prefix, options, &block) end.join("\n").html_safe end diff --git a/actionpack/test/template/record_tag_helper_test.rb b/actionpack/test/template/record_tag_helper_test.rb index 8a9a150910..e6ca015483 100644 --- a/actionpack/test/template/record_tag_helper_test.rb +++ b/actionpack/test/template/record_tag_helper_test.rb @@ -1,6 +1,6 @@ require 'abstract_unit' -class Post +class RecordTagPost extend ActiveModel::Naming include ActiveModel::Conversion attr_accessor :id, :body @@ -20,77 +20,77 @@ class RecordTagHelperTest < ActionView::TestCase def setup super - @post = Post.new + @post = RecordTagPost.new end def test_content_tag_for - expected = %(
  • ) + expected = %(
  • ) actual = content_tag_for(:li, @post) { } assert_dom_equal expected, actual end def test_content_tag_for_prefix - expected = %(
      ) + expected = %(
        ) actual = content_tag_for(:ul, @post, :archived) { } assert_dom_equal expected, actual end def test_content_tag_for_with_extra_html_options - expected = %() - actual = content_tag_for(:tr, @post, :class => "bar", :style => "background-color: #f0f0f0") { } + expected = %() + actual = content_tag_for(:tr, @post, :class => "special", :style => "background-color: #f0f0f0") { } assert_dom_equal expected, actual end def test_content_tag_for_with_prefix_and_extra_html_options - expected = %() - actual = content_tag_for(:tr, @post, :archived, :class => "bar", :style => "background-color: #f0f0f0") { } + expected = %() + actual = content_tag_for(:tr, @post, :archived, :class => "special", :style => "background-color: #f0f0f0") { } assert_dom_equal expected, actual end def test_block_not_in_erb_multiple_calls - expected = %(
        What a wonderful world!
        ) - actual = div_for(@post, :class => "bar") { @post.body } + expected = %(
        What a wonderful world!
        ) + actual = div_for(@post, :class => "special") { @post.body } assert_dom_equal expected, actual - actual = div_for(@post, :class => "bar") { @post.body } + actual = div_for(@post, :class => "special") { @post.body } assert_dom_equal expected, actual end def test_block_works_with_content_tag_for_in_erb - expected = %(What a wonderful world!) + expected = %(What a wonderful world!) actual = render_erb("<%= content_tag_for(:tr, @post) do %><%= @post.body %><% end %>") assert_dom_equal expected, actual end def test_div_for_in_erb - expected = %(
        What a wonderful world!
        ) - actual = render_erb("<%= div_for(@post, :class => 'bar') do %><%= @post.body %><% end %>") + expected = %(
        What a wonderful world!
        ) + actual = render_erb("<%= div_for(@post, :class => 'special') do %><%= @post.body %><% end %>") assert_dom_equal expected, actual end def test_content_tag_for_collection - post_1 = Post.new { |post| post.id = 101; post.body = "Hello!" } - post_2 = Post.new { |post| post.id = 102; post.body = "World!" } - expected = %(
      • Hello!
        • \n
      • World!
        • ) + post_1 = RecordTagPost.new { |post| post.id = 101; post.body = "Hello!" } + post_2 = RecordTagPost.new { |post| post.id = 102; post.body = "World!" } + expected = %(
      • Hello!
        • \n
      • World!
        • ) actual = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual end def test_div_for_collection - post_1 = Post.new { |post| post.id = 101; post.body = "Hello!" } - post_2 = Post.new { |post| post.id = 102; post.body = "World!" } - expected = %(
        Hello!
        \n
        World!
        ) + post_1 = RecordTagPost.new { |post| post.id = 101; post.body = "Hello!" } + post_2 = RecordTagPost.new { |post| post.id = 102; post.body = "World!" } + expected = %(
        Hello!
        \n
        World!
        ) actual = div_for([post_1, post_2]) { |post| post.body } assert_dom_equal expected, actual end def test_content_tag_for_single_record_is_html_safe - result = div_for(@post, :class => "bar") { @post.body } + result = div_for(@post, :class => "special") { @post.body } assert result.html_safe? end def test_content_tag_for_collection_is_html_safe - post_1 = Post.new { |post| post.id = 101; post.body = "Hello!" } - post_2 = Post.new { |post| post.id = 102; post.body = "World!" } + post_1 = RecordTagPost.new { |post| post.id = 101; post.body = "Hello!" } + post_2 = RecordTagPost.new { |post| post.id = 102; post.body = "World!" } result = content_tag_for(:li, [post_1, post_2]) { |post| post.body } assert result.html_safe? end -- cgit v1.2.3