[Turbo] [TurboStreamResponse] Custom action support

[DRaichev]

Q	A
Bug fix?	no
New feature?	yes
Issues	-
License	MIT

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

[setineos]

I think it should also work for <twig:Turbo:Stream:*> components.

[nicolas-grekas]

I think it should also work for twig:Turbo:Stream:* components.

This can be done by adding a generic <twig:Turbo:Stream> component maybe.

[DRaichev]

@nicolas-grekas
I just realised if you pass "action" or "targets" into the attributes array those will conflict with the existing attributes. I think we should throw an exception if those are present

[smnandre]

With the incoming <twig:Turbo:Frame> and <twig:Turbo:Stream>, do you think we still need this features ?

[DRaichev]

With the incoming <twig:Turbo:Frame> and <twig:Turbo:Stream>, do you think we still need this features ?

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.

[smnandre]

Looks good to me!

Could you check the type of values and throw if not expected (a non-stringable object) ?

[DRaichev]

Looks good to me!

Could you check the type of values and throw if not expected (a non-stringable object) ?

I believe this is going to throw:

foreach ($attr as $key => $value) {
    $key = htmlspecialchars($key);
    if (null === $value) {
        $attrString .= \sprintf(' %s', $key);
    } elseif (\is_int($value) || \is_float($value)) {
        $attrString .= \sprintf(' %s="%s"', $key, $value);
    } else {
        $attrString .= \sprintf(' %s="%s"', $key, htmlspecialchars($value)); // htmlspecialchars expect string so will throw here
    }
}

Or do you mean somewhere else ?

[DRaichev]

Looks good to me! Could you check the type of values and throw if not expected (a non-stringable object) ?

I believe this is going to throw:

foreach ($attr as $key => $value) {
    $key = htmlspecialchars($key);
    if (null === $value) {
        $attrString .= \sprintf(' %s', $key);
    } elseif (\is_int($value) || \is_float($value)) {
        $attrString .= \sprintf(' %s="%s"', $key, $value);
    } else {
        $attrString .= \sprintf(' %s="%s"', $key, htmlspecialchars($value)); // htmlspecialchars expect string so will throw here
    }
}

Or do you mean somewhere else ?

So this is ready for merge then, right

[smnandre]

After some thinking, why don't we rename the custom($action) method in action($name, .... ... ... .. ) ?

[DRaichev]

After some thinking, why don't we rename the custom($action) method in action($name, .... ... ... .. ) ?

Yeah, I was thinking about this when I renamed the one in TurboStreamResponse It should be either custom in both or action in both.

I don't really have a preference, both work, so I leave the choice to you

[smnandre]

Let's use action in both then (sorry for the late comment)

[DRaichev]

Ready to merge

[DRaichev]

Updated the changelog, now ready to merge

[DRaichev]

@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 now I'm waiting for v2.22 to do the <twig:Turbo:Frame/> and hopefully we can get this as well so I can do the instances of custom turbo actions.

[Kocal]

Hey, I rebasing your PR and will merge it in the next minutes.

[Kocal]

Thank you @DRaichev.

[smnandre]

@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 😄

[DRaichev]

@Kocal @smnandre Thanks for the speedy reaction. And thank you for all the work you're doing on this project. I used to hate to work on UX stuff, but thanks to Turbo and Symfony UX I realised you can have great UX without the convoluted mess. Keep up the good work.

[smnandre]

Thank you for such a kind message—it really means a lot!