Skip to content

Commit 5e0823c

Browse files
fabpotfabpot
committed
merged branch bschussek/issue1919 (PR #3156)
Commits ------- 8dc78bd [Form] Fixed YODA issues 600cec7 [Form] Added missing entries to CHANGELOG and UPGRADE b154f7c [Form] Fixed docblock and unneeded use statement 399af27 [Form] Implemented checks to assert that values and indices generated in choice lists match their requirements 5f6f75c [Form] Fixed outstanding issues mentioned in the PR 7c70976 [Form] Fixed text in UPGRADE file c26b47a [Form] Made query parameter name generated by ORMQueryBuilderLoader unique 18f92cd [Form] Fixed double choice fixing f533ef0 [Form] Added ChoiceView class for passing choice-related data to the view d72900e [Form] Incorporated changes suggested in PR comments 28d2f6d Removed duplicated lines from UPGRADE file e1fc5a5 [Form] Restricted form names to specific characters to (1) fix generation of HTML IDs and to (2) avoid problems with property paths. 87b16e7 [Form] Greatly improved ChoiceListInterface and all of its implementations Discussion ---------- [Form] Improved ChoiceList implementation and made form naming more restrictive Bug fix: yes Feature addition: yes Backwards compatibility break: **yes** Symfony2 tests pass: yes Fixes the following tickets: #2869, #3021, #1919, #3153 Todo: adapt documentation ![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1919) The changes in this PR are primarily motivated by the fact that invalid form/field names lead to various problems. 1. When a name contains any characters that are not permitted in HTML "id" attributes, these are invalid 2. When a name contains periods ("."), form validation is broken, because they confuse the property path resolution 3. Since choices in expanded choice fields are directly translated to field names, choices applying to either 1. or 2. lead to problems. But choices should be unrestricted. 4. Unless a choice field is not expanded and does not allow multiple selection, it is not possible to use empty strings as choices, which might be desirable in some occasions. The solution to these problems is to * Restrict form names to disallow unpermitted characters (solves 1. and 2.) * Generate integer indices to be stored in the HTML "id" and "name" attributes and map them to the choices (solves 3.). Can be reverted to the old behaviour by setting the option "index_generation" to ChoiceList::COPY_CHOICE * Generate integer values to be stored in the HTML "value" attribute and map them to the choices (solves 4.). Can be reverted to the old behaviour by setting the option "value_generation" to ChoiceList::COPY_CHOICE Apart from these fixes, it is now possible to write more flexible choice lists. One of these is `ObjectChoiceList`, which allows to use objects as choices and is bundled in the core. `EntityChoiceList` has been made an extension of this class. $form = $this->createFormBuilder() ->add('object', 'choice', array( 'choice_list' => new ObjectChoiceList( array($obj1, $obj2, $obj3, $obj4), // property path determining the choice label (optional) 'name', // preferred choices (optional) array($obj2, $obj3), // property path for object grouping (optional) 'category', // property path for value generation (optional) 'id', // property path for index generation (optional) 'id' ) )) ->getForm() ; --------------------------------------------------------------------------- by kriswallsmith at 2012-01-19T18:09:09Z Rather than passing `choices` and a `choice_labels` arrays to the view would it make sense to introduce a `ChoiceView` class and pass one array of objects? --------------------------------------------------------------------------- by stof at 2012-01-22T15:32:36Z @bschussek can you update your PR according to the feedback (and rebase it as it conflicts according to github) ? --------------------------------------------------------------------------- by bschussek at 2012-01-24T00:15:42Z @kriswallsmith fixed Fixed all outstanding issues. Would be glad if someone could review again, otherwise this PR is ready to merge. --------------------------------------------------------------------------- by fabpot at 2012-01-25T15:17:59Z Is it ready to be merged? --------------------------------------------------------------------------- by Tobion at 2012-01-25T15:35:50Z Yes I think so. He said it's ready to be merged when reviewed. --------------------------------------------------------------------------- by bschussek at 2012-01-26T02:30:36Z Yes. --------------------------------------------------------------------------- by bschussek at 2012-01-28T12:39:00Z Fixed outstanding issues. Ready for merge.
2 parents eb62f12 + 8dc78bd commit 5e0823c

File tree

74 files changed

+3417
-1601
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+3417
-1601
lines changed

‎CHANGELOG-2.1.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,45 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
166166
* allowed setting different options for RepeatedType fields (like the label)
167167
* added support for empty form name at root level, this enables rendering forms
168168
without form name prefix in field names
169+
170+
* [BC BREAK] made form naming more restrictive. Form and field names must
171+
start with a letter, digit or underscore and only contain letters, digits,
172+
underscores, hyphens and colons
173+
174+
* [BC BREAK] changed default name of the prototype in the "collection" type
175+
from "$$name$$" to "__name__". No dollars are appended/prepended to custom
176+
names anymore.
177+
178+
* [BC BREAK] greatly improved `ChoiceListInterface` and all of its
179+
implementations. `EntityChoiceList` was adapted, the methods `getEntities()`,
180+
`getEntitiesByKeys()`, `getIdentifier()` and `getIdentifierValues()` were
181+
removed/made private. Instead of the first two you can use `getChoices()`
182+
and `getChoicesByValues()`, for the latter two no replacement exists.
183+
`ArrayChoiceList` was replaced by `SimpleChoiceList`.
184+
`PaddedChoiceList`, `MonthChoiceList` and `TimezoneChoiceList` were removed.
185+
Their functionality was merged into `DateType`, `TimeType` and `TimezoneType`.
186+
187+
* [BC BREAK] removed `EntitiesToArrayTransformer` and `EntityToIdTransformer`.
188+
The former has been replaced by `CollectionToArrayTransformer` in combination
189+
with `EntityChoiceList`, the latter is not required in the core anymore.
190+
191+
* [BC BREAK] renamed
192+
193+
* `ArrayToBooleanChoicesTransformer` to `ChoicesToBooleanArrayTransformer`
194+
* `ScalarToBooleanChoicesTransformer` to `ChoiceToBooleanArrayTransformer`
195+
* `ArrayToChoicesTransformer` to `ChoicesToValuesTransformer`
196+
* `ScalarToChoiceTransformer` to `ChoiceToValueTransformer`
197+
198+
to be consistent with the naming in `ChoiceListInterface`.
199+
200+
* [BC BREAK] removed `FormUtil::toArrayKey()` and `FormUtil::toArrayKeys()`.
201+
They were merged into `ChoiceList` and have no public equivalent anymore.
202+
203+
* added `ComplexChoiceList` and `ObjectChoiceList`. Both let you select amongst
204+
objects in a choice field, but feature different constructors.
205+
* choice fields now throw a `FormException` if neither the "choices" nor the
206+
"choice_list" option is set
207+
* the radio field is now a child type of the checkbox field
169208

170209
### HttpFoundation
171210

‎UPGRADE-2.1.md

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,68 @@ UPGRADE FROM 2.0 to 2.1
7676
If you don't want to set the `Valid` constraint, or if there is no reference
7777
from the data of the parent form to the data of the child form, you can
7878
enable BC behaviour by setting the option "cascade_validation" to `true` on
79-
the parent form.
79+
the parent form.
80+
81+
* The strategy for generating the HTML attributes "id" and "name"
82+
of choices in a choice field has changed
83+
84+
Instead of appending the choice value, a generated integer is now appended
85+
by default. Take care if your Javascript relies on that. If you can
86+
guarantee that your choice values only contain ASCII letters, digits,
87+
letters, colons and underscores, you can restore the old behaviour by
88+
setting the option "index_strategy" of the choice field to
89+
`ChoiceList::COPY_CHOICE`.
90+
91+
* The strategy for generating the HTML attributes "value" of choices in a
92+
choice field has changed
93+
94+
Instead of using the choice value, a generated integer is now stored.
95+
Again, take care if your Javascript reads this value. If your choice field
96+
is a non-expanded single-choice field, or if the choices are guaranteed not
97+
to contain the empty string '' (which is the case when you added it manually
98+
or when the field is a single-choice field and is not required), you can
99+
restore the old behaviour by setting the option "value_strategy" to
100+
`ChoiceList::COPY_CHOICE`.
101+
102+
* In the template of the choice type, the structure of the "choices" variable
103+
has changed
104+
105+
"choices" now contains ChoiceView objects with two getters `getValue()`
106+
and `getLabel()` to access the choice data. The indices of the array
107+
store an index whose generation is controlled by the "index_generation"
108+
option of the choice field.
109+
110+
Before:
111+
112+
{% for choice, label in choices %}
113+
<option value="{{ choice }}"{% if _form_is_choice_selected(form, choice) %} selected="selected"{% endif %}>
114+
{{ label }}
115+
</option>
116+
{% endfor %}
117+
118+
After:
119+
120+
{% for choice in choices %}
121+
<option value="{{ choice.value }}"{% if _form_is_choice_selected(form, choice) %} selected="selected"{% endif %}>
122+
{{ choice.label }}
123+
</option>
124+
{% endfor %}
125+
126+
* In the template of the collection type, the default name of the prototype
127+
field has changed from "$$name$$" to "__name__"
128+
129+
For custom names, no dollars are prepended/appended anymore. You are advised
130+
to prepend and append double underscores wherever you have configured the
131+
prototype name manually.
132+
133+
Before:
134+
135+
$builder->add('tags', 'collection', array('prototype' => 'proto'));
136+
137+
// results in the name "$$proto$$" in the template
138+
139+
After:
140+
141+
$builder->add('tags', 'collection', array('prototype' => '__proto__'));
142+
143+
// results in the name "__proto__" in the template

0 commit comments

Comments
 (0)