Skip to content

[Form] Added radio button for empty value to expanded single-choice fields #7939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ CHANGELOG
unless FormInterface::setData() is called manually
* fixed CSRF error message to be translated
* custom CSRF error messages can now be set through the "csrf_message" option
* fixed: expanded single-choice fields now show a radio button for the empty value

2.2.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public function getRemainingViews();
/**
* Returns the choices corresponding to the given values.
*
* The choices can have any data type.
*
* @param array $values An array of choice values. Not existing values in
* this array are ignored.
*
Expand All @@ -105,6 +107,8 @@ public function getChoicesForValues(array $values);
/**
* Returns the values corresponding to the given choices.
*
* The values must be a strings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove "a" here?

*
* @param array $choices An array of choices. Not existing choices in this
* array are ignored.
*
Expand All @@ -116,20 +120,30 @@ public function getValuesForChoices(array $choices);
/**
* Returns the indices corresponding to the given choices.
*
* The indices must be positive integers or strings accepted by
* {@link FormConfigBuilder::validateName()}.
*
* The index "placeholder" is internally reserved.
*
* @param array $choices An array of choices. Not existing choices in this
* array are ignored.
*
* @return array An array of indices with ascending, 0-based numeric keys
* @return array An array of indices with ascending, 0-based numeric keys.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't put dots at the end of the @... lines

*/
public function getIndicesForChoices(array $choices);

/**
* Returns the indices corresponding to the given values.
*
* The indices must be positive integers or strings accepted by
* {@link FormConfigBuilder::validateName()}.
*
* The index "placeholder" is internally reserved.
*
* @param array $values An array of choice values. Not existing values in
* this array are ignored.
*
* @return array An array of indices with ascending, 0-based numeric keys
* @return array An array of indices with ascending, 0-based numeric keys.
*/
public function getIndicesForValues(array $values);
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ class ChoiceToBooleanArrayTransformer implements DataTransformerInterface
{
private $choiceList;

private $placeholderPresent;

/**
* Constructor.
*
* @param ChoiceListInterface $choiceList
* @param Boolean $placeholderPresent
*/
public function __construct(ChoiceListInterface $choiceList)
public function __construct(ChoiceListInterface $choiceList, $placeholderPresent)
{
$this->choiceList = $choiceList;
$this->placeholderPresent = $placeholderPresent;
}

/**
Expand Down Expand Up @@ -63,6 +67,10 @@ public function transform($choice)
$values[$i] = $i === $index;
}

if ($this->placeholderPresent) {
$values['placeholder'] = false === $index;
}

return $values;
}

Expand Down Expand Up @@ -97,6 +105,8 @@ public function reverseTransform($values)
if ($selected) {
if (isset($choices[$i])) {
return $choices[$i] === '' ? null : $choices[$i];
} elseif ($this->placeholderPresent && 'placeholder' === $i) {
return null;
} else {
throw new TransformationFailedException(sprintf('The choice "%s" does not exist', $i));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,26 @@ class FixRadioInputListener implements EventSubscriberInterface
{
private $choiceList;

private $placeholderPresent;

/**
* Constructor.
*
* @param ChoiceListInterface $choiceList
* @param Boolean $placeholderPresent
*/
public function __construct(ChoiceListInterface $choiceList)
public function __construct(ChoiceListInterface $choiceList, $placeholderPresent)
{
$this->choiceList = $choiceList;
$this->placeholderPresent = $placeholderPresent;
}

public function preSubmit(FormEvent $event)
{
$value = $event->getData();
$index = current($this->choiceList->getIndicesForValues(array($value)));

$event->setData(false !== $index ? array($index => $value) : array());
$event->setData(false !== $index ? array($index => $value) : ($this->placeholderPresent ? array('placeholder' => '') : array())) ;
}

/**
Expand Down
45 changes: 32 additions & 13 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Form\Extension\Core\Type;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\View\ChoiceView;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\FormView;
Expand Down Expand Up @@ -45,19 +46,34 @@ public function buildForm(FormBuilderInterface $builder, array $options)
}

if ($options['expanded']) {
$this->addSubForms($builder, $options['choice_list']->getPreferredViews(), $options);
$this->addSubForms($builder, $options['choice_list']->getRemainingViews(), $options);
// Initialize all choices before doing the index check below.
// This helps in cases where index checks are optimized for non
// initialized choice lists. For example, when using an SQL driver,
// the index check would read in one SQL query and the initialization
// requires another SQL query. When the initialization is done first,
// one SQL query is sufficient.
$preferredViews = $options['choice_list']->getPreferredViews();
$remainingViews = $options['choice_list']->getRemainingViews();

// Check if the choices already contain the empty value
// Only add the empty value option if this is not the case
if (null !== $options['empty_value'] && 0 === count($options['choice_list']->getIndicesForValues(array('')))) {
$placeholderView = new ChoiceView(null, '', $options['empty_value']);

// "placeholder" is a reserved index
// see also ChoiceListInterface::getIndicesForChoices()
$this->addSubForms($builder, array('placeholder' => $placeholderView), $options);
}

$this->addSubForms($builder, $preferredViews, $options);
$this->addSubForms($builder, $remainingViews, $options);

if ($options['multiple']) {
$builder
->addViewTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']))
->addEventSubscriber(new FixCheckboxInputListener($options['choice_list']), 10)
;
$builder->addViewTransformer(new ChoicesToBooleanArrayTransformer($options['choice_list']));
$builder->addEventSubscriber(new FixCheckboxInputListener($options['choice_list']), 10);
} else {
$builder
->addViewTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list']))
->addEventSubscriber(new FixRadioInputListener($options['choice_list']), 10)
;
$builder->addViewTransformer(new ChoiceToBooleanArrayTransformer($options['choice_list'], $builder->has('placeholder')));
$builder->addEventSubscriber(new FixRadioInputListener($options['choice_list'], $builder->has('placeholder')), 10);
}
} else {
if ($options['multiple']) {
Expand Down Expand Up @@ -104,7 +120,7 @@ public function buildView(FormView $view, FormInterface $form, array $options)

// Check if the choices already contain the empty value
// Only add the empty value option if this is not the case
if (0 === count($options['choice_list']->getIndicesForValues(array('')))) {
if (null !== $options['empty_value'] && 0 === count($options['choice_list']->getIndicesForValues(array('')))) {
$view->vars['empty_value'] = $options['empty_value'];
}

Expand Down Expand Up @@ -170,12 +186,15 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
};

$emptyValueNormalizer = function (Options $options, $emptyValue) {
if ($options['multiple'] || $options['expanded']) {
// never use an empty value for these cases
if ($options['multiple']) {
// never use an empty value for this case
return null;
} elseif (false === $emptyValue) {
// an empty value should be added but the user decided otherwise
return null;
} elseif ($options['expanded'] && '' === $emptyValue) {
// never use an empty label for radio buttons
return 'None';
}

// empty value has been set explicitly
Expand Down
10 changes: 6 additions & 4 deletions src/Symfony/Component/Form/Tests/AbstractLayoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ public function testMultipleChoice()
);
}

public function testMultipleChoiceSkipEmptyValue()
public function testMultipleChoiceSkipsEmptyValue()
{
$form = $this->factory->createNamed('name', 'choice', array('&a'), array(
'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
Expand Down Expand Up @@ -700,7 +700,7 @@ public function testSingleChoiceExpanded()
);
}

public function testSingleChoiceExpandedSkipEmptyValue()
public function testSingleChoiceExpandedWithEmptyValue()
{
$form = $this->factory->createNamed('name', 'choice', '&a', array(
'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
Expand All @@ -712,13 +712,15 @@ public function testSingleChoiceExpandedSkipEmptyValue()
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/div
[
./input[@type="radio"][@name="name"][@id="name_0"][@checked]
./input[@type="radio"][@name="name"][@id="name_placeholder"][not(@checked)]
/following-sibling::label[@for="name_placeholder"][.="[trans]Test&Me[/trans]"]
/following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@checked]
/following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"]
/following-sibling::input[@type="radio"][@name="name"][@id="name_1"][not(@checked)]
/following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"]
/following-sibling::input[@type="hidden"][@id="name__token"]
]
[count(./input)=3]
[count(./input)=4]
'
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

class FixRadioInputListenerTest extends \PHPUnit_Framework_TestCase
{
private $listener;
private $choiceList;

protected function setUp()
{
Expand All @@ -27,15 +27,14 @@ protected function setUp()

parent::setUp();

$list = new SimpleChoiceList(array(0 => 'A', 1 => 'B'));
$this->listener = new FixRadioInputListener($list);
$this->choiceList = new SimpleChoiceList(array('' => 'Empty', 0 => 'A', 1 => 'B'));
}

protected function tearDown()
{
parent::tearDown();

$this->listener = null;
$listener = null;
}

public function testFixRadio()
Expand All @@ -44,9 +43,11 @@ public function testFixRadio()
$form = $this->getMock('Symfony\Component\Form\Test\FormInterface');
$event = new FormEvent($form, $data);

$this->listener->preSubmit($event);
$listener = new FixRadioInputListener($this->choiceList, true);
$listener->preSubmit($event);

$this->assertEquals(array(1 => '1'), $event->getData());
// Indices in SimpleChoiceList are zero-based generated integers
$this->assertEquals(array(2 => '1'), $event->getData());
}

public function testFixZero()
Expand All @@ -55,18 +56,50 @@ public function testFixZero()
$form = $this->getMock('Symfony\Component\Form\Test\FormInterface');
$event = new FormEvent($form, $data);

$this->listener->preSubmit($event);
$listener = new FixRadioInputListener($this->choiceList, true);
$listener->preSubmit($event);

$this->assertEquals(array(0 => '0'), $event->getData());
// Indices in SimpleChoiceList are zero-based generated integers
$this->assertEquals(array(1 => '0'), $event->getData());
}

public function testIgnoreEmptyString()
public function testFixEmptyString()
{
$data = '';
$form = $this->getMock('Symfony\Component\Form\Test\FormInterface');
$event = new FormEvent($form, $data);

$this->listener->preSubmit($event);
$listener = new FixRadioInputListener($this->choiceList, true);
$listener->preSubmit($event);

// Indices in SimpleChoiceList are zero-based generated integers
$this->assertEquals(array(0 => ''), $event->getData());
}

public function testConvertEmptyStringToPlaceholderIfNotFound()
{
$list = new SimpleChoiceList(array(0 => 'A', 1 => 'B'));

$data = '';
$form = $this->getMock('Symfony\Component\Form\Test\FormInterface');
$event = new FormEvent($form, $data);

$listener = new FixRadioInputListener($list, true);
$listener->preSubmit($event);

$this->assertEquals(array('placeholder' => ''), $event->getData());
}

public function testDontConvertEmptyStringToPlaceholderIfNoPlaceholderUsed()
{
$list = new SimpleChoiceList(array(0 => 'A', 1 => 'B'));

$data = '';
$form = $this->getMock('Symfony\Component\Form\Test\FormInterface');
$event = new FormEvent($form, $data);

$listener = new FixRadioInputListener($list, false);
$listener->preSubmit($event);

$this->assertEquals(array(), $event->getData());
}
Expand Down
Loading