ux
ux copied to clipboard
UX FormCollection
Q | A |
---|---|
Bug fix? | no |
New feature? | yes |
Tickets | Fix #3 |
License | MIT |
Handle form collections, add & remove buttons
Thanks @weaverryan, for taking the time to the review. I'm intimidated by you because I know you through SymfonyCast ;-)
This is my first contribution, but you are all very kind to help me!
Thanks @weaverryan, for taking the time to the review. I'm intimidated by you because I know you through SymfonyCast ;-)
Well, I'm happy to know you know too ;)
This is my first contribution, but you are all very kind to help me!
It's a very big feature for a first contribution - nice work!
Hi @stakovicz !
Thanks very much for your PR, as Ryan I'm quite excited by this as it's a major requirement for Symfony Forms.
I think, as Ryan mentioned, the only point where I think we need to spend some time thinking is how templates are handled (ie. what HTML is generated). Generally speaking, Symfony as a framework has helpers that generate several HTML tags (like in forms) but usually these helpers become more complex to work with than the original HTML. This was the reasoning behind https://symfony.com/blog/new-in-symfony-5-2-form-field-helpers for instance: use inline attribute helpers instead of tags helpers.
Perhaps your solution is the best, I don't know yet as I didn't have a deep look into it, but it' worth spending some time: it will have a huge impact on developer experience.
I'll have a deeper look asap :) .
@stakovicz First I want to thank you for doing this. I have projects with deeply nested collections, this addition is more than welcome,
I have one question about the problem I encountered few times and suggested jquery.collection plugin suffered from it; I had to make my own implementation because of that.
TL;DR Will it be possible to affect how the index is increased in this line?
Or better: a way to set base index not as the number of existing, but for example seconds since Unix epoch?
Reason, click to expand
Imagine editing (create is not a problem) a form with a collection that has 3 Users.
So Symfony renders them with keys 0, 1, 2.
Now you delete all of them and add new User. Submitted array to the server will only have key 3; so far it is good, Symfony knows you deleted first 3 and added new one.
Here comes the problem. Validation fails and the form is rendered again, with that one user (the new one). You add 2 new users but instead of creating keys 4 and 5, the suggested Symfony plugin will reset keys.
The problem is that now backend will think that data has been updated (because submitted keys are the same as when form was first created), and not know that old users were deleted and new ones were created.
Keys are very important here; default mapper is sort-of doing some bypass by searching for objects (mostly entities) but I have more extreme cases.
I solved it by making the base index not to be the count of existing elements, but the seconds of the current time. Submitted data has big keys but no problem since that.
Any thoughts?
@zmitic
First I want to thank you for doing this. I have projects with deeply nested collections, this addition is more than welcome,
Thanks a lot !
... Any thoughts?
OK I see the problem. I watch what I can do.
@zmitic this commit allow you to set the default index : https://github.com/symfony/ux/pull/90/commits/8b23e78ac07447a0109eef0fe5abbfe7991ea7ef
The value is set automatically if you use a predefined theme.
@stakovicz Perfect! I will find a way to override it, still didn't figure that for Stimulus.
Thank you.
@weaverryan @tgalopin do you have any feedback ?
To be honest, I tried making - ahem, for my self - some re-usable javascript code to handle Collections. But every time I try to use it, my use case for a Collection is slightly different, and I end up customizing a lot anyway.
The HTML changes per project (sometimes only widgets in a table, sometimes a 'normal' bootstrap horizontal form, then again something different, then the keys are numeric and have no meaning, then the keys are identifiers which need to be precise, etc...).
Now aboard the Stimulus hype-train, I have a simple 'collection_controller' that handles the add-button and delete-button code. With a delete-button with a data-action=
attribute and a simple data-remove-key=
attribute, I have enough that all my everytime-it-is-slightly-different templates can work.
I make a macro that handles a single child of the collection. That macro is then also usable to render a prototype string (simply calling the macro in an html-attribute will escape it properly) and for me it's easier to write a small template for the collection-at-hand then try to make a 'one size fits all template'.
So personally, I wouldn't bother with trying to come up with some template-style that fits all... because it never will in my experience.
Include a 'simple' piece of form theme that uses existing form_row() and form_label() so that it is as generic as possible, but make sure the stimulus controller doesn't depend on it hard. And include a little bit of 'demo' template for people whishing to do it their way.
This way Symfony has - ahem, finally - working out-of-the-box javascript for Collections :), but it's still easy enough for people who know how to customize a collection to use a partial-twig as form-theme and just render it there how they see fit.
I am interested :)
Hi,
With example below, data-ux-form-collection-prototype-value
is badly encoded because there is html_attr on value : https://github.com/symfony/webpack-encore-bundle/blob/main/src/Twig/StimulusTwigExtension.php#L69
{% macro commentFormRow(commentForm) %}
<div
class="col-4"
data-symfony--ux-form-collection--collection-target="entry"
>
{{ form_errors(commentForm) }}
{{ form_row(commentForm.content) }}
{{ form_row(commentForm.otherField) }}
<a data-action="symfony--ux-form-collection--collection#delete">
Remove
</a>
</div>
{% endblock %}
<div
class="row"
{{ stimulus_controller('symfony/ux-form-collection/collection', {
prototype: _self.commentFormRow(form.comments.vars.prototype),
}) }}
>
{% for commentForm in form.comments %}
{{ _self.commentFormRow(commentForm) }}
{% endfor %}
<a data-action="symfony--ux-form-collection--collection#add">
Add Another
</a>
</div>
Hello @stakovicz
Sorry for the delayed answer, I didn't have much time to work on UX lately. I plan to spend more time on it in the coming weeks.
As Ryan explained, I think we need to find a way to tackle the templating, especially for custom needs. I'll have a look at your example repository and see if there are interesting ideas we could have.
@stakovicz any progress on this to finish it? Would love to use this.
I like @alexander-schranz's ideas, and I think if we can implement those, I would support this. @stakovicz you've done a lot of great work on this, but we've been slow to review. Are you still able/interested to finish this?
Thx :)
Yes, I'm in ! I think can do the last changes.
Friendly ping - let us know if and how we can help, or if anything is unclear :)
Good evening Ryan,
Thanks for the reminder, everything is clear. However, at the moment, I can't find time for this project.
If you wish, you can continue.
Best regards,
Totally understandable :). If anyone else wants to continue this PR, please feel free to take this code, build on top of it, and submit a new PR. I would like it to be finished - it's a very nice idea.
I'm currently trying based on this implement and the feedback trying to implemented a new variation of the UXCollectionType which does not requiring even .html.twig
to solve also the comment about the overwrite the collection_widget
block.
I'm nearly there to remove all .html.twig
and set all required attributes and even the buttons inside the form types. I sadly has one thing I could not yet solve and that is I need to wrap the entry_type
to add the remove button there, for this I'm doing the following in the UXCollectionType:
$entryTypeNormalizer = function (OptionsResolver $options, $value) {
return UXCollectionEntryType::class; // overwrite the given entry_point with wrapped component
};
$entryOptionsNormalizer = function (OptionsResolver $options, $value) {
return [
'entry_type' => $options['entry_type'], // TODO get the original entry_type here
'entry_options' => $value, // forwards options
];
};
$resolver->addNormalizer('entry_type', $entryTypeNormalizer);
$resolver->addNormalizer('entry_options', $entryOptionsNormalizer);
The problem is I'm loosing the original given 'entry_type'
option this way. Which I need to forward to my UXCollectionEntryType. Sure we could go with ux_entry_type
as option but that would in my case be a bad developer experience. So is there any way we get the original value here or somebody have another idea here?