-
Notifications
You must be signed in to change notification settings - Fork 16
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
Get rid of the <% yield p = np %> requirement. #1
Comments
I haven't looked at the related Rails internals at all, but perhaps there's some way of implementing something there? I'm not sure whether there would be performance implications if you invoked Nice Partials in every partial render indiscriminately. But you also wouldn't want to |
(But come on... it's funny. /cc @peterkeen) |
On the other hand, I also don't hate it. It serves a function and in conjunction with the explanation provided in the README, I feel like it actually demystifies what is going on. It makes really clear the idea of the "shared context object" or whatever we want to call it. |
Part of me thinks that if there's more magic, it could move away from being "just partials", but then perhaps I don't know enough about the internals to see what's possible?
<%# index.html.erb %>
<%= render 'path/to/partial' do |p| %>
<%= p.content_for :heading, 'Hello world' %>
Lorem ipsum
<%= p.content_for :footnotes, 'Foo bar' %>
<% end %> <%# _partial.html.erb %>
<% content = yield(p = np) %>
<h1><%= p.content_for :heading %></h1>
<%= content %>
<footer><%= p.content_for :footnotes %></footer> |
I suppose it doesn't matter where you call <%# index.html.erb %>
<%= render 'path/to/partial', p: np do |p| %>
<% p.content_for :heading, 'Hello world' %>
<% end %> <%# _partial.html.erb %>
<% yield p %>
<h1><%= p.content_for :heading %></h1> This might be more Rails-y 🤷♂️ Developers are familiar with defining locals, so it'd be reasonably understood to look for them there. Note: I don't think this would work when rendering collections because:
|
@domchristie From my perspective, I'd like to try and avoid the invoker of the partial from having to repeat any non-unique part of the set up whenever they use a partial, so I think |
I think I figured out conceptually where a right place to inject this all "into Rails" could be: Just override
This is getting into hacky territory, but one potential solution to those two problems would be to inspect the target partial and try to detect whether it's using Nice Partials. At first blush, this feels sort of wrong, error prone, and exactly the kind of magic people don't like. Not sure how to proceed. |
Ah, of course!
Oh, interesting. It looks like def _layout_for(*args, &block)
name = args.first
if block && !name.is_a?(Symbol)
p = NicePartials::Partial.new(self)
capture(*args, &-> { block.call(p) })
else
super
end
end Just need to work out:
I don't think the current approach is that bad and I like that it's transparent, but it's interesting to explore :) * some ideas: some kind of annotation within the partial or denoted by the file name? or perhaps nice partials should live in a specific directory? I'm not sure. |
Also, another benefit of an explicit <%= np do |p| %>
<% yield p %>
<%= p.content_for :heading %>
<% end %> def np(&block)
NicePartials::Partial.new(self).yield_self do |p|
capture(p, &block)
end
end Personally I still prefer the original one-liner, but it's something! |
Picking this thread back up, one idea I've had in the back of my mind is that we could do |
|
I haven't looked into how viable this is yet but Erubi, Rails' erb backend, supports a preamble that you theoretically could use to insert the |
@kaspth Amazing work on this one! Thank you! |
Quoting some feedback I received privately:
The text was updated successfully, but these errors were encountered: