[Form] Event form modification improvements #9254
Conversation
|
@vudaltsov you know Symfony Forms very well. If you have some time, could you please review this proposal? Thanks a lot! |
|
|
||
| The ``FormEvents::POST_SUBMIT`` event does not allow to modify the form | ||
| the listener is bound to, but it allows to modify its parent. | ||
|
|
There was a problem hiding this comment.
I think we can add it here. It supports this example https://symfony.com/doc/4.0/form/dynamic_form_modification.html#form-events-submitted-data.
There was a problem hiding this comment.
I'm not sure I understand your comment, what can we add here exactly?
There was a problem hiding this comment.
I just agree with your proposal! Let's add what you suggest :)
There was a problem hiding this comment.
Although it supports the example, I fail to see the logic behind it. Why does it work that way? It looks like a loophole (failure to verify that a form field is added when it's made from a children) that is used in a very common example.
form/events.rst
Outdated
| .. caution:: | ||
|
|
||
| At this point, you cannot add or remove fields to the form. | ||
|
|
There was a problem hiding this comment.
This was already discussed in symfony/symfony#20770
It doesn't make sense to change data on SUBMIT and POST_SUBMIT.
There was a problem hiding this comment.
This notice is not about changing the data but the form fields. Trying to do so only results in an exception being thrown on POST_SUBMIT event, not on SUBMIT event. Maybe an exception should be thrown on SUBMIT event too?
There was a problem hiding this comment.
Reading related discussions, it seems that SUBMIT event allowing to change form fields is intentional though it is suitable for a few edge cases only. What about updating the notice to say exactly that, rather than completely removing it?
| @@ -219,7 +215,8 @@ View data Normalized data transformed using a view transformer | |||
|
|
|||
| .. caution:: | |||
|
|
|||
| At this point, you cannot add or remove fields to the form. | |||
There was a problem hiding this comment.
Here I just make explicit that child fields cannot be changed on POST_SUBMIT, is that wrong?
There was a problem hiding this comment.
Sorry, misread the changes. This is correct.
|
Another related PR: #5996 |
|
@julienfalque do you think that your pull request is ready to merge? Thanks! |
|
@javiereguiluz I would like to have some feedback to #9254 (comment) before merging. |
|
I will give that feedback today, I will try to use those events the way you explain and we'll discuss it. |
|
I tried to add a field on a $builder = $this->createFormBuilder()
->add('children', FormType::class)
->add('submit', SubmitType::class, [
'attr' => ['class' => 'btn btn-black']
]);
$builder
->get('children')
->add('text', TextType::class)
->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) {
$event->getForm()->add('textarea', TextareaType::class);
});
$form = $builder
->getForm()
->handleRequest($request);
So I think the notice is correct and should not be removed. |
|
I noticed that ResizeFormListener actually removes fields from the listened form on |
5ecc632 to
d40285b
Compare
|
Ok, I reverted the notice removal. |
|
@julienfalque thanks for this contribution ... and thanks @vudaltsov for your great review. Cheers! |
This PR was merged into the 2.7 branch. Discussion ---------- [Form] Event form modification improvements * `FormEvents::SUBMIT` *does* allow to modify the form (tested on Symfony 3.3 but looking at the code, this seems to be true for 2.7 as well); * `FormEvents::POST_SUBMIT` allows to modify the parent form, this was not explicit in the documentation so I thought it would be a nice addition. Commits ------- d40285b Event form modifications improvements
FormEvents::SUBMITdoes allow to modify the form (tested on Symfony 3.3 but looking at the code, this seems to be true for 2.7 as well);FormEvents::POST_SUBMITallows to modify the parent form, this was not explicit in the documentation so I thought it would be a nice addition.