Skip to content

[Form] Added an alternative signature Form::add($name, $type, $options) #6355

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.

Sign up for GitHub

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 2 commits into from
Dec 18, 2012
Merged

[Form] Added an alternative signature Form::add($name, $type, $options) #6355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb71964
[Form] Added an alternative signature Form::add($name, $type, $options)
webmozart Dec 14, 2012
19d8510
[Form] Improved Form::add() and FormBuilder::add() to accept integers…
webmozart Dec 14, 2012
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
9 changes: 8 additions & 1 deletion UPGRADE-2.2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@

### Form

* The PasswordType is now not trimmed by default.
* The PasswordType is now not trimmed by default.

#### Deprecations

* The methods `getParent()`, `setParent()` and `hasParent()` in
Copy link
Contributor

Choose a reason for hiding this comment

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

Please no! We need know the parent data class in the form subscribers.

Copy link
Member

Choose a reason for hiding this comment

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

The form subscribers are not accessing the FormBuilder but the Form

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry. But we need this feature in the form type extensions.

Copy link
Member

Choose a reason for hiding this comment

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

@hason Please read the full deprecation message: this feature is not reliable (and cannot be made reliable without rewriting the full Form building as children can be build before adding them in their parent)

Copy link

Choose a reason for hiding this comment

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

really sad about this :(

Copy link
Member

Choose a reason for hiding this comment

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

@tiger-seo they never worked properly as the parent form is generally not available yet when calling buildForm on the type. The form is added into its parent after building the Form instance from the builder.

Copy link

Choose a reason for hiding this comment

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

@stof thank you very much for you time. the reason I need property parent is because I have a sophisticated field type (kinda collection of entities the main idea of which is to link entities received and those stored in DB by using ID of entities), because it is not possible with existing collection type, which uses ordinal position, so the reason is - I need to attach my event handler on the root form.
with Symfony 2.2 I get root-form this way:

        $rootFormBuilder = $builder;
        while ($rootFormBuilder->hasParent()) {
            $rootFormBuilder = $rootFormBuilder->getParent();
        }
        $subscriber = new IdBasedCollectionFormErrorKeyFixerListener($builder);
        $rootFormBuilder->addEventSubscriber($subscriber);

full listing https://gist.github.com/tiger-seo/67a6e8e7177b3b6c104d

i have no idea how i can get root-form to attach my event handler :(

Copy link
Contributor

Choose a reason for hiding this comment

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

The same problem is described in #8607

`FormBuilderInterface` were deprecated and will be removed in Symfony 2.3.
You should not rely on these methods in your form type because the parent
of a form can change after building it.

### Routing

Expand Down
7 changes: 2 additions & 5 deletions src/Symfony/Bridge/Propel1/Form/EventListener/TranslationFormListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
namespace Symfony\Bridge\Propel1\Form\EventListener;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Form\FormFactoryInterface;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormEvents;

Expand All @@ -24,13 +23,11 @@ class TranslationFormListener implements EventSubscriberInterface
{
private $columns;
private $dataClass;
private $formFactory;

public function __construct(FormFactoryInterface $formFactory, $columns, $dataClass)
public function __construct($columns, $dataClass)
{
$this->columns = $columns;
$this->dataClass = $dataClass;
$this->formFactory = $formFactory;
}

public static function getSubscribedEvents()
Expand Down Expand Up @@ -78,7 +75,7 @@ public function preSetData(FormEvent $event)

$options = array_merge($options, $customOptions);

$form->add($this->formFactory->createNamed($column, $type, null, $options));
$form->add($column, $type, $options);
}
}
}
5 changes: 3 additions & 2 deletions src/Symfony/Bridge/Propel1/Form/Type/TranslationType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ class TranslationType extends AbstractType
*/
public function buildForm(FormBuilderInterface $builder, array $options)
{
$listener = new TranslationFormListener($builder->getFormFactory(), $options['columns'], $options['data_class']);
$builder->addEventSubscriber($listener);
$builder->addEventSubscriber(
new TranslationFormListener($options['columns'], $options['data_class'])
);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ CHANGELOG
-----

* TrimListener now removes unicode whitespaces
* deprecated getParent(), setParent() and hasParent() in FormBuilderInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this change. We need this feature in the form type extensions.

* FormInterface::add() now accepts a FormInterface instance OR a field's name, type and options

2.1.0
-----
Expand Down
17 changes: 5 additions & 12 deletions src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use Symfony\Component\Form\FormEvents;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormFactoryInterface;
use Symfony\Component\Form\Exception\UnexpectedTypeException;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

Expand All @@ -24,11 +23,6 @@
*/
class ResizeFormListener implements EventSubscriberInterface
{
/**
* @var FormFactoryInterface
*/
protected $factory;

/**
* @var string
*/
Expand All @@ -51,9 +45,8 @@ class ResizeFormListener implements EventSubscriberInterface
*/
protected $allowDelete;

public function __construct(FormFactoryInterface $factory, $type, array $options = array(), $allowAdd = false, $allowDelete = false)
public function __construct($type, array $options = array(), $allowAdd = false, $allowDelete = false)
{
$this->factory = $factory;
$this->type = $type;
$this->allowAdd = $allowAdd;
$this->allowDelete = $allowDelete;
Expand Down Expand Up @@ -90,9 +83,9 @@ public function preSetData(FormEvent $event)

// Then add all rows again in the correct order
foreach ($data as $name => $value) {
$form->add($this->factory->createNamed($name, $this->type, null, array_replace(array(
$form->add($name, $this->type, array_replace(array(
'property_path' => '['.$name.']',
), $this->options)));
), $this->options));
}
}

Expand Down Expand Up @@ -122,9 +115,9 @@ public function preBind(FormEvent $event)
if ($this->allowAdd) {
foreach ($data as $name => $value) {
if (!$form->has($name)) {
$form->add($this->factory->createNamed($name, $this->type, null, array_replace(array(
$form->add($name, $this->type, array_replace(array(
'property_path' => '['.$name.']',
), $this->options)));
), $this->options));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ private function addSubForms(FormBuilderInterface $builder, array $choiceViews,
$choiceType = 'radio';
}

$builder->add((string) $i, $choiceType, $choiceOpts);
$builder->add($i, $choiceType, $choiceOpts);
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public function buildForm(FormBuilderInterface $builder, array $options)
}

$resizeListener = new ResizeFormListener(
$builder->getFormFactory(),
$options['type'],
$options['options'],
$options['allow_add'],
Expand Down
19 changes: 18 additions & 1 deletion src/Symfony/Component/Form/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Form;

use Symfony\Component\Form\Exception\FormException;
use Symfony\Component\Form\Exception\UnexpectedTypeException;
use Symfony\Component\Form\Exception\AlreadyBoundException;
use Symfony\Component\Form\Exception\TransformationFailedException;
use Symfony\Component\Form\Util\FormUtil;
Expand Down Expand Up @@ -859,7 +860,7 @@ public function hasChildren()
/**
* {@inheritdoc}
*/
public function add(FormInterface $child)
public function add($child, $type = null, array $options = array())
{
if ($this->bound) {
throw new AlreadyBoundException('You cannot add children to a bound form');
Expand Down Expand Up @@ -888,6 +889,22 @@ public function add(FormInterface $child)
$viewData = $this->getViewData();
}

if (!$child instanceof FormInterface) {
if (!is_string($child) && !is_int($child)) {
throw new UnexpectedTypeException($child, 'string, integer or Symfony\Component\Form\FormInterface');
}

if (null !== $type && !is_string($type) && !$type instanceof FormTypeInterface) {
throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
}

if (null === $type) {
$child = $this->config->getFormFactory()->createForProperty($this->config->getDataClass(), $child, null, $options);
} else {
$child = $this->config->getFormFactory()->createNamed($child, $type, null, $options);
}
}

$this->children[$child->getName()] = $child;

$child->setParent($this);
Expand Down
25 changes: 5 additions & 20 deletions src/Symfony/Component/Form/FormBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,6 @@
*/
class FormBuilder extends FormConfigBuilder implements \IteratorAggregate, FormBuilderInterface
{
/**
* The form factory.
*
* @var FormFactoryInterface
*/
private $factory;

/**
* The children of the form builder.
*
Expand Down Expand Up @@ -63,15 +56,7 @@ public function __construct($name, $dataClass, EventDispatcherInterface $dispatc
{
parent::__construct($name, $dataClass, $dispatcher, $options);

$this->factory = $factory;
}

/**
* {@inheritdoc}
*/
public function getFormFactory()
{
return $this->factory;
$this->setFormFactory($factory);
}

/**
Expand All @@ -93,8 +78,8 @@ public function add($child, $type = null, array $options = array())
return $this;
}

if (!is_string($child)) {
throw new UnexpectedTypeException($child, 'string or Symfony\Component\Form\FormBuilder');
if (!is_string($child) && !is_int($child)) {
throw new UnexpectedTypeException($child, 'string, integer or Symfony\Component\Form\FormBuilder');
}

if (null !== $type && !is_string($type) && !$type instanceof FormTypeInterface) {
Expand Down Expand Up @@ -125,10 +110,10 @@ public function create($name, $type = null, array $options = array())
}

if (null !== $type) {
return $this->factory->createNamedBuilder($name, $type, null, $options, $this);
return $this->getFormFactory()->createNamedBuilder($name, $type, null, $options, $this);
}

return $this->factory->createBuilderForProperty($this->getDataClass(), $name, null, $options, $this);
return $this->getFormFactory()->createBuilderForProperty($this->getDataClass(), $name, null, $options, $this);
}

/**
Expand Down
29 changes: 19 additions & 10 deletions src/Symfony/Component/Form/FormBuilderInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ interface FormBuilderInterface extends \Traversable, \Countable, FormConfigBuild
* If you add a nested group, this group should also be represented in the
* object hierarchy.
*
* @param string|FormBuilderInterface $child
* @param string|FormTypeInterface $type
* @param array $options
* @param string|integer|FormBuilderInterface $child
* @param string|FormTypeInterface $type
* @param array $options
*
* @return FormBuilderInterface The builder object.
*/
Expand All @@ -52,6 +52,7 @@ public function create($name, $type = null, array $options = array());
* @throws Exception\FormException if the given child does not exist
*/
public function get($name);

/**
* Removes the field with the given name.
*
Expand All @@ -77,13 +78,6 @@ public function has($name);
*/
public function all();

/**
* Returns the associated form factory.
*
* @return FormFactoryInterface The factory
*/
public function getFormFactory();

/**
* Creates the form.
*
Expand All @@ -97,20 +91,35 @@ public function getForm();
* @param FormBuilderInterface $parent The parent builder
*
* @return FormBuilderInterface The builder object.
*
* @deprecated Deprecated since version 2.2, to be removed in 2.3. You
* should not rely on the parent of a builder, because it is
* likely that the parent is only set after turning the builder
* into a form.
*/
public function setParent(FormBuilderInterface $parent = null);

/**
* Returns the parent builder.
*
* @return FormBuilderInterface The parent builder
*
* @deprecated Deprecated since version 2.2, to be removed in 2.3. You
* should not rely on the parent of a builder, because it is
* likely that the parent is only set after turning the builder
* into a form.
*/
public function getParent();

/**
* Returns whether the builder has a parent.
*
* @return Boolean
*
* @deprecated Deprecated since version 2.2, to be removed in 2.3. You
* should not rely on the parent of a builder, because it is
* likely that the parent is only set after turning the builder
* into a form.
*/
public function hasParent();
}
Loading