-
Notifications
You must be signed in to change notification settings - Fork 443
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
Support multiple views #387
Comments
@fsateler that would be great! The branch is |
Hey, I tried out that branch. The good news: it works for partials without locals. Things I couldn't get to work:
|
So, I know I agreed to file a new issue, but I think what I have written here covers this issue, while also touching on things like #39 and #410 . So I'll just post it here, and then maybe copy paste the relevant parts into new issues. I'm currently trying to migrate from Cells to ViewComponent. We are using cells for rendering whole pages, not just the individual components. There are a couple of use cases that (AFAICT) are not possible (yet) with view components. I'm going to show them in a simplified form, but sticking to what we actually do. Use case 1: splitting parts outSay we have a regular form, for an object with many fields. We split the fields into sections: <%# show.erb %>
<%= simple_form_for model do |f| %>
<%= render view: 'personal_information', locals: { form: f } %>
<%= render view: 'work_information', locals: { form: f } %>
<%= render view: 'actions' %>
<% end %> <%# personal_information.erb %>
<%= form.input :first_name %>
<%= form.input :last_name %> <%# work_information.erb %>
<%= form.input :company_name %>
<%= form.input :company_role %> <%# actions.erb %>
<%= cell(SubmitButtonCell) %>
<%= cell(GoBackButtonCell) %> Use case 2: polymorphismThis is an extension of the previous one. We are using STI for some of our models. For example, we store data from people from multiple countries, and since some of the data would differ between countries (think different national id regimes or different geographical units), we use STI to be able to validate the data for each country. Thus we would have I'm going to present a simplified version of the form component for said Person entity:
The base The main template (show.erb) would be as follows: <%= simple_form_for model do |f| %>
<%# we render the common parts in the main template %>
<%= f.input :first_name %>
<%= f.input :last_name %>
<%= render view: 'address_information', locals: { form: f } %>
<% end %> And then each country would implement a different version: <%# app/cells/person/country1/form/address_information.erb %>
<%= form.input :state %>
<%= form.input :county %> <%# app/cells/person/country2/form/address_information.erb %>
<%= form.input :region %>
<%= form.input :city %>
<%= form.input :town %> Usage in the edit and new views does not need to know about which country we are dealing with. We just call: <%= cell(Person::FormCell, @person) %> Use case 3: List/Table componentsSometimes, a component will render a list or a table of elements. In those cases, it is nice to be able to iterate over the list, and render each element from a separate template:
Where the templates are as follows: <%# show.erb %>
<table class="table table-condensed">
<tbody>
<% items.each_with_index do |item, i| %>
<%= render view: 'row', locals: { item: item, index: i } %>
<% end %>
</tbody>
</table <%# row.erb %>
<tr data-index="<%= index %>" class="<%= index.even? ? 'even' : 'odd' %>">
<td><%= item.first_name %></td>
<td><%= item.last_name %></td>
</tr> Requirements to be solved
IdeasThere is the idea of allowing slot templates. This would solve the issues 2 and 3, but may need to be carefully implemented to allow polymorphism (issue 4). Issue 1 is currently possible. What I don't understand is, given there are slot templates, are multiple templates needed? AFAICT, slot templates would be strictly more featureful than multiple templates (as currently implemented). Another idea, which we didn't get the chance to discuss, is: keep the multiple templates approach, but allow specifying (in the component), what arguments does the template take: class MyComponent < ViewComponent::Base
template_arguments row: [:item, :color]
# This would cause the row template to be compiled into:
def call_row item:, color:
# compiled erb template here
end
end A basic implementation on top of the multiple-templates branch:diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb
index 2f31aaf..c928904 100644
--- a/lib/view_component/base.rb
+++ b/lib/view_component/base.rb
@@ -262,11 +262,21 @@ module ViewComponent
else
"#{call_method_name(template[:variant])}_#{pieces.first.to_sym}"
end
+ method_args =
+ # If the template matches the name of the component,
+ # set the method name with call_method_name
+ if pieces.first == name.demodulize.underscore
+ []
+ # Otherwise, append the name of the template to
+ # call_method_name
+ else
+ (@template_arguments || {}).fetch(pieces.first, []).map { |arg| "#{arg}:" }
+ end
undef_method(method_name.to_sym) if instance_methods.include?(method_name.to_sym)
class_eval <<-RUBY, template[:path], line_number
- def #{method_name}
+ def #{method_name}(#{method_args.join(", ")})
@output_buffer = ActionView::OutputBuffer.new
#{compiled_template(template[:path])}
end
@@ -276,6 +286,11 @@ module ViewComponent
@compiled = true
end
+ def template_arguments(**args)
+ @template_arguments ||= {}
+ @template_arguments.merge! args.stringify_keys
+ end
+
# we'll eventually want to update this to support other types
def type
"text/html"
diff --git a/test/app/components/multiple_templates_component.rb b/test/app/components/multiple_templates_component.rb
index b42e5a6..cb4973f 100644
--- a/test/app/components/multiple_templates_component.rb
+++ b/test/app/components/multiple_templates_component.rb
@@ -7,12 +7,14 @@ class MultipleTemplatesComponent < ViewComponent::Base
@items = ["Apple", "Banana", "Pear"]
end
+ template_arguments list: [:number], summary: [:string]
+
def call
case @mode
when :list
- call_list
+ call_list number: 1
when :summary
- call_summary
+ call_summary string: "foo"
end
end
end
diff --git a/test/app/components/multiple_templates_component/list.html.erb b/test/app/components/multiple_templates_component/list.html.erb
index 02bf027..767328f 100644
--- a/test/app/components/multiple_templates_component/list.html.erb
+++ b/test/app/components/multiple_templates_component/list.html.erb
@@ -1,4 +1,5 @@
<ul>
+ <%= number %>
<% @items.each do |item| %>
<li><%= item %></li>
<% end %>
diff --git a/test/app/components/multiple_templates_component/summary.html.erb b/test/app/components/multiple_templates_component/summary.html.erb
index ef9845a..6b988c1 100644
--- a/test/app/components/multiple_templates_component/summary.html.erb
+++ b/test/app/components/multiple_templates_component/summary.html.erb
@@ -1 +1 @@
-<div>The items are: <%= @items.to_sentence %></div>
+<div>The items are: <%= @items.to_sentence %>, <%= string %></div>
diff --git a/test/view_component/view_component_test.rb b/test/view_component/view_component_test.rb
index b6a8b7a..1ec3c97 100644
--- a/test/view_component/view_component_test.rb
+++ b/test/view_component/view_component_test.rb
@@ -552,10 +552,12 @@ class ViewComponentTest < ViewComponent::TestCase
assert_selector("li", text: "Apple")
assert_selector("li", text: "Banana")
assert_selector("li", text: "Pear")
+ assert_selector("ul", text: "1")
render_inline(MultipleTemplatesComponent.new(mode: :summary))
assert_selector("div", text: "Apple, Banana, and Pear")
+ assert_selector("div", text: "foo")
end
private |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
So, we chatted with @BlakeWilliams @camertron @joelhawksley and the way forward for now is:
I think these are the main conclusions of what we discussed. Please comment if I forgot something. I'll begin work on updating the PR along these lines. |
Hi! I saw @joelhawksley just landed a PR in rails, adding a syntax for defining the arguments a view template accepts: rails/rails#45602 . I would be happy to update #451 to allow this. |
I think because VC does not have the legacy AV does, we can be a bit more strict about the That being said, there is still one point which this new marker does not address, is how to expose the compiled method. AV requires you to call I'd like to summarize what I believe are the options:
|
@fsateler I'm leaning towards #1. However, @BlakeWilliams and I are starting to wonder whether we should implement this feature in Rails first, generating What do you think? |
@joelhawksley what sort of conflict are you envisioning? Creating rails view objects is not something you can (easily) do, so how would you access them? Or perhaps you are intending to define them with a slug of the full view path. That is, given a view |
Yes. And I'm not sure a namespace is a good way to go, as I don't think consumers would expect that behavior. I'd expect such global helpers to feel just like the existing Rails URL/path helpers. How about we do |
For VC that sounds good to me! Because the methods are scoped to the component object, there is no ambiguity between templates with the same name but in different components. Just so we are all on the same page, what I want to do is, given the following component:
I would generate the method |
Hi! I have updated #451 as discussed. Please take a look. |
What's the status of this excellent idea? |
Hi! @joelhawksley! Can I do anything to move this forward? I'd really like to go forward with this feature. |
I feel there's a lot of value in the proposal to add support for "sidecar partials", and have very similar use cases as described above. Excuse my ignorance with this question... |
@scottbarrow I am working on something to do exactly this, it's still early days but with a bit of luck it could work https://github.com/reeganviljoen/render_kit |
Thanks, looks like an interesting idea, but this seems to address a slightly different problem of providing a rendering helper, which is a departure from being able to render sidecar partials within a components directory, which I'd prefer to do without having to learn an additional DSL or register said partials |
I'm not sure how your proposal would work. Wouldn't that make the component partials "escape" the component and be accessible throughout the controller? I wouldn't want that. |
Feature request
Allow refactoring small view parts into a separate view, and be able to
render
them into the template. These views should not be available outside the component.Motivation
Sometimes it is useful to separate a sub-component from the component. For example, if I want to render a popover with extra info about a particular model, I would want to extract that popover into a separate view.
Alternative Solutions:
content_for
. This is not suitable if the partial needs to take arguments. In my example, it wouldn't work if I'm displaying a list of models and I want to show the popover for each model.Prior art:
ActionView does support partials, of course.
Cells does allow multiple views per cell, and allows using
render view: 'a_part'
.Possible solutions I see
render template:
with the full path. I would suspect this is relatively slow given thatrender
is slow in ActionView.extra_views_{view_name}
.The text was updated successfully, but these errors were encountered: