Add optional method to TagBuilder for morph #665
Conversation
AlexKovynev
changed the title
AlexKovynev
force-pushed
the
AddMethodToTagBuilder
branch
from
836842c
to
6c9feb3
Compare
| @@ -77,8 +77,8 @@ def remove_all(targets) | |||
| # <%= turbo_stream.replace "clearance_5" do %> | |||
| # <div id='clearance_5'>Replace the dom target identified by clearance_5</div> | |||
| # <% end %> | |||
| def replace(target, content = nil, **rendering, &block) | |||
| action :replace, target, content, **rendering, &block | |||
| def replace(target, content = nil, method = nil, **rendering, &block) | |||
There was a problem hiding this comment.
maybe this should be a keyword argument instead of a positional argument so one can pass rendering options without specifying the method?
btw, thanks for opening this PR 🙌
| template = render_template(target, content, allow_inferred_rendering: allow_inferred_rendering, **rendering, &block) | ||
|
|
||
| turbo_stream_action_tag name, target: target, template: template | ||
| turbo_stream_action_tag name, target: target, template: template, **attributes_from_method(method) |
There was a problem hiding this comment.
I think there's no need for attributes_from_method, could simply be replaced by , method: method
AlexKovynev
force-pushed
the
AddMethodToTagBuilder
branch
4 times, most recently
from
e529329
to
82ae81e
Compare
| @@ -23,6 +23,7 @@ class Turbo::StreamsControllerTest < ActionDispatch::IntegrationTest | |||
| <turbo-stream action="replace" target="message_1"><template>#{render(message_1)}</template></turbo-stream> | |||
| <turbo-stream action="replace" target="message_1"><template>Something else</template></turbo-stream> | |||
| <turbo-stream action="replace" target="message_5"><template>Something fifth</template></turbo-stream> | |||
| <turbo-stream method="morph" action="replace" target="message_5"><template>Something fifth</template></turbo-stream> | |||
There was a problem hiding this comment.
Maybe also cover the action="update" case in exactly the same way?
There was a problem hiding this comment.
update not covered at all as you see but anyway added with morph :)
AlexKovynev
force-pushed
the
AddMethodToTagBuilder
branch
from
115a320
to
903eec4
Compare
|
Looks good to me! Eager to use this functionality in my rails project shortly. |
|
Thanks for this PR, @AlexKovynev Anything in particular holding it up at this point?? |
|
@dhh can it be merged somehow please? Cause this thing now missed when morph added. |
|
I'll release a new version with this one later today. |
continue of this #658