8000 [Form] fix edge cases with choice placeholder · symfony/symfony@7d34ccf · GitHub
[go: up one dir, main page]
Skip to content

Commit 7d34ccf

Browse files
TobionTobion
committed
[Form] fix edge cases with choice placeholder
1 parent ca6f1f7 commit 7d34ccf

File tree

7 files changed

+51
-8
lines changed

7 files changed

+51
-8
lines changed

‎src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
{%- endblock choice_widget_expanded -%}
5353

5454
{%- block choice_widget_collapsed -%}
55-
{%- if required and placeholder is none and not placeholder_in_choices and not multiple -%}
55+
{%- if required and placeholder is none and not placeholder_in_choices and not multiple and (attr.size is not defined or attr.size <= 1) -%}
5656
{% set required = false %}
5757
{%- endif -%}
5858
<select {{ block('widget_attributes') }}{% if multiple %} multiple="multiple"{% endif %}>

‎src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<select
2-
<?php if ($required && null === $placeholder && $placeholder_in_choices === false && $multiple === false):
2+
<?php if ($required && null === $placeholder && $placeholder_in_choices === false && $multiple === false && (!isset($attr['size']) || $attr['size'] <= 1)):
33
$required = false;
44
endif; ?>
55
<?php echo $view['form']->block($form, 'widget_attributes', array(

‎src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ private static function addChoiceViewGroupedBy($groupBy, $choice, $value, $label
246246

247247
$groupLabel = (string) $groupLabel;
248248

249-
// Initialize the group views if necessary. Unnnecessarily built group
249+
// Initialize the group views if necessary. Unnecessarily built group
250250
// views will be cleaned up at the end of createView()
251251
if (!isset($preferredViews[$groupLabel])) {
252252
$preferredViews[$groupLabel] = new ChoiceGroupView($groupLabel);

‎src/Symfony/Component/Form/ChoiceList/View/ChoiceListView.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,24 @@ public function __construct(array $choices = array(), array $preferredChoices =
4848
$this->choices = $choices;
4949
$this->preferredChoices = $preferredChoices;
5050
}
51+
52+
/**
53+
* Returns whether a placeholder is in the choices.
54+
*
55+
* A placeholder must be the first child element, not be in a group and have an empty value.
56+
*
57+
* @return bool
58+
*/
59+
public function hasPlaceholder()
60+
{
61+
if ($this->preferredChoices) {
62+
$firstChoice = reset($this->preferredChoices);
63+
64+
return $firstChoice instanceof ChoiceView && '' === $firstChoice->value;
65+
}
66+
67+
$firstChoice = reset($this->choices);
68+
69+
return $firstChoice instanceof ChoiceView && '' === $firstChoice->value;
70+
}
5171
}

‎src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public function buildView(FormView $view, FormInterface $form, array $options)
199199
}
200200

201201
// Check if the choices already contain the empty value
202-
$view->vars['placeholder_in_choices'] = 0 !== count($options['choice_list']->getChoicesForValues(array('')));
202+
$view->vars['placeholder_in_choices'] = $choiceListView->hasPlaceholder();
203203

204204
// Only add the empty value option if this is not the case
205205
if (null !== $options['placeholder'] && !$view->vars['placeholder_in_choices']) {
@@ -343,6 +343,9 @@ public function configureOptions(OptionsResolver $resolver)
343343
if ($options['multiple']) {
344344
// never use an empty value for this case
345345
return;
346+
} elseif ($options['required'] && ($options['expanded'] || isset($options['attr']['size']) && $options['attr']['size'] > 1)) {
347+
// placeholder for required radio buttons or a select with size > 1 does not make sense
348+
return;
346349
} elseif (false === $placeholder) {
347350
// an empty value should be added but the user decided otherwise
348351
return;

‎src/Symfony/Component/Form/Tests/AbstractLayoutTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,25 @@ public function testSingleChoice()
528528
);
529529
}
530530

531+
public function testSelectWithSizeBiggerThanOneCanBeRequired()
532+
{
533+
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ChoiceType', '&a', array(
534+
'choices' => array('Choice&A' => '&a', 'Choice&B' => '&b'),
535+
'multiple' => false,
536+
'expanded' => false,
537+
'attr' => array('size' => 2),
538+
));
539+
540+
$this->assertWidgetMatchesXpath($form->createView(), array(),
541+
'/select
542+
[@name="name"]
543+
[@required="required"]
544+
[@size="2"]
545+
[count(./option)=2]
546+
'
547+
);
548+
}
549+
531550
public function testSingleChoiceWithoutTranslation()
532551
{
533552
$form = $this->factory->createNamed('name', 'choice', '&a', array(
@@ -1001,6 +1020,7 @@ public function testSingleChoiceExpandedWithPlaceholder()
10011020
'multiple' => false,
10021021
'expanded' => true,
10031022
'placeholder' => 'Test&Me',
1023+
'required' => false,
10041024
));
10051025

10061026
$this->assertWidgetMatchesXpath($form->createView(), array(),

‎src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,7 +1689,7 @@ public function testDontPassPlaceholderIfContainedInChoices($multiple, $expanded
16891689
'expanded' => $expanded,
16901690
'required' => $required,
16911691
'placeholder' => $placeholder,
1692-
'choices' => array('A' => 'a', 'Empty' => ''),
1692+
'choices' => array('Empty' => '', 'A' => 'a'),
16931693
'choices_as_values' => true,
16941694
));
16951695
$view = $form->createView();
@@ -1716,9 +1716,9 @@ public function getOptionsWithPlaceholder()
17161716
array(false, true, false, '', 'None'),
17171717
array(false, true, false, null, null),
17181718
array(false, true, false, false, null),
1719-
array(false, true, true, 'foobar', 'foobar'),
1720-
// radios should never have an empty label
1721-
array(false, true, true, '', 'None'),
1719+
// required radios should never have a placeholder
1720+
array(false, true, true, 'foobar', null),
1721+
array(false, true, true, '', null),
17221722
array(false, true, true, null, null),
17231723
array(false, true, true, false, null),
17241724
// multiple non-expanded

0 commit comments

Comments
 (0)
0