-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Description
Q | A |
---|---|
Bug report? | no |
Feature request? | yes |
BC Break report? | no |
RFC? | yes |
Symfony version | 4.1 (I hope) |
Note: "all Intl choice types" === [CountryType, LocaleType, CurrencyType, LanguageType]
Request
The use case that I've is to show the list of countries in a locale different to the current one (provided by \Locale::getDefault()
).
For example:
$formBuilder->add('country', CountryType::class, [
'locale' => 'uk', // doing internally: Intl::getRegionBundle()->getCountryNames($locale)
]);
In fact, I need several country fields, each one with different locales.
Sub-request
Since the four choice types share almost the same code, it may be convenient (as sub-proposal) to decouple the implementation of the ChoiceLoaderInterface
removing most of the repeated code and simplify the addition of the new option.
As first step, I'm proposing to separate the responsibilities by decoupling the choice loader code. Something like this:
namespace Symfony\Component\Form\ChoiceList\Loader;
class IntlChoiceLoader extends CallbackChoiceLoader
{
public function loadChoicesForValues(array $values, $value = null)
{
// Optimize
$values = array_filter($values);
if (empty($values)) {
return array();
}
// If no callable is set, values are the same as choices
if (null === $value) {
return $values;
}
return $this->loadChoiceList($value)->getChoicesForValues($values);
}
public function loadValuesForChoices(array $choices, $value = null)
{
// Optimize
$choices = array_filter($choices);
if (empty($choices)) {
return array();
}
// If no callable is set, choices are the same as values
if (null === $value) {
return $choices;
}
return $this->loadChoiceList($value)->getValuesForChoices($choices);
}
}
Keeping improvements spotted in #18332 and fixes like e43bfaf. That's why I'm not proposing CallbackChoiceLoader
directly.
Then, we would be ready to add the new locale
option without much effort and the form type will be clean:
This way CountryType
will look in 5.0
(like we've already TimezoneType
):
class CountryType extends AbstractType
{
public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'choice_loader' => function (Options $options) {
$locale = $options['locale'];
return new IntlChoiceLoader(function () use ($locale) {
return array_flip(Intl::getRegionBundle()->getCountryNames($locale));
});
},
'choice_translation_domain' => false,
'locale' => null,
));
$resolver->setAllowedTypes('locale', ['null', 'string']);
}
public function getParent()
{
return __NAMESPACE__.'\ChoiceType';
}
public function getBlockPrefix()
{
return 'country';
}
}
Note: This sub-request requires deprecate the ChoiceLoaderInterface
implementation + property like we did in 183307b for TimezoneType
.
What do you think? Is it worth adding this new option and refactoring the code?