Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms #7767

murilolobato · 2017-04-07T21:06:00Z

No description provided.

…try to activate delete_empty for collections of compound forms.

HeahDude

Thanks for opening that PR!

HeahDude · 2017-04-07T21:11:49Z

reference/forms/types/collection.rst

+
+    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>`.


Please add:

or ``empty_data`` option explicitly set to null.

We could also mention that the required or empty_data options can be defined as entry_options of the CollectionType, not necessarily in the nested type class itself.

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.

HeahDude · 2017-04-07T21:13:03Z

reference/forms/types/collection.rst

@@ -268,6 +268,13 @@ delete_empty

 **type**: ``Boolean`` **default**: ``false``

+.. caution::


The block should be added after the description.

HeahDude · 2017-04-07T21:14:08Z

reference/forms/types/collection.rst

@@ -268,6 +268,13 @@ delete_empty

 **type**: ``Boolean`` **default**: ``false``

+.. caution::
+
+    Delete empty will only remove items when the normalized value is null.


I would either say:

The ``delete_empty`` option will...

or "This option will ...".

HeahDude · 2017-04-07T21:15:12Z

reference/forms/types/collection.rst

+
+    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>`.


"inside options" could be removed, but let's wait for other reviewers opinion before rewording it.

xabbuh · 2017-04-08T09:10:32Z

reference/forms/types/collection.rst

@@ -273,6 +273,15 @@ form you have to set this option to true. However, existing collection entries
 will only be deleted if you have the allow_delete_ option enabled. Otherwise
 the empty values will be kept.

+.. caution::
+
+    The ``delete_empty`` option only removes items when the normalized  value is


there are two spaces before "value"

HeahDude

Note to merger options must be renamed entry_options when merged in 2.8.

HeahDude · 2017-04-08T11:17:39Z

reference/forms/types/collection.rst

+.. 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


Then we should be even more explicit it here, before 2.8 options may be confusing, let's use:

If the nested `type`_ is a compound form type

WDYT?

👍 sounds good to me

Actually type must be renamed entry_type in 2.8 too.

xabbuh · 2017-07-21T10:13:58Z

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 forms Commits ------- ce30e47 Applied reviewer suggestion 52c0a94 Minor fix de25ede Minor reword ca04555 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 ...

Added caution block under delete_empty to warn the developer when he …

b9e375c

…try to activate delete_empty for collections of compound forms.

carsonbot added the Status: Needs Review label Apr 7, 2017

murilolobato mentioned this pull request Apr 7, 2017

[Form] [DX] Collection of form with delete empty is not clear #7630

Closed

HeahDude reviewed Apr 7, 2017

View reviewed changes

HeahDude added the Form label Apr 7, 2017

HeahDude added this to the 2.7 milestone Apr 7, 2017

murilolobato and others added 2 commits April 8, 2017 00:37

Improved format of message and added missing information.

ca04555

Minor reword

de25ede

xabbuh reviewed Apr 8, 2017

View reviewed changes

xabbuh approved these changes Apr 8, 2017

View reviewed changes

Minor fix

52c0a94

HeahDude reviewed Apr 8, 2017

View reviewed changes

Applied reviewer suggestion

ce30e47

xabbuh added Status: Reviewed and removed Status: Needs Review labels Apr 30, 2017

xabbuh merged commit ce30e47 into symfony:2.7 Jul 21, 2017

xabbuh added a commit that referenced this pull request Jul 21, 2017

[#7767] minor rewording

a839ed4

xabbuh added a commit that referenced this pull request Jul 21, 2017

[#7767] replace "options" with "entry_options"

df78588

xabbuh added a commit that referenced this pull request Jul 21, 2017

[#7767] remove trailing space

fc70098

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms #7767

Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms #7767

murilolobato commented Apr 7, 2017

HeahDude left a comment

HeahDude Apr 7, 2017

HeahDude Apr 7, 2017

murilolobato Apr 8, 2017

HeahDude Apr 7, 2017

HeahDude Apr 7, 2017

HeahDude Apr 7, 2017

xabbuh Apr 8, 2017

HeahDude left a comment

HeahDude Apr 8, 2017

xabbuh Apr 8, 2017

HeahDude Apr 8, 2017

xabbuh commented Jul 21, 2017

		@@ -268,6 +268,13 @@ delete_empty

		type: ``Boolean`` default: ``false``

		.. caution::

Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms #7767

Added caution block under delete_empty to warn the developer when he try to activate delete_empty for collections of compound forms #7767

Conversation

murilolobato commented Apr 7, 2017

HeahDude left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HeahDude left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xabbuh commented Jul 21, 2017