Skip to content
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

[Form] added prototype_data option in CollectionType #6450

Merged
merged 2 commits into from
Apr 18, 2016

Conversation

HeahDude
Copy link
Contributor

@HeahDude HeahDude commented Apr 9, 2016

Q A
Branch 2.8+
New doc finishes #4367

@HeahDude HeahDude force-pushed the fix/form_type-prototype_data_option branch 2 times, most recently from befaeb4 to 6f15771 Compare April 9, 2016 13:57

**type**: ``mixed`` **default**: ``null``

Allows you to define a specific data for the prototype. Each new row added will
Copy link
Member

Choose a reason for hiding this comment

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

[...] define specific data [...]

@xabbuh
Copy link
Member

xabbuh commented Apr 11, 2016

We need a versionadded directive for this option.

use Symfony\Component\Form\Extension\Core\Type\TextType;

$builder->add('tags', CollectionType::class, array(
'class' => \AppBundle\Entity\Tag::class,
Copy link
Member

Choose a reason for hiding this comment

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

For the Symfony 2.8 docs we always use a string to be compatible with PHP 5.3 and 5.4.

@HeahDude
Copy link
Contributor Author

@xabbuh Thanks for the review, comments addressed.

.. code-block:: php

use Symfony\Component\Form\Extension\Core\Type\CollectionType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
Copy link
Member

Choose a reason for hiding this comment

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

The use statements are useless now. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I made another commit to do that change in all examples, so you can easily revert it in 3.0+ branches. Thanks!

@HeahDude HeahDude force-pushed the fix/form_type-prototype_data_option branch from 1b5e46e to 2034e96 Compare April 11, 2016 09:26
@xabbuh
Copy link
Member

xabbuh commented Apr 11, 2016

👍

// ...

$builder->add('emails', CollectionType::class, array(
$builder->add('emails', 'Symfony\Component\Form\Extension\Core\Type\CollectionType', array(
Copy link
Member

Choose a reason for hiding this comment

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

Why the change away from the CollectionType::class? That option (with ::class) seems a little easier because we can use IDE auto-completion. Or have we been changing this elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weaverryan, the discussion with @xabbuh is on the outdated diff: #6450 (comment)
Because 2.8 supports PHP < 5.5, I made a different commit for those changes so it can be reverted in 3.0.
Thanks :)

Copy link
Member

Choose a reason for hiding this comment

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

@HeahDude It looks like I was mistaken here and we at some point agreed on using the class constant even in the 2.8 docs (see http://symfony.com/doc/2.8/book/forms.html, for example). Sorry for the confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xabbuh No worry, I just removed the last commit :)

@HeahDude HeahDude force-pushed the fix/form_type-prototype_data_option branch from 2034e96 to 3c08e10 Compare April 13, 2016 06:55
// ...

$builder->add('tags', CollectionType::class, array(
'class' => 'AppBundle\Entity\Tag',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xabbuh, I intentionally didn't change this one since it's almost the same:
'AppBundle\Entity\Tag' VS \AppBundle\Entity\Tag::class unless we add a use statement:

use AppBundle\Entity\Tag;

// ...
'class' => Tag::class

What do you like ?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I was a bit confused and I am even more confused now. Where from did you take the class option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, confusion with EntityType! Should be entry_type data_class in entry_options, I'll just remove it :)

Copy link
Member

Choose a reason for hiding this comment

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

Oh okay, but entry_type is already set to refer to the TextType, isn't 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.

Yes I answered too quickly and edited it.

@HeahDude HeahDude force-pushed the fix/form_type-prototype_data_option branch from 3c08e10 to 95bda57 Compare April 14, 2016 16:57
@HeahDude
Copy link
Contributor Author

Ok should be good now.

@xabbuh
Copy link
Member

xabbuh commented Apr 14, 2016

Cool 👍

@wouterj wouterj merged commit 95bda57 into symfony:2.8 Apr 18, 2016
wouterj added a commit that referenced this pull request Apr 18, 2016
…gilden, HeahDude)

This PR was merged into the 2.8 branch.

Discussion
----------

[Form] added prototype_data option in CollectionType

| Q | A |
|----|----|
| Branch | 2.8+ |
| New doc | finishes #4367 |

Commits
-------

95bda57 [Form] defined default and added example CollectionType `prototype_data` option
e18dc1f [Form] Document CollectionType's `prototype_data`
wouterj added a commit that referenced this pull request Apr 18, 2016
@wouterj
Copy link
Member

wouterj commented Apr 18, 2016

Thanks for your work on this @HeahDude and @kgilden! It's finally merged now.

Fyi, I've added a reference to the entry_options setting in 326ae53.

@HeahDude
Copy link
Contributor Author

Nice addition, thank you @wouterj! Also thanks to @kgilden @xabbuh

@HeahDude HeahDude deleted the fix/form_type-prototype_data_option branch April 18, 2016 21:31
wouterj added a commit to wouterj/symfony-docs that referenced this pull request Apr 20, 2016
wouterj added a commit that referenced this pull request Apr 20, 2016
wouterj added a commit that referenced this pull request Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants