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

Layout attribute #2173

Open
eelcoj opened this issue Dec 1, 2024 · 5 comments
Open

Layout attribute #2173

eelcoj opened this issue Dec 1, 2024 · 5 comments

Comments

@eelcoj
Copy link
Contributor

eelcoj commented Dec 1, 2024

Feature request

I want to check how you feel about adding support for a "layout" attribute in ViewComponent. This should work similarly to Rails partials' layout attribute:

<%= render partial: "user", layout: "shared/admin" %>

This would wrap the user partial with app/views/shared/_admin.html.erb.

For ViewComponent this could be:

<%= render(UserComponent.new(user: @user, layout: AdminComponent)) %>

Motivation

This would come in handy most often when rendering a collection. The ViewComponent renders the collection (eg. li, but not the wrapping element (eg. <ul>).

The only mention of this seems to be here: #198

if this sounds interesting, I am happy to spend some time on it.

@reeganviljoen
Copy link
Collaborator

reeganviljoen commented Dec 2, 2024

@eelcoj, this is possible with current features, like the slot api for example

<%= render(AdminComponent.new()) do |c| %>
  <% c.with_body do %>
     <%= render(UserComponent.new(user: @user) %>
  <% end %> 
<% end %> 

all that said the example above is a lot more dense and could possibly benefit from a layout api, I would like to however ask @joelhawksley what his thoughts are.

@joelhawksley
Copy link
Member

@eelcoj thank you for taking the time to share your idea here!

I honestly had never heard of the layout option for rendering partials before you mentioned it. Unfortunately, ViewComponent does not control the initializer of ViewComponent::Base subclasses, so the API you proposed is not possible. I could see a couple of options:

Add with_layout macro:

render(MyComponent.new(arg: "foo").with_layout(MyLayout)

Add layout option to with_collection only:

render(CollectionComponent.with_collection(@items, layout: "foo")

Either way, we'd probably have to support both template file and component layouts. My instinct is to do with_layout. What do you think?

@joelhawksley
Copy link
Member

@eelcoj @reeganviljoen thinking about this some more, I realized that we could probably just rely on the built-in Rails API:

render(MyComponent.new, layout: "foo")

To support this, we'll need to make an update to Rails core here: https://github.com/rails/rails/blob/a34fc56f9170dea2b0dfd2a6138c2b57bab35cda/actionview/lib/action_view/helpers/rendering_helper.rb#L149 to pass the layout option to render_in.

I hope to be able to work on a patch for this soon 🤞🏻

@eelcoj
Copy link
Contributor Author

eelcoj commented Jan 8, 2025

@joelhawksley That would be even cooler! Let me know if I can help in any way!

@reeganviljoen
Copy link
Collaborator

@joelhawksley Agreed this seems like a great idea, and also let me know if you need any help getting a patch done

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

No branches or pull requests

3 participants