-
-
Notifications
You must be signed in to change notification settings - Fork 398
[Turbo] Support custom TurboStreamResponse actions #2298
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
Conversation
13122e2 to
8514346
Compare
|
I think it should also work for |
This can be done by adding a generic |
|
@nicolas-grekas |
3e22298 to
66f7e36
Compare
|
With the incoming |
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Turbo] Add generic `<Turbo:Stream>` component | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Issues | #2298 (comment) | License | MIT I added a generic Turbo Stream component. To use it, you can do this: ```twig <twig:Turbo:Stream action="turbo_frame_reload" targets="#count-post" /> ``` The rendering is: ```twig <turbo-stream action="turbo_frame_reload" targets="#count-post"></turbo-stream> ``` Commits ------- f0ab499 [Turbo] Add generic `<Turbo:Stream>` component
I think they are complementary. I'd rather use the twig component syntax only in templates and stick to this helper on the php side, plus it's better to have consistent functionality in both, especially for people who dislike the twig component syntax. |
23a9740 to
03629f7
Compare
|
After some thinking, why don't we rename the |
Yeah, I was thinking about this when I renamed the one in TurboStreamResponse I don't really have a preference, both work, so I leave the choice to you |
|
Let's use action in both then (sorry for the late comment) |
bb8aeed to
488e061
Compare
smnandre
left a comment
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.
Last couple of minor suggestions..
Could you update the CHANGELOG in the src/Turbo repository ?
And let's merge this :)
8c46bf3 to
1b1aa6e
Compare
|
Ready to merge |
smnandre
left a comment
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 @DRaichev !
4b62702 to
6c3a3b7
Compare
|
Updated the changelog, now ready to merge |
smnandre
left a comment
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.
Perfect!
|
@smnandre Hey, can we get this merged ? I am in the process of replacing the usage of the old syntax at work. I've already replaced all basic cases, so |
|
Hey, I rebasing your PR and will merge it in the next minutes. |
6c3a3b7 to
d29e4d9
Compare
|
Thank you @DRaichev. |
|
@DRaichev we will probably release a 2.22 in the coming weeks, you can use it with the "2.x-dev" in the meanwhile! Thanks for this PR 😄 |
|
Thank you for such a kind message—it really means a lot! |
The current implementation of TurboStream & TurboStreamResponse has no generic action (only the predefined turbo actions) and what's more the class is marked as final, which does not allow adding support for more actions. I think the wrap function should be public or have a public function "custom" that exposes the functionality (I've opted for the latter)
I really like this library: https://github.com/marcoroth/turbo_power
It adds additional simple actions. For example I use set/delete attributes to enable/disable UI elements
This change will allow people to use this or similar libraries, or even build their own custom actions if they want to.
There is no impact to the rest of the functionality or DX
There is a minor change to the wrap function, it now accepts an array of attributes instead of a string. It builds the string from the array of attributes, and adds a leading space. This aims to prevent errors, as the previous implementation needs the $attr argument to start with a blank space otherwise it would produce invalid html