-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Views of compound forms without children are not considered rendered #17580
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
Comments
@webmozart can you review this? if my proposed behavior gets accepted, i can provide a PR for it |
Thank you for reporting this issue @backbone87! :)
I don't find any information related to that in your description? What behavior do you propose? |
always consider compound forms that have no children as rendered idk, if this has any other implications, obviously its a behavior change and could alter the rendering results of existing code. |
@backbone87 This is already the case (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/FormView.php#L70). Your problem is that your form contains a child, namely "components". That child is never rendered though. Your problem can be fixed by passing the {{ form_end(form, {'render_rest': false}) }} I don't think there's anything we can fix on our side. |
@webmozart Its about inconsistency: using the tpl code from the OP, we observe 2 cases:
ok that description wasnt as good as i expected. ok, lets try this with another one: if form is not empty, both checks always return the same bool. but if the form is an empty compound form my check will consider it rendered, while the current one doesnt. |
a similar reasoning for checking compound vs. checking number of children is given here: |
compare this to: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/FormView.php#L68 my proposed change: public function isRendered()
{
if ($this->rendered) {
return true;
}
if (!$this->vars->compound) {
return false;
}
foreach ($this->children as $child) {
if (!$child->isRendered()) {
return false;
}
}
return $this->rendered = true;
} |
@backbone87 Sounds reasonable. The change should only affect |
and update on this? |
@backbone87 It seems that @webmozart said that it "sounds reasonable" so please open a PR :) |
…ndered implicitly
…ndered implicitly
…ndered implicitly
…d rendered implicitly (backbone87) This PR was merged into the 2.7 branch. Discussion ---------- [Form] compound forms without children should be considered rendered implicitly | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | possibly | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17580 | License | MIT | Doc PR | n/a Commits ------- 8234f2a #17580 compound forms without children should be considered rendered implicitly
* 2.8: [TwigBundle] added missing dependencies for tests fixed CS [TwigBundle] Adjust CacheWarmingTest for TemplateCacheWarmer introduced in 2.8 [TwigBundle] Fix CacheWarmingTest are order dependent Revert "bug #20080 [Form] compound forms without children should be considered rendered implicitly (backbone87)" Fix #19943 Make sure to process each interface metadata only once #17580 compound forms without children should be considered rendered implicitly
* 3.1: [TwigBundle] added missing dependencies for tests fixed CS adding missing dep [TwigBundle] Adjust CacheWarmingTest for TemplateCacheWarmer introduced in 2.8 [TwigBundle] Fix CacheWarmingTest are order dependent Revert "bug #20080 [Form] compound forms without children should be considered rendered implicitly (backbone87)" [2.7][VarDumper] Fix PHP 7.1 compat [2.8][VarDumper] Fix PHP 7.1 compat silent file operation to avoid open basedir issues Fix #19943 Make sure to process each interface metadata only once #17580 compound forms without children should be considered rendered implicitly
Well the title says it all.
If
form.components
is empty, it is not considered as rendered at the timeform_end
is called, causing its "container" markup to be rendered. This is especially annoying when you have expanded choices with an empty array of choices.This is partly related to: #17579
The text was updated successfully, but these errors were encountered: