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

turbo_stream tag builder: support :partial with block #701

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

seanpdoyle
Copy link
Contributor

The Background

Consider an application with a shared application/flash partial that accepts its message as a partial-local assignment:

<%# app/views/application/_flash.html.erb %>

<p role="alert"><%= message %></p>

Also consider an application/layout.turbo_stream.erb template for appending the Flash messages to a element with [id="flashes"]:

<%# app/views/layouts/application.turbo_stream.erb %>

<% flash.each do |name, message| %>
  <%= turbo_stream.append "flashes", partial: "application/flash", locals: { message: message } %>
<% end %>

This works as you'd expect, since the :partial and :locals keyword arguments are forwarded along to the underlying render call.

The Scenario

Now consider that the application/flash message changed its interface to expect the message as block content yielded to the partial instead of an assignment to the :locals options:

 <%# app/views/application/_flash.html.erb %>

-<p role="alert"><%= message %></p>
+<p role="alert"><%= yield %></p>

The layouts/application.turbo_stream.erb template would need to change as well:

 <%# app/views/layouts/application.turbo_stream.erb %>

 <% flash.each do |name, message| %>
-  <%= turbo_stream.append "flashes", partial: "application/flash", locals: { message: message } %>
+  <%= turbo_stream.append "flashes", partial: "application/flash" do %>
+    <span style="color: red"><%= message %></span>
+  <%= end %>
 <% end %>

The Problem

This style of invocation of turbo_stream.append does not work the same as if it were passed a block of content generated by calling render with the same keywords.

The presence of a &block argument triggers an entirely separate code path than the presence of the **rendering keywords.

To work around this issue, you'd have to capture the rendering separately:

 <%# app/views/layouts/application.turbo_stream.erb %>

 <% flash.each do |name, message| %>
-  <%= turbo_stream.append "flashes", partial: "application/flash", locals: { message: message } %>
+  <% content = capture do %>
+    <%= render partial: "application/flash" do %>
+      <span style="color: red"><%= message %></span>
+    <% end %>
+  <% end %>
+
+  <%= turbo_stream.append "flashes", content %>
 <% end %>

The Proposal

This commit alters the tag builder's decision making process to incorporate a check for a combination of both a &block and :partial or :layout keyword arguments.

The Background
---

Consider an application with a shared `application/flash` partial that
accepts its message as a partial-local assignment:

```erb
<%# app/views/application/_flash.html.erb %>

<p role="alert"><%= message %></p>
```

Also consider an `application/layout.turbo_stream.erb` template for
appending the `Flash` messages to a element with `[id="flashes"]`:

```erb
<%# app/views/layouts/application.turbo_stream.erb %>

<% flash.each do |name, message| %>
  <%= turbo_stream.append "flashes", partial: "application/flash", locals: { message: message } %>
<% end %>
```

This works as you'd expect, since the `:partial` and `:locals` keyword
arguments are forwarded along to the underlying `render` call.

The Scenario
---

Now consider that the `application/flash` message changed its interface
to expect the `message` as block content yielded to the partial instead
of an assignment to the `:locals` options:

```diff
 <%# app/views/application/_flash.html.erb %>

-<p role="alert"><%= message %></p>
+<p role="alert"><%= yield %></p>
```

The `layouts/application.turbo_stream.erb` template would need to change
as well:

```diff
 <%# app/views/layouts/application.turbo_stream.erb %>

 <% flash.each do |name, message| %>
-  <%= turbo_stream.append "flashes", partial: "application/flash", locals: { message: message } %>
+  <%= turbo_stream.append "flashes", partial: "application/flash" do %>
+    <span style="color: red"><%= message %></span>
+  <%= end %>
 <% end %>
```

The Problem
---

This style of invocation of `turbo_stream.append` does not work the same
as if it were passed a block of content generated by calling `render`
with the same keywords.

The presence of a `&block` argument triggers an entirely separate code
path than the presence of the `**rendering` keywords.

To work around this issue, you'd have to capture the rendering
separately:

```diff
 <%# app/views/layouts/application.turbo_stream.erb %>

 <% flash.each do |name, message| %>
-  <%= turbo_stream.append "flashes", partial: "application/flash", locals: { message: message } %>
+  <% content = capture do %>
+    <%= render partial: "application/flash" do %>
+      <span style="color: red"><%= message %></span>
+    <% end %>
+  <% end %>
+
+  <%= turbo_stream.append "flashes", content %>
 <% end %>
```

The Proposal
---

This commit alters the tag builder's decision making process to
incorporate a check for a combination of both a `&block` and `:partial`
or `:layout` keyword arguments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant