From ebd2ca6cfe77543c8bcd06435d87902f5f765aa3 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Thu, 27 Jan 2011 13:04:02 +0100 Subject: [PATCH] [Form] Moved option 'empty_value' to ChoiceField. An empty value is displayed if the field is not required. --- src/Symfony/Component/Form/ChoiceField.php | 56 +++++++++++++------ src/Symfony/Component/Form/CountryField.php | 17 +----- src/Symfony/Component/Form/Field.php | 6 +- src/Symfony/Component/Form/LanguageField.php | 17 +----- src/Symfony/Component/Form/LocaleField.php | 17 +----- src/Symfony/Component/Form/TimezoneField.php | 16 +----- .../Tests/Component/Form/ChoiceFieldTest.php | 50 +++++++++++++++++ .../Tests/Component/Form/CountryFieldTest.php | 20 ------- .../Component/Form/LanguageFieldTest.php | 20 ------- .../Tests/Component/Form/LocaleFieldTest.php | 20 ------- .../Component/Form/TimezoneFieldTest.php | 20 ------- 11 files changed, 99 insertions(+), 160 deletions(-) diff --git a/src/Symfony/Component/Form/ChoiceField.php b/src/Symfony/Component/Form/ChoiceField.php index 600a486e98..350ea8e429 100644 --- a/src/Symfony/Component/Form/ChoiceField.php +++ b/src/Symfony/Component/Form/ChoiceField.php @@ -21,6 +21,9 @@ use Symfony\Component\Form\Exception\InvalidOptionsException; * * choices: An array of key-value pairs that will represent the choices * * preferred_choices: An array of choices (by key) that should be displayed * above all other options in the field + * * empty_value: If set to a non-false value, an "empty" option will + * be added to the top of the countries choices. A + * common value might be "Choose a country". Default: false. * * The multiple and expanded options control exactly which HTML element * that should be used to render this field: @@ -42,10 +45,10 @@ class ChoiceField extends HybridField /** * Stores the choices - * You should only access this property through getInitializedChoices() + * You should only access this property through getChoices() * @var array */ - protected $initializedChoices = array(); + private $choices = array(); protected function configure() { @@ -53,6 +56,7 @@ class ChoiceField extends HybridField $this->addOption('preferred_choices', array()); $this->addOption('multiple', false); $this->addOption('expanded', false); + $this->addOption('empty_value', ''); parent::configure(); @@ -73,7 +77,7 @@ class ChoiceField extends HybridField if ($this->isExpanded()) { $this->setFieldMode(self::GROUP); - $choices = $this->getInitializedChoices(); + $choices = $this->getChoices(); foreach ($this->preferredChoices as $choice => $_) { $this->add($this->newChoiceField($choice, $choices[$choice])); @@ -113,13 +117,31 @@ class ChoiceField extends HybridField */ protected function initializeChoices() { - $this->initializedChoices = $this->getOption('choices'); - - if ($this->initializedChoices instanceof \Closure) { - $this->initializedChoices = $this->initializedChoices->__invoke(); + if (!$this->choices) { + $this->choices = $this->getInitializedChoices(); } } + protected function getInitializedChoices() + { + $choices = $this->getOption('choices'); + + if ($choices instanceof \Closure) { + $choices = $choices->__invoke(); + } + + // TESTME + if (!is_array($choices)) { + throw new InvalidOptionsException('The "choices" option must be an array or a closure returning an array', array('choices')); + } + + if (!$this->isRequired()) { + $choices = array_merge(array('' => $this->getOption('empty_value')), $choices); + } + + return $choices; + } + /** * Returns the choices * @@ -128,28 +150,26 @@ class ChoiceField extends HybridField * * @return array */ - protected function getInitializedChoices() + protected function getChoices() { - if (!$this->initializedChoices) { - $this->initializeChoices(); - } + $this->initializeChoices(); - return $this->initializedChoices; + return $this->choices; } public function getPreferredChoices() { - return array_intersect_key($this->getInitializedChoices(), $this->preferredChoices); + return array_intersect_key($this->getChoices(), $this->preferredChoices); } public function getOtherChoices() { - return array_diff_key($this->getInitializedChoices(), $this->preferredChoices); + return array_diff_key($this->getChoices(), $this->preferredChoices); } public function getLabel($choice) { - $choices = $this->getInitializedChoices(); + $choices = $this->getChoices(); return isset($choices[$choice]) ? $choices[$choice] : null; } @@ -225,7 +245,7 @@ class ChoiceField extends HybridField { if ($this->isExpanded()) { $value = parent::transform($value); - $choices = $this->getInitializedChoices(); + $choices = $this->getChoices(); foreach ($choices as $choice => $_) { $choices[$choice] = $this->isMultipleChoice() @@ -235,6 +255,7 @@ class ChoiceField extends HybridField return $choices; } + return parent::transform($value); } @@ -265,9 +286,10 @@ class ChoiceField extends HybridField if ($this->isMultipleChoice()) { $value = $choices; } else { - $value = count($choices) > 0 ? current($choices) : null; + $value = count($choices) > 0 ? current($choices) : null; } } + return parent::reverseTransform($value); } } diff --git a/src/Symfony/Component/Form/CountryField.php b/src/Symfony/Component/Form/CountryField.php index 1054af8d7d..43ea5a993f 100644 --- a/src/Symfony/Component/Form/CountryField.php +++ b/src/Symfony/Component/Form/CountryField.php @@ -16,13 +16,6 @@ use Symfony\Component\Locale\Locale; /** * A field for selecting from a list of countries. * - * In addition to the ChoiceField options, this field has the following - * options: - * - * * empty_value: If set to a non-false value, an "empty" option will - * be added to the top of the countries choices. A - * common value might be "Choose a country". Default: false. - * * @see Symfony\Component\Form\ChoiceField * @author Bernhard Schussek */ @@ -30,15 +23,7 @@ class CountryField extends ChoiceField { protected function configure() { - $this->addOption('empty_value', false); - - $choices = Locale::getDisplayCountries($this->locale); - - if (false !== $this->getOption('empty_value')) { - $choices = array('' => $this->getOption('empty_value')) + $choices; - } - - $this->addOption('choices', $choices); + $this->addOption('choices', Locale::getDisplayCountries($this->locale)); parent::configure(); } diff --git a/src/Symfony/Component/Form/Field.php b/src/Symfony/Component/Form/Field.php index 733603f4c1..04e69113bf 100644 --- a/src/Symfony/Component/Form/Field.php +++ b/src/Symfony/Component/Form/Field.php @@ -88,7 +88,6 @@ class Field extends Configurable implements FieldInterface $this->normalizedData = $this->normalize($this->data); $this->transformedData = $this->transform($this->normalizedData); - $this->required = $this->getOption('required'); $this->setPropertyPath($this->getOption('property_path')); } @@ -184,9 +183,14 @@ class Field extends Configurable implements FieldInterface */ public function isRequired() { + if (null === $this->required) { + $this->required = $this->getOption('required'); + } + if (null === $this->parent || $this->parent->isRequired()) { return $this->required; } + return false; } diff --git a/src/Symfony/Component/Form/LanguageField.php b/src/Symfony/Component/Form/LanguageField.php index 64a2d669f7..4823d019c9 100644 --- a/src/Symfony/Component/Form/LanguageField.php +++ b/src/Symfony/Component/Form/LanguageField.php @@ -16,13 +16,6 @@ use Symfony\Component\Locale\Locale; /** * A field for selecting from a list of languages. * - * In addition to the ChoiceField options, this field has the following - * options: - * - * * empty_value: If set to a non-false value, an "empty" option will - * be added to the top of the languages choices. A - * common value might be "Choose a language". Default: false. - * * @see Symfony\Component\Form\ChoiceField * @author Bernhard Schussek */ @@ -33,15 +26,7 @@ class LanguageField extends ChoiceField */ protected function configure() { - $this->addOption('empty_value', false); - - $choices = Locale::getDisplayLanguages($this->locale); - - if (false !== $this->getOption('empty_value')) { - $choices = array('' => $this->getOption('empty_value')) + $choices; - } - - $this->addOption('choices', $choices); + $this->addOption('choices', Locale::getDisplayLanguages($this->locale)); parent::configure(); } diff --git a/src/Symfony/Component/Form/LocaleField.php b/src/Symfony/Component/Form/LocaleField.php index ad7ef8f374..d2ce0dd929 100644 --- a/src/Symfony/Component/Form/LocaleField.php +++ b/src/Symfony/Component/Form/LocaleField.php @@ -16,13 +16,6 @@ use Symfony\Component\Locale\Locale; /** * A field for selecting from a list of locales. * - * In addition to the ChoiceField options, this field has the following - * options: - * - * * empty_value: If set to a non-false value, an "empty" option will - * be added to the top of the locale choices. A - * common value might be "Choose a locale". Default: false. - * * @see Symfony\Component\Form\ChoiceField * @author Bernhard Schussek */ @@ -33,15 +26,7 @@ class LocaleField extends ChoiceField */ protected function configure() { - $this->addOption('empty_value', false); - - $choices = Locale::getDisplayLocales($this->locale); - - if (false !== $this->getOption('empty_value')) { - $choices = array('' => $this->getOption('empty_value')) + $choices; - } - - $this->addOption('choices', $choices); + $this->addOption('choices', Locale::getDisplayLocales($this->locale)); parent::configure(); } diff --git a/src/Symfony/Component/Form/TimezoneField.php b/src/Symfony/Component/Form/TimezoneField.php index a06b4fe2b2..831b679a9e 100644 --- a/src/Symfony/Component/Form/TimezoneField.php +++ b/src/Symfony/Component/Form/TimezoneField.php @@ -14,11 +14,7 @@ namespace Symfony\Component\Form; /** * Represents a field where each timezone is broken down by continent. * - * Available options: - * - * * empty_value: If set to a non-false value, an "empty" option will - * be added to the top of the timezone choices. A - * common value might be "Choose a timezone". Default: false. + * @author Bernhard Schussek */ class TimezoneField extends ChoiceField { @@ -33,15 +29,7 @@ class TimezoneField extends ChoiceField */ public function configure() { - $this->addOption('empty_value', false); - - $choices = self::getTimezoneChoices(); - - if (false !== $this->getOption('empty_value')) { - $choices = array('' => $this->getOption('empty_value')) + $choices; - } - - $this->addOption('choices', $choices); + $this->addOption('choices', self::getTimezoneChoices()); parent::configure(); } diff --git a/tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php b/tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php index 43a277f5c0..8e84731341 100644 --- a/tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php +++ b/tests/Symfony/Tests/Component/Form/ChoiceFieldTest.php @@ -87,6 +87,56 @@ class ChoiceFieldTest extends \PHPUnit_Framework_TestCase ); } + /** + * @expectedException Symfony\Component\Form\Exception\InvalidOptionsException + */ + public function testClosureShouldReturnArray() + { + $field = new ChoiceField('name', array( + 'choices' => function () { return 'foobar'; }, + )); + + // trigger closure + $field->getOtherChoices(); + } + + public function testNonRequiredContainsEmptyField() + { + $field = new ChoiceField('name', array( + 'multiple' => false, + 'expanded' => false, + 'choices' => $this->choices, + 'required' => false, + )); + + $this->assertEquals(array('' => '') + $this->choices, $field->getOtherChoices()); + } + + public function testRequiredContainsNoEmptyField() + { + $field = new ChoiceField('name', array( + 'multiple' => false, + 'expanded' => false, + 'choices' => $this->choices, + 'required' => true, + )); + + $this->assertEquals($this->choices, $field->getOtherChoices()); + } + + public function testEmptyValueConfiguresLabelOfEmptyField() + { + $field = new ChoiceField('name', array( + 'multiple' => false, + 'expanded' => false, + 'choices' => $this->choices, + 'required' => false, + 'empty_value' => 'Foobar', + )); + + $this->assertEquals(array('' => 'Foobar') + $this->choices, $field->getOtherChoices()); + } + /** * @dataProvider getChoicesVariants */ diff --git a/tests/Symfony/Tests/Component/Form/CountryFieldTest.php b/tests/Symfony/Tests/Component/Form/CountryFieldTest.php index 7547bf3795..600bedd964 100644 --- a/tests/Symfony/Tests/Component/Form/CountryFieldTest.php +++ b/tests/Symfony/Tests/Component/Form/CountryFieldTest.php @@ -42,24 +42,4 @@ class CountryFieldTest extends \PHPUnit_Framework_TestCase $this->assertArrayNotHasKey('ZZ', $choices); } - - public function testEmptyValueOption() - { - // empty_value false - $field = new CountryField('country', array('empty_value' => false)); - $choices = $field->getOtherChoices(); - $this->assertArrayNotHasKey('', $choices); - - // empty_value as a blank string - $field = new CountryField('country', array('empty_value' => '')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('', $choices['']); - - // empty_value as a normal string - $field = new CountryField('country', array('empty_value' => 'Choose a country')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('Choose a country', $choices['']); - } } \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/Form/LanguageFieldTest.php b/tests/Symfony/Tests/Component/Form/LanguageFieldTest.php index a6bb3baa72..7c94820512 100644 --- a/tests/Symfony/Tests/Component/Form/LanguageFieldTest.php +++ b/tests/Symfony/Tests/Component/Form/LanguageFieldTest.php @@ -42,24 +42,4 @@ class LanguageFieldTest extends \PHPUnit_Framework_TestCase $this->assertArrayNotHasKey('mul', $choices); } - - public function testEmptyValueOption() - { - // empty_value false - $field = new LanguageField('language', array('empty_value' => false)); - $choices = $field->getOtherChoices(); - $this->assertArrayNotHasKey('', $choices); - - // empty_value as a blank string - $field = new LanguageField('language', array('empty_value' => '')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('', $choices['']); - - // empty_value as a normal string - $field = new LanguageField('language', array('empty_value' => 'Choose a language')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('Choose a language', $choices['']); - } } \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/Form/LocaleFieldTest.php b/tests/Symfony/Tests/Component/Form/LocaleFieldTest.php index 8f64cf0d00..7d6eb127d0 100644 --- a/tests/Symfony/Tests/Component/Form/LocaleFieldTest.php +++ b/tests/Symfony/Tests/Component/Form/LocaleFieldTest.php @@ -30,24 +30,4 @@ class LocaleFieldTest extends \PHPUnit_Framework_TestCase $this->assertArrayHasKey('zh_Hans_MO', $choices); $this->assertEquals('Chinesisch (vereinfacht, Sonderverwaltungszone Macao)', $choices['zh_Hans_MO']); } - - public function testEmptyValueOption() - { - // empty_value false - $field = new LocaleField('language', array('empty_value' => false)); - $choices = $field->getOtherChoices(); - $this->assertArrayNotHasKey('', $choices); - - // empty_value as a blank string - $field = new LocaleField('language', array('empty_value' => '')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('', $choices['']); - - // empty_value as a normal string - $field = new LocaleField('language', array('empty_value' => 'Choose a locale')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('Choose a locale', $choices['']); - } } \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/Form/TimezoneFieldTest.php b/tests/Symfony/Tests/Component/Form/TimezoneFieldTest.php index d6df701b99..c7921d8ec0 100644 --- a/tests/Symfony/Tests/Component/Form/TimezoneFieldTest.php +++ b/tests/Symfony/Tests/Component/Form/TimezoneFieldTest.php @@ -28,24 +28,4 @@ class TimezoneFieldTest extends \PHPUnit_Framework_TestCase $this->assertArrayHasKey('America/New_York', $choices['America']); $this->assertEquals('New York', $choices['America']['America/New_York']); } - - public function testEmptyValueOption() - { - // empty_value false - $field = new TimezoneField('timezone', array('empty_value' => false)); - $choices = $field->getOtherChoices(); - $this->assertArrayNotHasKey('', $choices); - - // empty_value as a blank string - $field = new TimezoneField('timezone', array('empty_value' => '')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('', $choices['']); - - // empty_value as a normal string - $field = new TimezoneField('timezone', array('empty_value' => 'Choose your timezone')); - $choices = $field->getOtherChoices(); - $this->assertArrayHasKey('', $choices); - $this->assertEquals('Choose your timezone', $choices['']); - } } \ No newline at end of file