-
-
Notifications
You must be signed in to change notification settings - Fork 368
[TwigComponent]: Add the ability to spread attributes from the ...
attribute
#2923
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
base: 2.x
Are you sure you want to change the base?
Conversation
foreach ($data as $key => $value) { | ||
if ($value instanceof \Stringable) { | ||
$data[$key] = (string) $value; | ||
} | ||
} |
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.
why this removal ?
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.
Because this loop force attributes to be strings if they are Stringable. However, StimulusAttributes implements Stringable so I can't pass recursively the object in the ComponentAttributes. Maybe I can exclude StimulusAttributes like this :
if (!$value instanceof \StimulusAttributes && $value instanceof \Stringable) {
?
@@ -103,6 +109,31 @@ public function __clone(): void | |||
$this->rendered = []; | |||
} | |||
|
|||
private function handleAttributes(array $attributes): array | |||
{ | |||
$spreadAttributes = $attributes[self::SPREADABLE_ATTRIBUTE] ?? null; |
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.
This check should be done in constructor to avoid method call for everyone not using this feature
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.
I didn't want to break the code guideline but ok, if it's allowed
|
||
unset($attributes[self::SPREADABLE_ATTRIBUTE]); | ||
|
||
return [...$attributes, ...$spreadAttributes]; |
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.
This is something that can be done userland, no ? Wether in PHP or Twig
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.
What do you mean ? The interest is to be recursive. Currently, for the "primary" attributes it's not that usefull but for nested attributes, we need that
I'm not a bin fan of hard-coding something that uses a Twig construct (the <twig:TableRow btn:...="{{ confirm() }}" /> I'm very surprised we allow "..." as attribute name to be honest, and think this is something we should fix. For your initial need, why don't you prefix your keys in your function ? Or pass it as prop instead as attribute ? Anything not working as expected here ? |
My first thought was to use the "*" attribute but I had to edit the PreLexer to allow this character, but I saw the "." was allowed so I was like... Ok, why not...
Here is my ConfirmExtension function : readonly class ConfirmExtension
{
public function __construct(
#[Autowire(service: 'stimulus.helper')]
private StimulusHelper $stimulusHelper,
) {
}
#[AsTwigFunction(name: 'confirm', isSafe: ['html_attr'])]
public function getConfirm(
string $question = 'Êtes-vous sûr de vouloir effectuer cette action ?',
string $confirmLabel = 'Confirmer',
string $cancelLabel = 'Annuler',
string $title = 'Confirmation',
): StimulusAttributes {
$attributes = $this->stimulusHelper->createStimulusAttributes();
$attributes->addController('admin--confirm');
$attributes->addAction(
'admin--confirm',
'showConfirm',
'click',
[
'message' => $question,
'confirm-label' => $confirmLabel,
'cancel-label' => $cancelLabel,
'title' => $title,
]
);
return $attributes;
}
} There is no way to pass prefix in the StimulusHelper. Maybe I could transform the And also, it's possible to have the functionality in the first instance of attributes, why not for the nested ones ? |
[TwigComponent]: Add the ability to spread attributes from the
...
attributeAs a front-end developer working primarily with Twig, I frequently create components that require nested attributes. However, there's currently no way to pass an array of attributes with a prefix.
For example, given a component:
I can currently write:
or:
But I can't do:
or:
That's why I'm proposing support for a new spread-like syntax using
...
to allow passing an array of attributes (array
,Traversable
, orStimulusAttributes
) with a prefix.Real-world use case:
I have a Twig extension (
App\Twig\Extension\ConfirmExtension
) that provides aconfirm()
function returning aStimulusAttributes
object.I also have a component
<twig:TableRow/>
that contains a delete button.Currently, I cannot write:
With this PR, my proposed syntax would allow the following:
This improves flexibility and developer experience when working with prefixed dynamic attributes in Twig components.