Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make content_tag_for and div_for accepts the array of records #2816

Merged
merged 1 commit into from
Sep 4, 2011

Conversation

sikachu
Copy link
Member

@sikachu sikachu commented Sep 2, 2011

So instead of having to do this:

   @items.each do |item|
     content_tag_for(:li, item) do
        Title: <%= item.title %>
     end
   end

You can now do this:

   content_tag_for(:li, @items) do |item|
     Title: <%= item.title %>
   end

(Same goes for the div_for)

@@ -17,6 +17,19 @@ module ActionView
#
# <div id="person_123" class="person foo"> Joe Bloggs </div>
#
# You can also passed an array of Active Record objects, which will then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/passed/pass

@sikachu
Copy link
Member Author

sikachu commented Sep 3, 2011

@vijaydev pull request updated as requested.

@sikachu
Copy link
Member Author

sikachu commented Sep 3, 2011

@josevalim bro, can I have a review of this commit?

options.merge!({ :class => "#{dom_class(record, prefix)} #{options[:class]}".strip, :id => dom_id(record, prefix) })
content_tag(tag_name, options, &block)
if record.respond_to?(:to_ary)
record.map do |single_record|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be record.to_ary.map ? I don't think implementing to_ary means it implements all Enumerable methods.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I totally agree on this one.

So instead of having to do this:

   @items.each do |item|
     content_tag_for(:li, item) do
        Title: <%= item.title %>
     end
   end

You can now do this:

   content_tag_for(:li, @Items) do |item|
     Title: <%= item.title %>
   end
@sikachu
Copy link
Member Author

sikachu commented Sep 4, 2011

@josevalim pull request updated.

I think it's better to change the method signature a bit too, to make sure what to expected from the variable.

josevalim added a commit that referenced this pull request Sep 4, 2011
Make `content_tag_for` and `div_for` accepts the array of records
@josevalim josevalim merged commit 036a250 into rails:master Sep 4, 2011
@josevalim
Copy link
Contributor

Perfect pull request: docs + code + tests. Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants