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

Support targets: in Broadcastable methods - Update #511

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

JuannFerrari
Copy link
Contributor

#210 added the ability to set targets on a stream tag. But that doesn't work nicely with the Broadcastable helper methods. Currently you have to do this to target some targets:

after_update_commit -> { broadcast_update_to self, target: nil, targets: ".class_name" }

This PR improves things so that you don't need to provide target: nil anymore.

--

This PR is a continuation of the work made by @ghiculescu at PR #408

hotwired#210 added the ability to set `targets` on a stream tag. But that doesn't work nicely with the `Broadcastable` helper methods. Currently you have to do this to target some `targets`:

```ruby
after_update_commit -> { broadcast_update_to self, target: nil, targets: ".class_name" }
```

This PR improves things so that you don't need to provide `target: nil` anymore.

--

This PR is a continuation of the work made by @ghiculescu at PR turbo-rails#408
@JuannFerrari JuannFerrari marked this pull request as ready for review November 9, 2023 16:39
@fguillen-getsafe
Copy link

Yes, please. It is just weird we have to desactivate target to be able to use targets. I would even say that the right way is using target for both cases, and always using a CSS query selector.

@dhh
Copy link
Member

dhh commented Sep 15, 2024

If you could rebase, I'd be happy to take this.

@JuannFerrari
Copy link
Contributor Author

@dhh rebase done :)

@dhh dhh merged commit 5199904 into hotwired:main Sep 16, 2024
15 checks passed
@JamesChevalier
Copy link

Is it intended for this to cause broadcast_remove_to to require render: false?

For example:
after_destroy -> { broadcast_remove_to("#{user_id}-activities") }
throws "ActionView::MissingTemplate: Missing partial" errors unless it's rewritten to
after_destroy -> { broadcast_remove_to("#{user_id}-activities", render: false) }

Changed in 5d6e604
Later rebased in 76f492a

@seanpdoyle
Copy link
Contributor

@JamesChevalier no, that should not be necessary.

Could you open a new issue to track the resolution of that problem?

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.

5 participants