Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add _is_first and _is_last to partial rendered by collection #5634

Closed
wants to merge 2 commits into from

7 participants

@dv
dv commented

Much like the _counter variable that is accessible from within a partial called by a collection, the _is_first and _is_last designate if the current call to the partial is for the first or the last item respectively inside the collection.

Useful if in the view, structural changes need to be made to the partial HTML in case of the first or last item of a collection.

dv added some commits
@dv dv Add _is_first and _is_last to partial called by a collection
Much like the _counter variable that is accessible from within a
partial called by a collection, the _is_first and _is_last designate if
the current call to the partial is for the first or the last item
respectively inside the collection.
6fb0361
@dv dv Merge remote branch 'origin/master' into add_is_first_and_is_last_to_…
…partial_by_collection
ee7f408
@josevalim
Owner

Thanks for the PR but if we are going down this road, I would rather have a iteration object we could check iteration.first? or iteration.last? than adding new variables. We could also use iteration.current instead of the variable counter we use today. /cc @jeremy @fxn

@isaacsanders

Is this still an issue?

@drogus drogus was assigned
@drogus
Collaborator

@dv do you have the time to refactor the code based on @josevalim's comments?

@dv
dv commented

Yes, and I'm up for it, however I have trouble finding an existing example "mini-class" like that in the Rails source, to know what the Rails convention is on classes like this (e.g. where do I put the file).

@drogus
Collaborator

@dv I think that it depends on the final look of such iterator. If it's something that could be potentially used in other places, it could go to ActiveSupport. If it's tied to use in views, it should be namespaced with ActionView. If in doubt it's safer to start with ActionView::

@steveklabnik
Collaborator

While you're refactoring, this will need a rebase as well.

@carlosantoniodasilva

Hey, are we going forward with this feature request, or can I consider it closed? Thanks!

@steveklabnik
Collaborator

@dv since we haven't heard from you in months, I'm going to close this. If you'd like to finish up the refactoring, please do so and we can re-open. :)

@dv
dv commented

@steveklabnik yeah, sorry I was unexpectedly swamped with new work and was unable to follow through on this. If I get around to it I'll reopen.

@drogus thanks for that information by the way!

@steveklabnik
Collaborator

No worries! We all get that way at times.

@joeljunstrom

I like this path quite a bit but when going through the source I must agree with @dv that it is really hard to see where in the codebase such small utility classes would be put.

I made a stab at it at joeljunstrom@8fd2174

@josevalim Is this the direction you was thinking? This implementation adds partial_name_iteration as a local, I kept the partial_name_counter for now but that can be removed if we do not need backwards compatibility.

Let me know if I should make a pull request out of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 28, 2012
  1. @dv

    Add _is_first and _is_last to partial called by a collection

    dv authored
    Much like the _counter variable that is accessible from within a
    partial called by a collection, the _is_first and _is_last designate if
    the current call to the partial is for the first or the last item
    respectively inside the collection.
  2. @dv
This page is out of date. Refresh to see the latest.
View
20 actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -340,7 +340,7 @@ def setup(context, options, block)
end
if @path
- @variable, @variable_counter = retrieve_variable(@path)
+ @variable, @variable_counter, @variable_is_first, @variable_is_last = retrieve_variable(@path)
else
paths.map! { |path| retrieve_variable(path).unshift(path) }
end
@@ -371,7 +371,11 @@ def find_partial
if path = @path
locals = @locals.keys
locals << @variable
- locals << @variable_counter if @collection
+ if @collection
+ locals << @variable_counter
+ locals << @variable_is_first
+ locals << @variable_is_last
+ end
find_template(path, locals)
end
end
@@ -383,12 +387,14 @@ def find_template(path=@path, locals=@locals.keys)
def collection_with_template
segments, locals, template = [], @locals, @template
- as, counter = @variable, @variable_counter
+ as, counter, is_first, is_last = @variable, @variable_counter, @variable_is_first, @variable_is_last
locals[counter] = -1
@collection.each do |object|
locals[counter] += 1
+ locals[is_first] = locals[counter] == 0
+ locals[is_last] = locals[counter] == @collection.size-1
locals[as] = object
segments << template.render(@view, locals)
end
@@ -445,8 +451,12 @@ def merge_prefix_into_object_path(prefix, object_path)
def retrieve_variable(path)
variable = @options[:as].try(:to_sym) || path[%r'_?(\w+)(\.\w+)*$', 1].to_sym
- variable_counter = :"#{variable}_counter" if @collection
- [variable, variable_counter]
+ if @collection
+ variable_counter = :"#{variable}_counter"
+ variable_is_first = :"#{variable}_is_first"
+ variable_is_last = :"#{variable}_is_last"
+ end
+ [variable, variable_counter, variable_is_first, variable_is_last]
end
end
end
View
4 actionpack/test/controller/render_test.rb
@@ -612,6 +612,10 @@ def partial_collection_with_as_and_counter
render :partial => "customer_counter_with_as", :collection => [ Customer.new("david"), Customer.new("mary") ], :as => :client
end
+ def partial_collection_with_first_and_last
+ render :partial => "customer_first_last", :collection => [ Customer.new("david"), Customer.new("mary"), Customer.new("joseph") ]
+ end
+
def partial_collection_with_locals
render :partial => "customer_greeting", :collection => [ Customer.new("david"), Customer.new("mary") ], :locals => { :greeting => "Bonjour" }
end
View
1  actionpack/test/fixtures/test/_customer_first_last.erb
@@ -0,0 +1 @@
+<%= customer_first_last.name %><%= customer_first_last_is_first %><%= customer_first_last_is_last %>
View
7 actionpack/test/template/render_test.rb
@@ -223,10 +223,15 @@ def test_render_partial_collection_as_by_symbol
end
def test_render_partial_collection_without_as
- assert_equal "local_inspector,local_inspector_counter",
+ assert_equal "local_inspector,local_inspector_counter,local_inspector_is_first,local_inspector_is_last",
@view.render(:partial => "test/local_inspector", :collection => [ Customer.new("mary") ])
end
+ def test_render_partial_collection_with_first_and_last
+ assert_equal "davidtruefalsemaryfalsefalsejosephfalsetrue",
+ @view.render(:partial => "test/customer_first_last", :collection => [ Customer.new("david"), Customer.new("mary"), Customer.new("joseph") ])
+ end
+
def test_render_partial_with_empty_collection_should_return_nil
assert_nil @view.render(:partial => "test/customer", :collection => [])
end
Something went wrong with that request. Please try again.