[Turbo] Support custom TurboStreamResponse actions

[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)

[smnandre]

Last couple of minor suggestions..

Could you update the CHANGELOG in the src/Turbo repository ?

And let's merge this :)

[DRaichev]

Ready to merge

[smnandre]

Thank you @DRaichev !

[DRaichev]

Updated the changelog, now ready to merge

[smnandre]

Perfect!