[Form] fix edge cases with choice placeholder

This commit is contained in:
Tobias Schultze 2016-02-13 05:58:41 +01:00
parent ca6f1f7428
commit 0efbc30304
8 changed files with 73 additions and 8 deletions

View File

@ -52,7 +52,7 @@
{%- endblock choice_widget_expanded -%}
{%- block choice_widget_collapsed -%}
{%- if required and placeholder is none and not placeholder_in_choices and not multiple -%}
{%- if required and placeholder is none and not placeholder_in_choices and not multiple and (attr.size is not defined or attr.size <= 1) -%}
{% set required = false %}
{%- endif -%}
<select {{ block('widget_attributes') }}{% if multiple %} multiple="multiple"{% endif %}>

View File

@ -1,5 +1,5 @@
<select
<?php if ($required && null === $placeholder && $placeholder_in_choices === false && $multiple === false):
<?php if ($required && null === $placeholder && $placeholder_in_choices === false && $multiple === false && (!isset($attr['size']) || $attr['size'] <= 1)):
$required = false;
endif; ?>
<?php echo $view['form']->block($form, 'widget_attributes', array(

View File

@ -246,7 +246,7 @@ class DefaultChoiceListFactory implements ChoiceListFactoryInterface
$groupLabel = (string) $groupLabel;
// Initialize the group views if necessary. Unnnecessarily built group
// Initialize the group views if necessary. Unnecessarily built group
// views will be cleaned up at the end of createView()
if (!isset($preferredViews[$groupLabel])) {
$preferredViews[$groupLabel] = new ChoiceGroupView($groupLabel);

View File

@ -48,4 +48,24 @@ class ChoiceListView
$this->choices = $choices;
$this->preferredChoices = $preferredChoices;
}
/**
* Returns whether a placeholder is in the choices.
*
* A placeholder must be the first child element, not be in a group and have an empty value.
*
* @return bool
*/
public function hasPlaceholder()
{
if ($this->preferredChoices) {
$firstChoice = reset($this->preferredChoices);
return $firstChoice instanceof ChoiceView && '' === $firstChoice->value;
}
$firstChoice = reset($this->choices);
return $firstChoice instanceof ChoiceView && '' === $firstChoice->value;
}
}

View File

@ -199,7 +199,7 @@ class ChoiceType extends AbstractType
}
// Check if the choices already contain the empty value
$view->vars['placeholder_in_choices'] = 0 !== count($options['choice_list']->getChoicesForValues(array('')));
$view->vars['placeholder_in_choices'] = $choiceListView->hasPlaceholder();
// Only add the empty value option if this is not the case
if (null !== $options['placeholder'] && !$view->vars['placeholder_in_choices']) {
@ -343,6 +343,9 @@ class ChoiceType extends AbstractType
if ($options['multiple']) {
// never use an empty value for this case
return;
} elseif ($options['required'] && ($options['expanded'] || isset($options['attr']['size']) && $options['attr']['size'] > 1)) {
// placeholder for required radio buttons or a select with size > 1 does not make sense
return;
} elseif (false === $placeholder) {
// an empty value should be added but the user decided otherwise
return;

View File

@ -232,6 +232,26 @@ abstract class AbstractBootstrap3LayoutTest extends AbstractLayoutTest
);
}
public function testSelectWithSizeBiggerThanOneCanBeRequired()
{
$form = $this->factory->createNamed('name', 'choice', null, array(
'choices' => array('a', 'b'),
'choices_as_values' => true,
'multiple' => false,
'expanded' => false,
'attr' => array('size' => 2),
));
$this->assertWidgetMatchesXpath($form->createView(), array('attr' => array('class' => '')),
'/select
[@name="name"]
[@required="required"]
[@size="2"]
[count(./option)=2]
'
);
}
public function testSingleChoiceWithoutTranslation()
{
$form = $this->factory->createNamed('name', 'choice', '&a', array(
@ -754,6 +774,7 @@ abstract class AbstractBootstrap3LayoutTest extends AbstractLayoutTest
'multiple' => false,
'expanded' => true,
'placeholder' => 'Test&Me',
'required' => false,
));
$this->assertWidgetMatchesXpath($form->createView(), array(),

View File

@ -528,6 +528,26 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg
);
}
public function testSelectWithSizeBiggerThanOneCanBeRequired()
{
$form = $this->factory->createNamed('name', 'choice', null, array(
'choices' => array('a', 'b'),
'choices_as_values' => true,
'multiple' => false,
'expanded' => false,
'attr' => array('size' => 2),
));
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/select
[@name="name"]
[@required="required"]
[@size="2"]
[count(./option)=2]
'
);
}
public function testSingleChoiceWithoutTranslation()
{
$form = $this->factory->createNamed('name', 'choice', '&a', array(
@ -1001,6 +1021,7 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg
'multiple' => false,
'expanded' => true,
'placeholder' => 'Test&Me',
'required' => false,
));
$this->assertWidgetMatchesXpath($form->createView(), array(),

View File

@ -1689,7 +1689,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
'expanded' => $expanded,
'required' => $required,
'placeholder' => $placeholder,
'choices' => array('A' => 'a', 'Empty' => ''),
'choices' => array('Empty' => '', 'A' => 'a'),
'choices_as_values' => true,
));
$view = $form->createView();
@ -1716,9 +1716,9 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
array(false, true, false, '', 'None'),
array(false, true, false, null, null),
array(false, true, false, false, null),
array(false, true, true, 'foobar', 'foobar'),
// radios should never have an empty label
array(false, true, true, '', 'None'),
// required radios should never have a placeholder
array(false, true, true, 'foobar', null),
array(false, true, true, '', null),
array(false, true, true, null, null),
array(false, true, true, false, null),
// multiple non-expanded