-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Rename "virtual" to "inherit_data" #5899
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
Comments
+1 from me; I guess we cannot support both for BC? |
@schmittjoh We will support both until 2.3. |
-1 on dropping virtual. There are many tools using the
|
@havvg This was the reason for choosing that name in the first place. The problem is that people can't apply their knowledge from databases here. They first have to learn how data mapping works in order to learn what "virtual" means in the form context. Once they know about data mapping, "inherit_data" should be much more intuitive. |
+1, reasonable naming |
-1, "inherit_data" does not sound much better to me. "Mixin" or "Trait" would probably be better than "Inherit" in term of OOP. |
@vicb I don't see why mixin would be better. It is inheriting the data from the parent form. |
@vicb A virtual form is essentially the same as a form with $builder->add('sub', 'form', array(
'property_path' => '..',
)); only that this syntax doesn't (and can't) exist. So I really think that an option name related to data mapping should make it easier to understand (even if you like to think of such a form as mixin or trait). |
@bschussek your comment made me think of something: <?php
$builder->add('sub', 'form', array(
'property_path' => '..',
)); but <?php
$builder->include('form'); would be possible with a helper |
but the |
This PR was merged into the master branch. Discussion ---------- [2.3] [Form] Renamed option "virtual" to "inherit_data" and improved handling of such forms Bug fix: yes Feature addition: yes Backwards compatibility break: yes Symfony2 tests pass: yes Fixes the following tickets: #5899, #5720, #5578 Todo: - License of the code: MIT Documentation PR: symfony/symfony-docs#2107 This PR renames the option "virtual" to "inherit_data" for more clarity (the old option is deprecated and usable until 2.3). It also fixes the behavior of forms having that option set. Forms with that option set will now correctly return their parents' data from `getData()`, `getNormData()` and `getViewData()`. Furthermore, `getPropertyPath()` was fixed for forms that inherit their parent data. Commits ------- 1290b80 [Form] Fixed the deprecation notes for the "virtual" option ac2ca44 [Form] Moved parent data inheritance from data mappers to Form 8ea5e1a [Form] Renamed option "virtual" to "inherit_data"
I propose to rename the "virtual" option to "inherit_data". The reasoning is quite simple: While it is necessary to explain what a "virtual form" is, the name "inherit_data" is self-explanatory, namely that the form inherits the data from its parent form.
Fieldsets can then be added to core by extending form, setting "inherit_data" to true and adding custom rendering code.
Thoughts?
Update:
BC will be kept until 2.3.
The text was updated successfully, but these errors were encountered: