[Form] label path in ObjectChoiceList can also be callable #3479
Conversation
|
I don't see any reason to add this. there is already a |
|
IMHO this doesn't belong to the entity class |
|
why not ? it is about getting some display name. Adding a getter in the entity makes this reusable |
|
i could imagine the case where the label needs to be rendered using a service, e.g.: $builder->add('product', 'entity', array(
// ... //
'label_builder' => function($entity) use ($priceManager) {
return sprintf('%s (%.2f)', $entity->getName(), $priceManager->calculateDiscountPrice($entity));
})
));This is definitely not something that the entity should handle. |
|
Just asked myself the same question today... I would like to display a dropdown list of hierarchical entity data, something like Europe
To do that, I could add a method getChoiceText() in my entity : public function getChoiceText() {
return ltrim(str_repeat('-', $this->level) . ' ' . $this->name);
}It works, but it feels a bit odd to add purely "form/presentation" logic in my entity. This method will only be used in my form. The callback idea makes sense, although there's no doubt we can live without. Wouldn't it be possible to use the 'property' option to achieve what @mvrhov is suggesting ? This option could accept either a string, to be used by PropertyPath, or a closure that would be provided with an entity ? |
|
Would be nice to have something like this. Enhancing the @stof: I'd need to call the method of a service to build each label, which is not possible currently. |
|
+1 |
|
+1, it's not going to be used a lot, but still a useful feature |
|
Nice. I'd love to have this. |
| @@ -64,6 +64,9 @@ class EntityChoiceList extends ObjectChoiceList | ||
| */ | ||
| private $loaded = false; | ||
|
|
||
|
|
|
This should probably be implemented in the ObjectChoiceList instead of the EntityChoiceList |
|
Added test, moved the implementation to base class (ObjectChoiceList) as suggested by @stof. and extended $labelPath parameter as suggested by @pvanliefland. I'd really like to see some comments about changed phpdoc from someone more fluent in English. |
| $labels[$i] = $this->labelPath->getValue($choice); | ||
| } elseif (is_callable($this->labelPath)) { | ||
| $labels[$i] = $callable($choice); |
There was a problem hiding this comment.
this is wrong. It won't work for callable like array($object, $method) on PHP 5.3
|
@bschussek is it ok for you? |
| * is used instead. | ||
| * by calling the getter on the object. You can | ||
| * also pass any callable. When callable will be | ||
| * executed it will get the choice parameter passed. |
There was a problem hiding this comment.
I would say :
Alternatively, $labelPath can be a PHP callable
taking an element from the ObjectChoiceList as
parameter and returning a string.
But then $labelPath should be documented as a mixed parameter ?
|
@fabpot I'm not yet satisfied with this PR. I think this problem should be solved on a more general scope than just ObjectChoiceList, but I need to give this some more thought. |
|
Closing this PR in favor of #4067 |
|
ping @bschussek |
|
@bschussek ok, thanks! |
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
This PR adds a callback support as additional option to label path in ObjectChoiceList class.
Right now you either have to set a property path to a property or a getter or implement __toString method in the objects you pass as choices to get the choice label. Usually this is ok, but sometimes you need to have different labels depending on where the choice filed is displayed, or __toString is used elsewhere for different purposes.
Usage example: