Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7d34ccf

Browse filesBrowse files
TobionTobion
committed
[Form] fix edge cases with choice placeholder
1 parent ca6f1f7 commit 7d34ccf
Copy full SHA for 7d34ccf

File tree

7 files changed

+51
-8
lines changed
Filter options

7 files changed

+51
-8
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/ChoiceList/Factory/DefaultChoiceListFactory.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/ChoiceList/View/ChoiceListView.php
+20Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
+4-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/AbstractLayoutTest.php
+20Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
+4-4Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.