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

Clarify how to render the "default content" passed to a partial #29

Closed
adamlogic opened this issue Jun 29, 2022 · 6 comments
Closed

Clarify how to render the "default content" passed to a partial #29

adamlogic opened this issue Jun 29, 2022 · 6 comments

Comments

@adamlogic
Copy link

I ran into a bit of confusion integrating Nice Partials into some existing partials that were calling <%= yield %>.

This raised an error after I added the p block variable into the caller because p is nil when calling yield without an argument:

    <%= render "fieldset", legend: "Autoscaling" do |p| %>
      <% p.content_for :actions do %>
        TEST
      <% end %>

      ... the main content

The solution (I think) is to capture the result of yield p = np and use it instead of calling yield again:

<% content = yield(p = np) %>

<div class="divide-y pb-6">
  <div class="flex justify-between items-center">
    <legend class="text-2xl font-medium pb-6"><%= legend %></legend>

    <% if p.content_for? :actions %>
      <%= p.yield :actions %>
    <% end %>
  </div>

  <%= content %>
</div>

Assuming I'm following the recommended approach here, I think this probably just needs a little documentation in the README.

@kaspth
Copy link
Contributor

kaspth commented Jul 3, 2022

In #32 we just added the ability to remove yield(p = np) and just refer to p directly (note: we're debating other names in #36)

To your question, in #32, we also added p.output_buffer which has the captured output like your content = yield(p = np) does. So on main you're able to do:

<div class="divide-y pb-6">
  <div class="flex justify-between items-center">
    <legend class="text-2xl font-medium pb-6"><%= legend %></legend>

    <% if p.content_for? :actions %>
      <%= p.yield :actions %>
    <% end %>
  </div>

  <%= p.output_buffer %>
</div>

So I'll close this as fixed for now.

@kaspth kaspth closed this as completed Jul 3, 2022
@adamlogic
Copy link
Author

👏 Thanks for the detailed follow-up!

Any thoughts on calling p.yield with no arguments instead of (or in addition to) p.output_buffer? This would align nicely with how yield is used in Rails layouts, and it would feel a bit more "supported".

By "supported", I mean that calling output_buffer feels a bit like I'm going off the happy path and using an internal API. Perhaps that's intentional?

@adamlogic
Copy link
Author

I just pointed my app to the main branch here so I could try out this change, and I'm having a problem when calling the partial with no arguments.

Here is my (simplified) partial:

<div>
  <%= p.yield :actions %>
  <%= p.output_buffer %>
</div>

When I render this partial with an argument (render "fieldset" do |p|) it works as expected, but when I render without an argument (render "fieldset" do), p.output_buffer does not output the content.

It does work as expected when I use the old style:

<% content = yield(p = np) %>

<div>
  <%= p.yield :actions %>
  <%= content %>
</div>

@seanpdoyle
Copy link
Collaborator

Any thoughts on calling p.yield with no arguments instead of (or in addition to) p.output_buffer? This would align nicely with how yield is used in Rails layouts, and it would feel a bit more "supported".

I was going to suggest the same method name! I'm in favor of this proposal.

When I render this partial with an argument (render "fieldset" do |p|) it works as expected, but when I render without an argument (render "fieldset" do), p.output_buffer does not output the content.

The changes made in #32 hinge on a check on the number of arguments passed to the partial's block: when its arity is one, the block is made available, otherwise, it isn't.

I believe the check aims prevent breaking backwards compatibility and tries to avoid disrupting the default partial rendering process with extra or conflicting variables names.

seanpdoyle added a commit to seanpdoyle/nice_partials that referenced this issue Jul 3, 2022
When called without a `name` argument, treat `Partial#yield` as an
`output_buffer` reader. Along with that change, make `output_buffer` a
`Partial`-private attribute accessor.

bullet-train-co#29 (comment)
seanpdoyle added a commit to seanpdoyle/nice_partials that referenced this issue Jul 3, 2022
Related to [bullet-train-co#29][]

When called without a `name` argument, treat `Partial#yield` as an
`output_buffer` reader. Along with that change, make `output_buffer` a
`Partial`-private attribute accessor.

[bullet-train-co#29]: bullet-train-co#29 (comment)
@kaspth
Copy link
Contributor

kaspth commented Jul 3, 2022

Hm, I'm not quite liking p.yield because all this yield stuff is already confusing enough as is. I think I'd rather have p.captured_content or similar.

That said, for @seanpdoyle's last example here #27 (comment), it would be interesting if you could do:

<%# app/views/application/_form.html.erb %>
<% form_options = local_assigns.slice(:model, :url, :id, :data, :html) %>

<%= form_with some_magic_helper_that_can_deeply_merge_attributes_and_tokens(form_options, data: { controller: "form", action: "submit->form#doSomethingSpecial" }) do |form| %>
  <% p.yield form %> <%# <- Call `capture(form, p, &block)` and pass the form along, also passing `p` as the second argument %>
  <%= p.yield :controls %>

  <div>
    <%= link_to "Cancel", :back %>
    <%= p.yield :actions %>
  </div>
<% end %>

<%# elsewhere %>

<%= render "form", model: @post, data: { controller: "special-form", action: "reset->special-form#resetInASpecialWay" } do |form, builder| %>
  <%= builder.controls do %>
    <%= form.label :name %>
    <%= form.text_field :name %>
    <%# ... %>
  <% end %>

  <%= builder.actions do %>
    <%= form.button "Reset", type: "reset" %>
    <%= form.button "Submit" %>
  <% end %>
<% end %>

@kaspth
Copy link
Contributor

kaspth commented Jul 3, 2022

Ok, seeing yield in action in #41, I'm good with it.

seanpdoyle added a commit to seanpdoyle/nice_partials that referenced this issue Jul 4, 2022
Related to [bullet-train-co#29][]

When called without a `name` argument, treat `Partial#yield` as an
`output_buffer` reader. Along with that change, make `output_buffer` a
`Partial`-private attribute accessor.

[bullet-train-co#29]: bullet-train-co#29 (comment)

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
seanpdoyle added a commit to seanpdoyle/nice_partials that referenced this issue Jul 4, 2022
Related to [bullet-train-co#29][]

When called without a `name` argument, treat `Partial#yield` as an
`output_buffer` reader. Along with that change, make `output_buffer` a
`Partial`-private attribute accessor.

[bullet-train-co#29]: bullet-train-co#29 (comment)

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
seanpdoyle added a commit to seanpdoyle/nice_partials that referenced this issue Jul 27, 2022
Related to [bullet-train-co#29][]

When called without a `name` argument, treat `Partial#yield` as an
`output_buffer` reader. Along with that change, make `output_buffer` a
`Partial`-private attribute accessor.

[bullet-train-co#29]: bullet-train-co#29 (comment)

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
seanpdoyle added a commit to seanpdoyle/nice_partials that referenced this issue Jul 27, 2022
Related to [bullet-train-co#29][]

When called without a `name` argument, treat `Partial#yield` as an
`output_buffer` reader. Along with that change, make `output_buffer` a
`Partial`-private attribute accessor.

[bullet-train-co#29]: bullet-train-co#29 (comment)

Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
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