Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
[Cookbook][Dynamic Form Modification] Add AJAX sample#3411
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
yes |
| ->add('sport', 'entity', array(...)) | ||
| ->add('sport', 'entity', array( | ||
| 'class' => 'AcmeDemoBundle:Sport', | ||
| 'empty_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.
please line out the=> (same below)
Thanks for the review, issues are fixed now. Remaining question is if it's ok to use FOSJsRoutingBundle. |
@bicpi I vote yes to FOSJsRoutingBundle. I would like to see more instances where we basically say "this isn't core, but it's the standard tool people use to solve X". Obviously, we need to be careful to promote bundles that are agreed-upon. Fortunately, the FOS* bundles have kind of already gone through that process and are company-agnostic. So, I think it's an easy win to include it. Good proposal. |
| use Symfony\Bundle\FrameworkBundle\Controller\Controller; | ||
| use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; | ||
| use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template; |
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 like the@Route (very easy to understand, and short), but I'm not as excited about the@Template here. But, I may be splitting hairs. What do others think?
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.
Actually, I'm against using one of those. But if we use one of them, we can use them both imo.
@weaverryan For FOSJsRoutingBundle, I think it is fine to recommend it:#796 |
@bicpi I think this is ready to merge if you find some time for this last round of tweaks. I'm trying to get things perfect so that I can just hit the big merge button without making un-PR'ed commits of my own :). Thanks! |
I've updated this PR to the approach of submitting the whole form via AJAX. This means no JSON and no FOSJsRoutingBundle. I've also removed the ParamConverter, Route and Template annotations. Everything is reduced to the bare minimum now :-) Please let me now if I can improve the naming schema for the Also, do we need a more detailed note at the end, maybe mentioning that the validation for AJAX form submission needs to fail? |
@weaverryan Found another old but ready-to-merge PR, any further suggestions for this? |
Thanks for the poke - this is too good of an improvement to leave unmerged ;). Thans a lot for this! |
…bicpi)This PR was merged into the 2.3 branch.Discussion----------[Cookbook][Dynamic Form Modification] Add AJAX sample| Q | A| ------------- | ---| Doc fix? | no| New docs? | yes| Applies to | 2.3+| Fixed tickets |#2464 ("AJAX experience")Do we need a PHP version of the `create.html.twig` which would mainly contain redundant JavaScript?Commits-------a75ad9c Shorten AJAX exampleee33dcd Remove some empty lines from code samplesf47a7c3 Updates & Fixes after public review2533f29 [Cookbook][Dynamic Form Modification] Add AJAX sample
Do we need a PHP version of the
create.html.twigwhich would mainly contain redundant JavaScript?