Description
Previous works and discussions
The famous #14050.
But also #17019, symfony/symfony-docs#6144, symfony/symfony-docs#6393 and #16834.
Current definition
Since #14050, choice_value
, choice_label
, choice_name
, choice_attr
, preferred_choices
and group_by
options are dynamics.
Here's the current allowed types defined in ChoiceType
:
choice_value => null, callable, string (or PropertyPath)
choice_name => null, callable, string (or PropertyPath)
choice_label => null, callable, string (or PropertyPath), bool (false to discard label)
choice_attr => null, callable, string (or PropertyPath), array
preferred_choices => callable, string (or PropertyPath), array or \Traversable
group_by => null, callable, string (or PropertyPath), (use nested choices by default)
The current signatures of the callables are the same for all: function ($choice, $key, $value)
Excepted for choice_value
: function ($choice)
.
Now considering the needed returned types from the callables or property paths:
choice_label => string
choice_value => string (unique AND not null or falls back on i++)
choice_name => string (unique AND valid html "id" or falls back on i++)
choice_attr => array
preferred_choices => bool (preferred)
group_by => string (group name) | null (no group)
Problem
-
It's actually impossible to add a
choice_label_attr
option in the core following this model without breaking BC (ref discussion in [Form] added "choice_label_attr" option and deprecated "choice_attr" as multi-arrays or property path #16834 and 616fc8a for the currently "consistent" implementation).
Worse, it can't be easily implemented in user land.
For example, in one of my first project with symfony last year (2.7 first betas), I found how to "hack"choice_attr
(see here).
(When I updated this project last week on 3.0, I figured that it can be done in a BC way but without support for property path, which lead me to:) -
Of courses it makes much sense to use property paths to return string or bool, especially when they are relevant properties of a choice object:
$builder->add('tags', ChoiceType::class, array( 'choices' => $tags, 'choice_value' => 'id', // (string) $tag->getId() 'choice_label' => 'displayName', // (string) $tag->getDisplayName() 'choice_name' => 'name', // (string) $tag->getName() 'group_by' => 'category.name', // (string) $tag->getCategory()->getName() 'preferred_choices' => 'favorite', // (bool) $tag->isFavorite() 'choice_attr' => ? // (array) $tag->getHtmlAttributes() ?? );
That's very great for DX, whereas this might be irrelevant for
choice_attr
IMHO, unless using special classes for html form options (is there a use case ?). -
Using
choice_attr
as an array may lead to confusion (ref [Form] choice_attr option in ChoiceType generates options attributes wrong #17019), currently even the doc is wrong about how to use it (see here).
Indeed, when using an array, one has to pass nested arrays mapped by their key to the corresponding choice key:$builder->add('tags', ChoiceType::class, array( 'choices' => array( 'Choice 1' => 1, 'Choice 2' => 2, ), 'choice_attr' => array( 'Choice 1' => array('class' => 'class_choice_one'), 'Choice 2' => array('class' => 'class_choice_two'), ), );
Although:
- In 2.x versions if
choices_as_values
isfalse
one must rely on the numerical index instead. - Worse, when
choices_as_values
istrue
(even in 3.x), if a choice key is not a valid html "id" attribute (e.g 'Tom & Jerry'), keys fall back on an incrementing integer, so the attributes arrays are not mapped anymore. - It may be impossible to know the initial choice keys when choices are loaded.
Even worse (from a DX perspective), if one wants to use the same attributes for all the choices, he would expect to use a single array and that must be why the doc currently says in a total contradiction:
This can be an array of attributes (if they are the same for each choice)
Which is wrong, instead, it must be:
'choice_attr' => function () { return array('class' => 'choice_option'); }
That's exactly what I needed to do in a form with 5 choice fields needing the same
choice_attr
option as this example. I could not keep it in a variable as some fields need some extra classes.
(Btw, this is the same form I use in which I needchoice_label_attr
with this exact same behavior and "hacking" this option allowed me to divide by four the lengh of my template, by preventing me to loop over the fields, test variables etc.
One thing I've learned, the more accurate is yourFormType
the nearest you are to just writeform(form)
in your template without needing to override any block.
Hum, hum... "Bref":)- If one need a "static" array (same for all), it must be wrapped in a callable without using any argument.
- If one need it "dynamic", he needs to be sure about what keys to set and that they won't change in the future... Hence he'd better use a callable instead.
IMHO we should expect the same behavior as
entry_options
ofCollectionType
here, one array for all. - In 2.x versions if
Solution
- We can deprecate property path for
choice_attr
(point 2) - We can deprecate using nested arrays for
choice_attr
(point 3) - Then we could introduce a
choice_label_attr
with the same "new" behavior without breaking BC while keeping it all constistent (point 1) - Eventually move the logic of
choice_label_attr
resolution toDefaultChoiceListFactory
in 3.4 when we can change the interface for more consistency.
So we get:
choice_value => null, callable, string (or PropertyPath)
choice_name => null, callable, string (or PropertyPath)
choice_label => null, callable, string (or PropertyPath), bool (false to discard label)
preferred_choices => callable, string (or PropertyPath), array or \Traversable
group_by => null, callable, string (or PropertyPath),
choice_attr => array, callable
choice_label_attr => array, callable
I've totally refactored and updated #16834 to fix this issue accordingly.
(See 616fc8a for the previous implementation breaking BC).