-
Notifications
You must be signed in to change notification settings - Fork 329
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
Allow broadcasting without rendering and broadcasting with custom attributes #427
Allow broadcasting without rendering and broadcasting with custom attributes #427
Conversation
a291589
to
4bae00a
Compare
Broadcastable
4bae00a
to
f135fd4
Compare
Broadcastable
Broadcastable
Broadcastable
Broadcastable
8b8abf4
to
b1f16f0
Compare
Broadcastable
@seanpdoyle @dhh Hey there, it's been 4 months since I opened this PR. I know y'all are super busy, but I wonder if you could skim through this PR. It's a really simple change that creates a lot of flexibility and power. Basically it's allowing:
I'm unfamiliar with the repo's rules or usual wait times, so I'm unsure if this PR has been silently rejected or not. Appreciate your time and efforts spent bringing this project to life. |
I'm fine with the changes, but the test suite need to pass. Can you have a look at that? |
b1f16f0
to
f45749f
Compare
@dhh Done, tests are passing now! Thank you. |
@dhh I think I missed you by a week 🤦🏼♂️ , so just a friendly ping on this after 2 weeks. Wouldn't like to pester with notifications though, so LMK if I just need to be patient. |
@dhh Another friendly ping, test are passing 😃 |
Need to settle the commented out tests. Can't merge with that is it is. |
f45749f
to
baed1ef
Compare
@dhh All good now! (Had to call |
@dhh Another month has passed, so I'm feeling free to ping you on this 😃 All tests are passing and ready to merge! |
@dhh Pinging you on this again, hoping y'all are out of a cycle and have time to merge? 😃 P.S. I'm pinging on a ~3-4 weeks interval so as to not annoy you. Let me know if this is too frequent. |
…attributes In order to allow custom Turbo actions that don't require a template and work only via attributes passed to the turbo_stream_tag.
baed1ef
to
8e28570
Compare
@dhh Pinging once more 😶 Another cool usage thanks to this PR that I just added to my app. Printing from anywhere: # From anywhere
Current.venue.print(data) Thanks to: module TurboBroadcastable
def print(data:, to: self)
broadcast_action_later_to(
to,
action: :print,
render: false, # New in this PR
attributes: { # New in this PR
data: data.to_json
}
)
end
end class ApplicationRecord < ActiveRecord::Base
include TurboBroadcastable
end StreamActions.print = async function () {
const printerService = new PrinterService();
await printerService.print(this.getAttribute("data"));
} |
Description
Key Enhancements
This PR improves the flexibility of custom Turbo Stream actions by allowing:
Broadcastable
concern.Motivation
Thanks to the work on hotwired/turbo#479 and #373, custom
turbo_stream
actions can now be created. However, some limitations still exist (See here for an example of both current limitations):turbo_stream_action_tag
andturbo_stream
.Broadcastable
concern)turbo_stream
response on the controller or inside a view response (.turbo_stream.erb
) for a single user.Example
A custom stream action that doesn't need a template, only attributes (e.g. a toast):
Create a module with custom broadcast methods, just to encapsulate them and include them wherever we want:
Including it on
ApplicationRecord
so that we cantoast
to any model/streamable (as long as they're listening throughturbo_stream_from
.To be called anywhere, such as from a controller:
Notes
This change only affects
broadcast_action_
and relatedaction_
methods, since this is the method you'd use for custom actions.The default Javascript implementations for actions such as
prepend
,append
, etc. are provided by Turbo onStreamAction
and thus passing attributes here is unnecessary, since they won't be used.