Explained a more simple method of dynamic form handling available since #8827 #2927
Conversation
|
could you please include the pull request format in your description? |
| @@ -412,8 +412,7 @@ sport like this:: | |||
| public function buildForm(FormBuilderInterface $builder, array $options) | |||
| { | |||
| $builder | |||
| ->add('number_of_people', 'text') | |||
| ->add('discount_coupon', 'text') | |||
| ->add('sport', 'entity', array(/* whatever is necessary to fetch the list of sports */)) | |||
There was a problem hiding this comment.
->add('sport', 'entity', array(...));|
@wouterj I'll apply your changes accordingly. Thanks for the review. However, I'm up for a git challenge now. I created the branch for #2928 based on the branch for this one, rebased onto 2.3 and added an additional commit for the extra changes. If I rewrite and rebase -i the changes you explained in this PR, how can I cleanly merge them into #2928 without all git hell breaking loose? :) |
|
Well, as said in #2928, you should create that PR after this one is merged. The reason is that you now have 2 commits doing the same thing in 2 branches. That'll confuse GIT. So, first do this PR. Let it review by @weaverryan and let him merge this one. After that, create a new PR adding new stuff for the 2.3 branch. I know it's a worse workflow, but I didn't found a better one yet. |
|
@wouterj ah right. I thought that after rebasing, the exact same commit would be in both PR's, but then I forgot that I had to fix merge conflicts in the rebase, which was amended, effectively creating a new commit. I'll wait for the 2.3 PR then |
| ; | ||
| $factory = $builder->getFormFactory(); | ||
|
|
||
| $builder->addEventListener( | ||
| FormEvents::PRE_SET_DATA, | ||
| function(FormEvent $event) use($factory){ | ||
| function(FormEvent $event) use ($factory) { |
There was a problem hiding this comment.
It's not really necessary to pass the factory here, no?
There was a problem hiding this comment.
Since the chapter is about dynamic form modification, you're going to need the $factory at some point, even though it isn't in the example. I've changed the example to use it though, to make it more clear
There was a problem hiding this comment.
Not necessarily, since Form::add() supports the same interface as FormBuilder::add().
There was a problem hiding this comment.
Ah! I thought that was a 2.3 feature but I see it's been there since 2.2. I'll update the examples
|
Thank you @Burgov! :) |
| .. versionadded:: 2.2.6 | ||
| If you add a POST_BIND listener to a form child, and add new children to the parent | ||
| from there, the Form component will detect the new field automatically and maps it | ||
| to the client data if it is available. |
There was a problem hiding this comment.
I think the versionadded directive can be omitted.
There was a problem hiding this comment.
Are you sure about this?
There was a problem hiding this comment.
As far as I understand this was just a bugfix and not a new feature. There are no other places in the docs where we mention specific bugfixes. So I see no reason to do it here.
There was a problem hiding this comment.
Interesting, I saw it as a new feature :) You could probably argue for both... I have removed the directive as suggested.
There was a problem hiding this comment.
👍
If you have a look at the release notes, you can see that it is listed there as a bugfix, too.
|
This is great - thanks @Burgov! I made some small tweaks at sha: b998f86 - there was one actual error, fixed here: b998f86#L0R501. I'm pretty confident in the change, but let me know if I'm wrong :). Cheers! |
symfony/symfony#8827
This PR explains the changes in the above mentioned PR.
Also I rewrote the chapter a bit, as it was pretty confusing. In the listener, the client data "event" was used to update the form in PRE_BIND, but in the example this field wasn't even added to the form, making the example broken.
cc @bschussek