[Form] fix edge cases with choice placeholder
This commit is contained in:
parent
ca6f1f7428
commit
0efbc30304
@ -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 %}>
|
||||
|
@ -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(
|
||||
|
@ -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);
|
||||
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
@ -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;
|
||||
|
@ -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(),
|
||||
|
@ -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(),
|
||||
|
@ -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
|
||||
|
Reference in New Issue
Block a user