8000 [Form] Reverse order of $name and $type in FormFactory::createNamed* · Issue #4342 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Reverse order of $name and $type in FormFactory::createNamed* #4342

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

Jump to bottom
Closed
webmozart opened this issue May 20, 2012 · 1 comment
Closed

[Form] Reverse order of $name and $type in FormFactory::createNamed* #4342

webmozart opened this issue May 20, 2012 · 1 comment
Labels
Enhancement Form

Comments

@webmozart
Copy link
Contributor

Currently, the createNamed* methods of FormFactoryInterface are defined like this:

function createNamed($type, $name, $data = null, array $options = array(), FormBuilder $parent = null);
function createNamedBuilder($type, $name, $data = null, array $options = array(), FormBuilder $parent = null);

This is very confusing since the parameters $type and $name are defined in FormBuilder::add() in the exact opposite order:

$builder->add('field', 'text');
// but:
$form = $factory->createNamed('text', 'field');

I think that the order of the parameters should be switched in order to make usage of the FormFactory methods more intuitive. This would break BC.

Thoughts?
@vicb
Copy link
Contributor
vicb commented May 20, 2012

👍 yes please (I didn't propose before as it breaks BC but this is the right time for it I think).

@webmozart webmozart mentioned this issue May 23, 2012
Merged
fabpot added a commit that referenced this issue May 25, 2012
@fabpot
merged branch bschussek/issue3879 (PR #4387)
023dbf8 
Commits
-------

bc15e2d [Form] Some code cleanup
94f6f77 Restructured Form section of UPGRADE
3d800af [Form] Remove usages of deprecated features
ee803cd [Form] Renamed setVars() to addVars() in FormViewInterface
1c4f632 [Form] Fixed API docs and usage of FormBuilderInterface instead of FormBuilder
2e6cdd1 [Form] Inverted the logic of "single_control" and renamed it to "compound". The opposite is now "simple".
98a7c0c [Form] Consolidated FormInterface, FormBuilderInterface and FormViewInterface
8e128fc [Form][OptionsResolver] Fixed typos
877d8f7 [Form] Reversed the order of $type and $name in FormFactory::createNamed[Builder]()
33fecca [Form] Merged various form events and added class FormEvent
bec8015 [Form] Renamed client and application format to view and model format
8cae328 [Form] setDefaultOptions() is now coded against OptionsResolverInterface
1ecddbc [OptionsResolver] Renamed recommended method to setDefaultOptions()
dc2fa9d [OptionsResolver] Added OptionsResolverInterface
2cd99e8 [Form] Added FormBuilderInterface and FormViewInterface and cleaned up FormTypeInterface and FormTypeExtensionInterface
0ef4066 [Form] Options are now passed to buildView() and buildViewBottomUp()
027259e [Form] Changed getDefaultOptions() to setDefaultOptions(OptionsResolver $resolver) in FormTypeInterface
b4e8bcf [OptionsResolver] Relaxed tests to check that allowed values can also be passed as scalars
97de004 [OptionsResolver] Added option type validation capabilities
0af5f06 [OptionsResolver] Added method setFilters() for augmenting the final option values

Discussion
----------

[Form] Cleaned up the Form API

Bug fix: no
Feature addition: no
Backwards compatibility break: **YES**
Symfony2 tests pass: yes
Fixes the following tickets: #3855, #3879, #4342, #4371, #4375
Todo: -

This PR cleans up the Form API as described in the referenced tickets in order to stabilize and freeze this API in the future. BC is kept wherever possible. After this PR, form types are expected to follow the following interface:

```php
<?php

class MyType extends AbstractType
{
	public function buildForm(FormBuilderInterface $builder, array $options)
	{
	}

	public function buildView(FormViewInterface $view, FormInterface $form, array $options)
	{
	}

	public function finishView(FormViewInterface $view, FormInterface $form, array $options)
	{
	}

	public function setDefaultOptions(OptionsResolverInterface $resolver)
	{
	}

	public function getParent()
	{
	    return 'form';
	}

	public function getName()
	{
	    return 'my_type';
	}
}
```

Note that the options are now passed to `buildView` and `finishView` (formerly `buildViewBottomUp`) as well, reducing the need for creating form attributes in most cases.

---------------------------------------------------------------------------

by travisbot at 2012-05-23T19:07:44Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414486) (merged 277f5f78 into e023807).

---------------------------------------------------------------------------

by bschussek at 2012-05-23T19:51:40Z

The PR now also contains the fix for #4342.

---------------------------------------------------------------------------

by travisbot at 2012-05-23T19:51:55Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414932) (merged 13d284ba into e023807).

---------------------------------------------------------------------------

by travisbot at 2012-05-24T06:55:35Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1419776) (merged 5aba0778 into e023807).

---------------------------------------------------------------------------

by travisbot at 2012-05-24T06:56:28Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1419783) (merged 00c4f7e into b07fb3c).

---------------------------------------------------------------------------

by travisbot at 2012-05-24T12:26:25Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1421748) (merged 73cd9f8 into b07fb3c).

---------------------------------------------------------------------------

by bschussek at 2012-05-24T12:27:32Z

The FormView changes described in #4371 are now contained as well. Invitation for final review.

---------------------------------------------------------------------------

by travisbot at 2012-05-24T14:03:10Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1422116) (merged e1f502b into b07fb3c).

---------------------------------------------------------------------------

by cordoval at 2012-05-25T03:26:05Z

any ETA @bschussek ? I want to use it for tackling the problem of dynamic selects city-state
here http://www.craftitonline.com/2011/08/symfony2-ajax-form-republish/

I am told:
"getDefaultOptions is changed to setDefaultOptions which will allow you to set depenedent values based on other forms"

so I am most interested +300!

---------------------------------------------------------------------------

by bschussek at 2012-05-25T06:08:53Z

@cordoval I think you misunderstood this. The OptionsResolver allows you to set options dependent on other options, but of the same field. Also, this is already possible before this merge by setting an option to a closure as described in the README of the OptionsResolver component.

---------------------------------------------------------------------------

by travisbot at 2012-05-25T06:35:53Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1430534) (merged b61cc55 into b07fb3c).

--------------------
6263
-------------------------------------------------------

by vicb at 2012-05-25T06:42:24Z

@bschussek great changes ! I have just sent you a PR with some modifs related to deprecated features. I'll rebase and submit the other one we have already discussed.

---------------------------------------------------------------------------

by travisbot at 2012-05-25T07:16:18Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1430699) (merged e18830d into b07fb3c).

---------------------------------------------------------------------------

by cordoval at 2012-05-25T07:19:07Z

@bschussek what is already possilble @ "Also, this is already possible before"

I am confused could you link to what you are referring to please?

---------------------------------------------------------------------------

by travisbot at 2012-05-25T07:22:07Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1430727) (merged 20c02a7 into b07fb3c).

---------------------------------------------------------------------------

by bschussek at 2012-05-25T07:22:29Z

```
public function getDefaultOptions()
{
    return array(
        'a' => 'foo',
        'b' => function (Options $options) {
            return 'bar' === $options['a'] ? 'x' : 'y';
        }
    );
}

---------------------------------------------------------------------------

by travisbot at 2012-05-25T10:38:04Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1431903) (merged bc15e2d into 45849ce).
@fabpot fabpot closed this as completed May 25, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement Form
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @webmozart @vicb
0