8000 [Form] Added an alternative signature Form::add($name, $type, $options) by webmozart · Pull Request #6355 · symfony/symfony · GitHub
[go: up one dir, main page]
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 1 commit
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
Next Next commit
[Form] Added an alternative signature Form::add($name, $type, $options)
  • Loading branch information
@webmozart
webmozart committed Dec 18, 2012
commit fb71964adc69690844fca1dd2a035dac24eaa8a9
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((string) $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((string) $name, $this->type, array_replace(array(
Copy link
Contributor

Choose a reason for hiding this comment

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

The form should probably also accept an integer as name, which makes it more fault-tolerant (instead of throwing an exception).
The Form::has does also accept an integer as there is no difference between $array[1] or $array['1']. Furthermore you would not need to cast to string here.

Copy link
Member

Choose a reason for hiding this comment

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

casting is necessary because of the check in Form::add: an integer would trigger the exception line 862

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes thats what I'd like to see improved...

Copy link
Member

Choose a reason for hiding this comment

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

Well, this condition is the same than when adding a child through the builder: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/FormBuilder.php#L96

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed Form::add() and FormBuilder::add() to accept numerics.

'property_path' => '['.$name.']',
), $this->options)));
), $this->options));
}
}
}
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 EED3 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)) {
throw new UnexpectedTypeException($child, 'string or Symfony\Component\Form\FormInterface');
Copy link
Contributor

Choose a reason for hiding this comment

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

missing phpdoc for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

}

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

F438 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
21 changes: 3 additions & 18 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 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
23 changes: 16 additions & 7 deletions src/Symfony/Component/Form/FormBuilderInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
27 changes: 27 additions & 0 deletions src/Symfony/Component/Form/FormConfigBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ class FormConfigBuilder implements FormConfigBuilderInterface
*/
private $dataLocked;

/**
* @var FormFactoryInterface
*/
private $formFactory;

/**
* @var array
*/
Expand Down Expand Up @@ -611,6 +616,14 @@ public function getDataLocked()
return $this->dataLocked;
}

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

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -849,6 +862,20 @@ public function setDataLocked($locked)
return $this;
}

/**
* {@inheritdoc}
*/
public function setFormFactory(FormFactoryInterface $formFactory)
{
if ($this->locked) {
throw new FormException('The config builder cannot be modified anymore.');
}

$this->formFactory = $formFactory;

return $this;
}

/**
* {@inheritdoc}
*/
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Component/Form/FormConfigBuilderInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,13 @@ public function setData($data);
*/
public function setDataLocked($locked);

/**
* Sets the form factory used for creating new forms.
*
* @param FormFactoryInterface $formFactory The form factory.
*/
public function setFormFactory(FormFactoryInterface $formFactory);

/**
* Builds and returns the form configuration.
*
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Component/Form/FormConfigInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ public function getDataClass();
*/
public function getDataLocked();

/**
* Returns the form factory used for creating new forms.
*
* @return FormFactoryInterface The form factory.
*/
public function getFormFactory();

/**
* Returns all options passed during the construction of the form.
*
Expand Down
11 changes: 7 additions & 4 deletions src/Symfony/Component/Form/FormInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,17 @@ public function getParent();
/**
* Adds a child to the form.
*
* @param FormInterface $child The FormInterface to add as a child
* @param FormInterface|string $child The FormInterface instance or the name of the child.
* @param string|null $type The child's type, if a name was passed.
* @param array $options The child's options, if a name was passed.
*
* @return FormInterface The form instance
*
* @throws Exception\AlreadyBoundException If the form has already been bound.
* @throws Exception\FormException When trying to add a child to a non-compound form.
* @throws Exception\AlreadyBoundException If the form has already been bound.
* @throws Exception\FormException When trying to add a child to a non-compound form.
* @throws Exception\UnexpectedTypeException If $child or $type has an unexpected type.
*/
public function add(FormInterface $child);
public function add($child, $type = null, array $options = array());

/**
* Returns the child with the given name.
Expand Down
Loading
0