From df5bb4aefacaa4032bbf245f17a11464382f93ed Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 9 Jul 2012 16:09:12 +0200 Subject: [PATCH] [Form] Unified rendering of errors for nested elements --- .../views/Form/form_div_layout.html.twig | 16 ++--- .../views/Form/form_table_layout.html.twig | 27 +++---- .../Resources/views/Form/form_row.html.php | 4 +- .../Resources/views/Form/form_rows.html.php | 1 - .../views/Form/form_widget_compound.html.php | 7 ++ .../views/FormTable/form_errors.html.php | 72 ++++++------------- .../views/FormTable/form_row.html.php | 4 +- .../FormTable/form_widget_compound.html.php | 3 + src/Symfony/Component/Form/CHANGELOG.md | 1 + .../Form/Extension/Core/Type/FormType.php | 6 ++ .../Form/Tests/AbstractDivLayoutTest.php | 13 ++-- .../Form/Tests/AbstractTableLayoutTest.php | 18 ++--- 12 files changed, 73 insertions(+), 99 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig index 1d15e502b114..13ead741260c 100644 --- a/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig +++ b/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig @@ -20,6 +20,9 @@ {% block form_widget_compound %} {% spaceless %}
+ {% if form.parent is empty %} + {{ form_errors(form) }} + {% endif %} {{ block('form_rows') }} {{ form_rest(form) }}
@@ -236,6 +239,10 @@ {% block repeated_row %} {% spaceless %} + {# + No need to render the errors here, as all errors are mapped + to the first child (see RepeatedTypeValidatorExtension). + #} {{ block('form_rows') }} {% endspaceless %} {% endblock repeated_row %} @@ -244,13 +251,7 @@ {% spaceless %}
{{ form_label(form, label|default(null)) }} - {# - If the child is a compound form, the errors are rendered inside - the container. See also block form_rows. - #} - {% if not compound %} - {{ form_errors(form) }} - {% endif %} + {{ form_errors(form) }} {{ form_widget(form) }}
{% endspaceless %} @@ -298,7 +299,6 @@ {% block form_rows %} {% spaceless %} - {{ form_errors(form) }} {% for child in form %} {{ form_row(child) }} {% endfor %} diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig index fbfe96adcff8..27f830fe2d4f 100644 --- a/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig +++ b/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig @@ -7,31 +7,13 @@ {{ form_label(form, label|default(null)) }} - {% if not compound %} - {{ form_errors(form) }} - {% endif %} + {{ form_errors(form) }} {{ form_widget(form) }} {% endspaceless %} {% endblock form_row %} -{% block form_errors %} -{% spaceless %} - {% if not compound %} - {{ parent() }} - {% else %} - {% if errors|length > 0 %} - - - {{ parent() }} - - - {% endif %} - {% endif %} -{% endspaceless %} -{% endblock form_errors %} - {% block hidden_row %} {% spaceless %} @@ -45,6 +27,13 @@ {% block form_widget_compound %} {% spaceless %} + {% if form.parent is empty and errors|length > 0 %} + + + + {% endif %} {{ block('form_rows') }} {{ form_rest(form) }}
+ {{ form_errors(form) }} +
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php index 6f994156aad4..091807020d23 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php @@ -1,7 +1,5 @@
label($form, isset($label) ? $label : null) ?> - - errors($form) ?> - + errors($form) ?> widget($form) ?>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_rows.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_rows.html.php index a5f1dfbf5f17..8c3ba86f7a3a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_rows.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_rows.html.php @@ -1,4 +1,3 @@ -errors($form) ?> row($child) ?> diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget_compound.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget_compound.html.php index 705664b4b921..9321a69fa106 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget_compound.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget_compound.html.php @@ -1,4 +1,11 @@
renderBlock('widget_container_attributes') ?>> + hasParent() && $errors): ?> + + + errors($form) ?> + + + renderBlock('form_rows') ?> rest($form) ?>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_errors.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_errors.html.php index f1dbfc3486ed..339e3d000985 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_errors.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_errors.html.php @@ -1,51 +1,21 @@ - - - - - - 0): ?> - - - - - - - - - + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php index 03daaf19fcf4..b9e5c5639ce4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php @@ -3,9 +3,7 @@ label($form, isset($label) ? $label : null) ?> - - errors($form) ?> - + errors($form) ?> widget($form) ?> diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_widget_compound.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_widget_compound.html.php index 9c90f15e58fc..b02283060305 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_widget_compound.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_widget_compound.html.php @@ -1,4 +1,7 @@ renderBlock('widget_container_attributes') ?>> + hasParent()): ?> + errors($form) ?> + renderBlock('form_rows') ?> rest($form) ?>
diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index f468c38db29b..bddf1b8769e4 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -141,3 +141,4 @@ CHANGELOG * FormBuilder now implements \IteratorAggregate * [BC BREAK] compound forms now always need a data mapper * FormBuilder now maintains the order when explicitely adding form builders as children + * [BC BREAK] fixed rendering of errors for DateType, BirthdayType and similar ones diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index cb6421517173..706b92c9a0d3 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -175,6 +175,11 @@ public function setDefaultOptions(OptionsResolverInterface $resolver) return false !== $options['property_path']; }; + // Compound forms are not displayed inline + $inline = function (Options $options) { + return !$options['compound']; + }; + $resolver->setDefaults(array( 'data' => null, 'data_class' => $dataClass, @@ -195,6 +200,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver) 'virtual' => false, 'compound' => true, 'translation_domain' => null, + 'inline' => $inline, )); $resolver->setAllowedTypes(array( diff --git a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php index 0841f620163e..d4ae48ee4a4d 100644 --- a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php @@ -57,11 +57,13 @@ public function testRepeatedRow() $view = $form->createView(); $html = $this->renderRow($view); + // The errors of the form are not rendered by intention! + // In practice, repeated fields cannot have errors as all errors + // on them are mapped to the first child. + // (see RepeatedTypeValidatorExtension) + $this->assertMatchesXpath($html, -'/ul - [./li[.="[trans]Error![/trans]"]] - [count(./li)=1] -/following-sibling::div +'/div [ ./label[@for="name_first"] /following-sibling::input[@id="name_first"] @@ -373,7 +375,8 @@ public function testNestedFormError() $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./div/div[@id="name_child"][./ul/li[.="[trans]Error![/trans]"]] + ./div/label + /following-sibling::ul[./li[.="[trans]Error![/trans]"]] ] [count(.//li[.="[trans]Error![/trans]"])=1] ' diff --git a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php index 62ac603bb94f..801f6a8ae643 100644 --- a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php @@ -59,12 +59,12 @@ public function testRepeatedRow() /following-sibling::td [./input[@id="name_second"]] ] - [count(../tr)=3] /following-sibling::tr[@style="display: none"] [./td[@colspan="2"]/input [@type="hidden"] [@id="name__token"] ] + [count(../tr)=3] ' ); } @@ -76,12 +76,13 @@ public function testRepeatedRowWithErrors() $view = $form->createView(); $html = $this->renderRow($view); + // The errors of the form are not rendered by intention! + // In practice, repeated fields cannot have errors as all errors + // on them are mapped to the first child. + // (see RepeatedTypeValidatorExtension) + $this->assertMatchesXpath($html, '/tr - [./td[@colspan="2"]/ul - [./li[.="[trans]Error![/trans]"]] - ] -/following-sibling::tr [ ./td [./label[@for="name_first"]] @@ -95,12 +96,12 @@ public function testRepeatedRowWithErrors() /following-sibling::td [./input[@id="name_second"]] ] - [count(../tr)=4] /following-sibling::tr[@style="display: none"] [./td[@colspan="2"]/input [@type="hidden"] [@id="name__token"] ] + [count(../tr)=3] ' ); } @@ -235,9 +236,8 @@ public function testNestedFormError() $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr/td/table - [@id="name_child"] - [./tr/td/ul/li[.="[trans]Error![/trans]"]] + ./tr/td/ul[./li[.="[trans]Error![/trans]"]] + /following-sibling::table[@id="name_child"] ] [count(.//li[.="[trans]Error![/trans]"])=1] '