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] Document CollectionType's prototype_data option #4367

Closed
wants to merge 1 commit into from

Conversation

kgilden
Copy link
Contributor

@kgilden kgilden commented Oct 24, 2014

Q A
Doc fix? no
New docs? yes (PR symfony/symfony#12314)
Applies to 2.8+
Fixed tickets -

@kgilden kgilden changed the title [Form] Document CollectionType's prototype_data [WCM][Form] Document CollectionType's prototype_data Oct 24, 2014
@kgilden kgilden changed the title [WCM][Form] Document CollectionType's prototype_data [WCM][Form] Document CollectionType's prototype_data option Oct 24, 2014
webmozart added a commit to symfony/symfony that referenced this pull request Jun 17, 2015
…(kgilden)

This PR was submitted for the master branch but it was merged into the 2.8 branch instead (closes #12314).

Discussion
----------

[Form] Add "prototype_data" option to collection type

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #5095
| License       | MIT
| Doc PR        | symfony/symfony-docs#4367

Makes it possible to have default data for collection prototypes.

Commits
-------

80b0a80 [Form] Add "prototype_data" option to collection type
@kgilden kgilden changed the title [WCM][Form] Document CollectionType's prototype_data option [Form] Document CollectionType's prototype_data option Jun 17, 2015
@kgilden
Copy link
Contributor Author

kgilden commented Jun 17, 2015

Alright @wouterj this should be ready for merging. Should I make a PR against 2.8 instead though?

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

Allows you to specify data for the prototype. This is useful if you want to
have the fields filled with some basic values.
Copy link
Member

Choose a reason for hiding this comment

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

A very minor suggestion: would it better to say ... some initial values. instead of ... some basic values.?

Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if everybody will actually understand how to use this option. Would it be more clear how to use it if added a sentence explaining that what you configure here will be passed to the data option of the embedded type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I like some initial values better too. To address xabbuh's concerns - what about something along the lines of Each new row added will initially contain the data set by this option?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kgilden i like the sentence with the row...

Copy link
Member

Choose a reason for hiding this comment

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

I agree. That's a nice suggestion.

@peterrehm
Copy link
Contributor

Any update on that?

@OskarStark
Copy link
Contributor

@kgilden can you finish this PR soon?

@kgilden kgilden force-pushed the add-prototype-data branch 2 times, most recently from 5355d8b to 8364f55 Compare October 21, 2015 08:58
@kgilden
Copy link
Contributor Author

kgilden commented Oct 21, 2015

@OskarStark @xabbuh updated the PR and rebased on top of master. Or should I rebase on top of 2.8 now that it exists?

@stof
Copy link
Member

stof commented Oct 21, 2015

@kgilden as your PR targets the master branch, keep it based on the master branch to have a clean diff. The doc mergers will switch the target to 2.8 when merging. Rebasing it on 2.8 may cause some weird diff to appear here in case 2.8 is not fully merged into master.

@OskarStark
Copy link
Contributor

thank you for the explanation @stof

thank you for rebasing @kgilden

@OskarStark
Copy link
Contributor

Status: Reviewed

ping @xabbuh

@javiereguiluz
Copy link
Member

@kgilden thanks for providing this doc. Although your explanation is correct, I find it a bit lacking. Could you please add some real examples of this option in use? Thanks.

@ghost
Copy link

ghost commented Jan 13, 2016

ping @kgilden

@HeahDude
Copy link
Contributor

ping :)

@HeahDude
Copy link
Contributor

HeahDude commented Mar 2, 2016

@xabbuh, this one needs to get merged in master so I can rebase #6319 and add the deprecation note.

@HeahDude
Copy link
Contributor

HeahDude commented Mar 2, 2016

Also it should mention that the default value is the data value set in entry_options, something like :

When ``null`` the default prototype data is the same as set in the ``_entry_options:`` option.

@xabbuh
Copy link
Member

xabbuh commented Apr 11, 2016

I am closing here in favor of #6450. Thank you @kgilden for starting it.

@xabbuh xabbuh closed this Apr 11, 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`
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.

8 participants