Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms#7767
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…try to activate delete_emptyfor collections of compound forms.
HeahDude left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for opening that PR!
reference/forms/types/collection.rst Outdated
| Delete empty will only remove items when the normalized value is null. | ||
| If your `type`_ is a compound form type, you need to have the:ref:`required<reference-form-option-required>` | ||
| option set to ``false`` inside `options`_, See:ref:`empty_data<reference-form-option-empty-data>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please add:
or ``empty_data`` option explicitly set to null.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could also mention that therequired orempty_data options can be defined asentry_options of theCollectionType, not necessarily in the nested type class itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In the above text, when I mean 'options' I'm referring to 'entry_options', it is because I'm making the pull request to syfmony 2.7.
reference/forms/types/collection.rst Outdated
| **type**: ``Boolean`` **default**: ``false`` | ||
| ..caution:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The block should be added after the description.
reference/forms/types/collection.rst Outdated
| ..caution:: | ||
| Delete empty will only remove items when the normalized value is null. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would either say:
The ``delete_empty`` option will...or "This option will ...".
reference/forms/types/collection.rst Outdated
| Delete empty will only remove items when the normalized value is null. | ||
| If your `type`_ is a compound form type, you need to have the:ref:`required<reference-form-option-required>` | ||
| option set to ``false`` inside `options`_, See:ref:`empty_data<reference-form-option-empty-data>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"inside options" could be removed, but let's wait for other reviewers opinion before rewording it.
reference/forms/types/collection.rst Outdated
| ..caution:: | ||
| The ``delete_empty`` option only removes items when the normalized value is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
there are two spaces before "value"
HeahDude left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Note to mergeroptions must be renamedentry_options when merged in 2.8.
reference/forms/types/collection.rst Outdated
| ..caution:: | ||
| The ``delete_empty`` option only removes items when the normalized value is | ||
| ``null``. If your `type`_ is a compound form type, you must either set the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Then we should be even more explicit it here, before 2.8options may be confusing, let's use:
If the nested `type`_ is a compound form typeWDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍 sounds good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actuallytype must be renamedentry_type in 2.8 too.
xabbuh commentedJul 21, 2017
Thank you@murilolobato. |
…per when he try to activate delete_empty for collections of compound forms (murilolobato, javiereguiluz)This PR was merged into the 2.7 branch.Discussion----------Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound formsCommits-------ce30e47 Applied reviewer suggestion52c0a94 Minor fixde25ede Minor rewordca04555 Improved format of message and added missing information.b9e375c Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms.
* 2.7: [#7767] minor rewording [#8047] add inline code comment Fixed the issue in a different way Jquery datePicker syntax update [#8104] minor rewording Add more precision about automatic provider assignation Update data.rst.inc Minor reword to explain that path() generates absolute URLs Added a caution note about UploadedFile and insulated tests Postpone talk about front controllers Applied reviewer suggestion Minor fix Minor reword Improved format of message and added missing information. Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms.
* 2.7: Reworded the article about form login redirects Explained the edge-case where the use_referer option doesn't work [#7572] fix wording [#7585] remove trailing whitespaces [#7585] minor rewording Fixed a typo Fixed a typo Update parent_services for tip consistency [#7685] use the method role Minor change Updating doc to specify priority of default normalizer [#7767] remove trailing space Minor reword Moved array validation to the Raw values sub-guide Fixed some syntax issues missing constraint example from the old readme Update console dialog helper documentation Improved the advantages/drawbacks of "controllers as services"
* 2.8: (37 commits) [#8192] use path() in PHP templates Reworded the article about form login redirects Explained the edge-case where the use_referer option doesn't work [#7572] fix wording [#7585] remove trailing whitespaces [#7585] minor rewording Fixed a typo Fixed a typo Update parent_services for tip consistency [#7685] use the method role Minor change Updating doc to specify priority of default normalizer [#7767] remove trailing space [#7767] replace "options" with "entry_options" [#7767] minor rewording [#8047] add inline code comment Fixed the issue in a different way Jquery datePicker syntax update [#8104] minor rewording Add more precision about automatic provider assignation ...
* 3.2: (38 commits) [#8192] use path() in PHP templates Reworded the article about form login redirects Explained the edge-case where the use_referer option doesn't work [#7572] fix wording [#7585] remove trailing whitespaces [#7585] minor rewording Fixed a typo Fixed a typo Update parent_services for tip consistency [#7685] use the method role Minor change Updating doc to specify priority of default normalizer [#7767] remove trailing space [#7767] replace "options" with "entry_options" [#7767] minor rewording [#8047] add inline code comment Fixed the issue in a different way Jquery datePicker syntax update Fix framework instantiation in event-dispatcher [#8104] minor rewording ...
* 3.3: (46 commits) [#8192] use path() in PHP templates Reworded the article about form login redirects Update Flex documentation with latest structure Explained the edge-case where the use_referer option doesn't work [#7572] fix wording [#7585] remove trailing whitespaces [#7585] minor rewording Fixed a typo Fixed a typo Update parent_services for tip consistency [#7685] use the method role Minor change Updating doc to specify priority of default normalizer [#7767] remove trailing space [#7767] replace "options" with "entry_options" [#7767] minor rewording [#8047] add inline code comment Fixed the issue in a different way Jquery datePicker syntax update Fix framework instantiation in event-dispatcher ...
* 3.4: (48 commits) [#8192] use path() in PHP templates Reworded the article about form login redirects Update Flex documentation with latest structure Explained the edge-case where the use_referer option doesn't work [#7572] fix wording [#7585] remove trailing whitespaces [#7585] minor rewording Fixed a typo Fixed a typo Update parent_services for tip consistency [#7685] use the method role Minor change Updating doc to specify priority of default normalizer [#7767] remove trailing space [#7767] replace "options" with "entry_options" [#7767] minor rewording [#8047] add inline code comment Fixed the issue in a different way Jquery datePicker syntax update Fix framework instantiation in event-dispatcher ...
No description provided.