Allow assigning a spacer component between items in a rendered collection #2137
Conversation
lib/view_component/collection.rb
Outdated
|
|
||
| def rendered_spacer(view_context) | ||
| if @spacer_component | ||
| spacer = @spacer_component.new |
There was a problem hiding this comment.
Instead of newing ourselves, would it make sense to accept a pre-initialized component? Thoughts being that we can support components that require arguments as a spacer?
There was a problem hiding this comment.
I considered that (but did not need it for my current use-case), so went with the simplest option, which also matches Rails spacer template implementation that does not accept any arguments.
There was a problem hiding this comment.
@BlakeWilliams do you want any other changes here? I feel like accepting components with arguments could be a future enhancement if the need arises.
There was a problem hiding this comment.
@nickcoyne up until now, ViewComponent has not handled component instantiation (and neither has Rails). I think I'd prefer we keep it that way for consistency's sake.
There was a problem hiding this comment.
@joelhawksley I've updated this to accept a component instance.
What are you trying to accomplish?
This PR extends
with_collectionto allow an optionalspacer_componentto be specified. This component will be rendered between items in the rendered collection.What approach did you choose and why?
In use, this is a parallel for Rails' partial rendering with a spacer template using the
spacer_templateoption.The implementation is similar to that used in Rails' partial rendering, where the elements of the collection are simply joined with the content of the rendered spacer component.
Anything you want to highlight for special attention from reviewers?
I'm not sure that I'm completely following the existing patterns for the updates in
collection.rb. Please suggest edits if necessary. Thanks!