From 8b727664733027fa31ddd49dcf2c0fd4db098dae Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 16 Jul 2012 16:56:02 +0200 Subject: [PATCH] [Form] Tweaked the generation of option tags for performance (PHP +200ms, Twig +50ms) --- .../Bridge/Twig/Extension/FormExtension.php | 6 +-- .../views/Form/form_div_layout.html.twig | 13 +++--- .../Form/choice_widget_collapsed.html.php | 4 +- .../views/Form/choice_widget_options.html.php | 10 ++--- .../Templating/Helper/FormHelper.php | 11 ++--- .../Extension/Core/ChoiceList/ChoiceList.php | 44 +++++++++---------- .../Core/ChoiceList/SimpleChoiceList.php | 6 ++- src/Symfony/Component/Form/FormRenderer.php | 17 ++----- .../Component/Form/FormRendererInterface.php | 15 ++----- .../Component/Form/Tests/FormRendererTest.php | 32 +------------- 10 files changed, 50 insertions(+), 108 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Extension/FormExtension.php b/src/Symfony/Bridge/Twig/Extension/FormExtension.php index f7206bd5f80c..f67c3e17538e 100644 --- a/src/Symfony/Bridge/Twig/Extension/FormExtension.php +++ b/src/Symfony/Bridge/Twig/Extension/FormExtension.php @@ -69,8 +69,6 @@ public function getFunctions() 'form_row' => new \Twig_Function_Method($this, 'renderer->renderRow', array('is_safe' => array('html'))), 'form_rest' => new \Twig_Function_Method($this, 'renderer->renderRest', array('is_safe' => array('html'))), 'csrf_token' => new \Twig_Function_Method($this, 'renderer->renderCsrfToken'), - '_form_is_choice_group' => new \Twig_Function_Method($this, 'renderer->isChoiceGroup', array('is_safe' => array('html'))), - '_form_is_choice_selected' => new \Twig_Function_Method($this, 'renderer->isChoiceSelected', array('is_safe' => array('html'))), ); } @@ -80,7 +78,9 @@ public function getFunctions() public function getFilters() { return array( - 'humanize' => new \Twig_Filter_Method($this, 'renderer->humanize'), + 'humanize' => new \Twig_Filter_Method($this, 'renderer->humanize'), + 'is_choice_group' => new \Twig_Filter_Function('is_array', array('is_safe' => array('html'))), + 'is_choice_selected' => new \Twig_Filter_Method($this, 'renderer->isChoiceSelected', array('is_safe' => array('html'))), ); } 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 13ead741260c..a088ae533718 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 @@ -86,15 +86,14 @@ {% block choice_widget_options %} {% spaceless %} - {% for index, choice in options %} - {% if _form_is_choice_group(choice) %} - - {% for nested_choice in choice %} - - {% endfor %} + {% for group_label, choice in options %} + {% if choice|is_choice_group %} + + {% set options = choice %} + {{ block('choice_widget_options') }} {% else %} - + {% endif %} {% endfor %} {% endspaceless %} diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php index dd3655b733ab..0bc61c4525da 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php @@ -4,10 +4,10 @@ > 0): ?> - block('choice_widget_options', array('options' => $preferred_choices)) ?> + block('choice_widget_options', array('choices' => $preferred_choices)) ?> 0 && null !== $separator): ?> - block('choice_widget_options', array('options' => $choices)) ?> + block('choice_widget_options', array('choices' => $choices)) ?> diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php index d099b41f0a55..ebcff3973a5f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php @@ -1,11 +1,9 @@ - $choice): ?> - isChoiceGroup($choice)): ?> + $choice): ?> + - - - + block('choice_widget_options', array('choices' => $choice)) ?> - + diff --git a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php index 15eed3b9153f..35dbe93ba51e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php +++ b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php @@ -49,14 +49,9 @@ public function getName() return 'form'; } - public function isChoiceGroup($label) + public function isChoiceSelected(ChoiceView $choice, $selectedValue) { - return $this->renderer->isChoiceGroup($label); - } - - public function isChoiceSelected(FormView $view, ChoiceView $choice) - { - return $this->renderer->isChoiceSelected($view, $choice); + return $this->renderer->isChoiceSelected($choice, $selectedValue); } /** @@ -176,7 +171,7 @@ public function rest(FormView $view, array $variables = array()) */ public function renderBlock($block, array $variables = array()) { - return $this->block($block, $variables); + return $this->renderer->renderBlock($block, $variables); } /** diff --git a/src/Symfony/Component/Form/Extension/Core/ChoiceList/ChoiceList.php b/src/Symfony/Component/Form/Extension/Core/ChoiceList/ChoiceList.php index 80abc7342955..4059757b854a 100644 --- a/src/Symfony/Component/Form/Extension/Core/ChoiceList/ChoiceList.php +++ b/src/Symfony/Component/Form/Extension/Core/ChoiceList/ChoiceList.php @@ -240,27 +240,19 @@ public function getIndicesForValues(array $values) * view objects. * @param array $bucketForRemaining The bucket where to store the * non-preferred view objects. - * @param array $choices The list of choices. - * @param array $labels The labels corresponding to the choices. - * @param array $preferredChoices The preferred choices. + * @param array $choices The list of choices. + * @param array $labels The labels corresponding to the choices. + * @param array $preferredChoices The preferred choices. * * @throws UnexpectedTypeException If the structure of the $labels array * does not match the structure of the * $choices array. */ - protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices) + protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices) { - if (!is_array($choices) && !$choices instanceof \Traversable) { - throw new UnexpectedTypeException($choices, 'array or \Traversable'); - } - // Add choices to the nested buckets foreach ($choices as $group => $choice) { if (is_array($choice)) { - if (!is_array($labels)) { - throw new UnexpectedTypeException($labels, 'array'); - } - // Don't do the work if the array is empty if (count($choice) > 0) { $this->addChoiceGroup( @@ -292,11 +284,11 @@ protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choic * view objects. * @param array $bucketForRemaining The bucket where to store the * non-preferred view objects. - * @param array $choices The list of choices in the group. - * @param array $labels The labels corresponding to the choices in the group. - * @param array $preferredChoices The preferred choices. + * @param array $choices The list of choices in the group. + * @param array $labels The labels corresponding to the choices in the group. + * @param array $preferredChoices The preferred choices. */ - protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices) + protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices) { // If this is a choice group, create a new level in the choice // key hierarchy @@ -323,13 +315,15 @@ protected function addChoiceGroup($group, &$bucketForPreferred, &$bucketForRemai /** * Adds a new choice. * - * @param array $bucketForPreferred The bucket where to store the preferred - * view objects. - * @param array $bucketForRemaining The bucket where to store the - * non-preferred view objects. - * @param mixed $choice The choice to add. - * @param string $label The label for the choice. - * @param array $preferredChoices The preferred choices. + * @param array $bucketForPreferred The bucket where to store the preferred + * view objects. + * @param array $bucketForRemaining The bucket where to store the + * non-preferred view objects. + * @param mixed $choice The choice to add. + * @param string $label The label for the choice. + * @param array $preferredChoices The preferred choices. + * + * @throws InvalidConfigurationException If no valid value or index could be created. */ protected function addChoice(&$bucketForPreferred, &$bucketForRemaining, $choice, $label, array $preferredChoices) { @@ -366,8 +360,10 @@ protected function addChoice(&$bucketForPreferred, &$bucketForRemaining, $choice * * @param mixed $choice The choice to test. * @param array $preferredChoices An array of preferred choices. + * + * @return Boolean Whether the choice is preferred. */ - protected function isPreferred($choice, $preferredChoices) + protected function isPreferred($choice, array $preferredChoices) { return false !== array_search($choice, $preferredChoices, true); } diff --git a/src/Symfony/Component/Form/Extension/Core/ChoiceList/SimpleChoiceList.php b/src/Symfony/Component/Form/Extension/Core/ChoiceList/SimpleChoiceList.php index f1af179b6b4d..3e76900a7881 100644 --- a/src/Symfony/Component/Form/Extension/Core/ChoiceList/SimpleChoiceList.php +++ b/src/Symfony/Component/Form/Extension/Core/ChoiceList/SimpleChoiceList.php @@ -89,7 +89,7 @@ public function getValuesForChoices(array $choices) * * @see parent::addChoices */ - protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choices, $labels, array $preferredChoices) + protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, array $choices, array $labels, array $preferredChoices) { // Add choices to the nested buckets foreach ($choices as $choice => $label) { @@ -126,8 +126,10 @@ protected function addChoices(&$bucketForPreferred, &$bucketForRemaining, $choic * * @param mixed $choice The choice to test. * @param array $preferredChoices An array of preferred choices. + * + * @return Boolean Whether the choice is preferred. */ - protected function isPreferred($choice, $preferredChoices) + protected function isPreferred($choice, array $preferredChoices) { // Optimize performance over the default implementation return isset($preferredChoices[$choice]); diff --git a/src/Symfony/Component/Form/FormRenderer.php b/src/Symfony/Component/Form/FormRenderer.php index 5f1c98629d65..c4ecb7f4b52b 100644 --- a/src/Symfony/Component/Form/FormRenderer.php +++ b/src/Symfony/Component/Form/FormRenderer.php @@ -175,24 +175,15 @@ public function renderBlock($block, array $variables = array()) /** * {@inheritdoc} */ - public function isChoiceGroup($choice) + public function isChoiceSelected(ChoiceView $choice, $selectedValue) { - return is_array($choice) || $choice instanceof \Traversable; - } - - /** - * {@inheritdoc} - */ - public function isChoiceSelected(FormView $view, ChoiceView $choice) - { - $value = $view->vars['value']; $choiceValue = $choice->value; - if (is_array($value)) { - return false !== array_search($choiceValue, $value, true); + if (is_array($selectedValue)) { + return false !== array_search($choiceValue, $selectedValue, true); } - return $choiceValue === $value; + return $choiceValue === $selectedValue; } /** diff --git a/src/Symfony/Component/Form/FormRendererInterface.php b/src/Symfony/Component/Form/FormRendererInterface.php index 8ef742561649..748ba86b3e28 100644 --- a/src/Symfony/Component/Form/FormRendererInterface.php +++ b/src/Symfony/Component/Form/FormRendererInterface.php @@ -146,24 +146,15 @@ public function renderBlock($block, array $variables = array()); */ public function renderCsrfToken($intention); - /** - * Returns whether the given choice is a group. - * - * @param mixed $choice A choice - * - * @return Boolean Whether the choice is a group - */ - public function isChoiceGroup($choice); - /** * Returns whether the given choice is selected. * - * @param FormView $view The view of the choice field - * @param ChoiceView $choice The choice to check + * @param ChoiceView $choice The choice to check. + * @param string|array $selectedValue The selected value(s). * * @return Boolean Whether the choice is selected */ - public function isChoiceSelected(FormView $view, ChoiceView $choice); + public function isChoiceSelected(ChoiceView $choice, $selectedValue); /** * Makes a technical name human readable. diff --git a/src/Symfony/Component/Form/Tests/FormRendererTest.php b/src/Symfony/Component/Form/Tests/FormRendererTest.php index 2ba2f3373042..fa36e75a59d1 100644 --- a/src/Symfony/Component/Form/Tests/FormRendererTest.php +++ b/src/Symfony/Component/Form/Tests/FormRendererTest.php @@ -40,34 +40,6 @@ protected function setUp() $this->renderer = new FormRenderer($this->engine, $this->csrfProvider); } - public function isChoiceGroupProvider() - { - return array( - array(false, 0), - array(false, '0'), - array(false, '1'), - array(false, 1), - array(false, ''), - array(false, null), - array(false, true), - - array(true, array()), - ); - } - - /** - * @dataProvider isChoiceGroupProvider - */ - public function testIsChoiceGroup($expected, $value) - { - $this->assertSame($expected, $this->renderer->isChoiceGroup($value)); - } - - public function testIsChoiceGroupPart2() - { - $this->assertTrue($this->renderer->isChoiceGroup(new \SplFixedArray(1))); - } - public function isChoiceSelectedProvider() { // The commented cases should not be necessary anymore, because the @@ -96,10 +68,8 @@ public function isChoiceSelectedProvider() */ public function testIsChoiceSelected($expected, $choice, $value) { - $view = new FormView(); - $view->vars['value'] = $value; $choice = new ChoiceView($choice, $choice . ' label'); - $this->assertSame($expected, $this->renderer->isChoiceSelected($view, $choice)); + $this->assertSame($expected, $this->renderer->isChoiceSelected($choice, $value)); } }