-
Notifications
You must be signed in to change notification settings - Fork 337
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
Remove morph broadcast helpers #647
Conversation
aa7617f
to
8ce8c7b
Compare
@@ -279,6 +283,10 @@ def broadcast_replace(**rendering) | |||
# # Sends <turbo-stream action="update" target="clearance_5"><template><div id="clearance_5">Other partial</div></template></turbo-stream> | |||
# # to the stream named "identity:2:clearances" | |||
# clearance.broadcast_update_to examiner.identity, :clearances, partial: "clearances/other_partial", locals: { a: 1 } | |||
# | |||
# # sends <turbo-stream action="update" method="morph" target="clearance_5"><template><div id="clearance_5">Other partial</div></template></turbo-stream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does the [method="morph"]
come from in these code samples?
Would the default value of the :method
attribute be "replace"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an optional attribute, updated the docs to reflect it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the default value would be replace but we don't need to explicitly pass it as you know! in case the method attribute is not present on the element the action will be processed as replace right?
test/streams/streams_channel_test.rb
Outdated
assert_broadcast_on "stream", turbo_stream_action_tag("morph", target: "message_1", template: render(options)) do | ||
Turbo::StreamsChannel.broadcast_morph_to "stream", target: "message_1", **options | ||
test "broadcasting actions with method morph now" do | ||
options = { method: :morph, partial: "messages/message", locals: { message: "hello!" } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When does :method
need to be nested within an :attributes
Hash option, and when can it be passed at the top level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a miss on my part it should always be an optional key under attributes.
Update unit test Update documentation
8ce8c7b
to
a298b86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @omarluq 🙏
I just tried implementing this after the release of 2.0.6 and wanted to share a challenge I had. The first thing I tried was adding <%= turbo_stream.update @dom_id, method: :morph do %>
… This didn't change the rendering of the tag, so I tried: <% turbo_stream_action_tag :update, target: @dom_id, method: :morph do %>
… Which truncated the content in the block (whether or not I wrapped it in This was the only formulation that worked: <%= tag.turbo_stream(action: :update, target: @dom_id, method: :morph) do %>
<template>…</template>
<% end %>
Path of least surprise: does it seem like the convenience methods of |
@searls from an ergonomics perspective, I think that special treatment of a If a view template relied on local assignments like |
Update: Whoops, my bad. That <%= tag.turbo_stream(action: :update, targets: @selector, method: :morph) do %>
<template>
…
</template>
<% end %> @seanpdoyle I'd leave up whether to pull up <%= turbo_stream.update @dom_id, attributes: {method: :morph} do %>
<%= turbo_stream.update_all @selector, attributes: {method: :morph} do %>
|
It's not super clear, but was this ever resolved? I'm struggling to find any way to render an |
Follow up to morph action API restructure by @seanpdoyle in hotwired/turbo#1240
This PR Removes morph stream action broadcast helpers and updates unit test and documentation for morph usage