8000 [Form] Added locale option for intl form types. by Koc · Pull Request #23629 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Added locale option for intl form types. #23629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[Form] Added locale option for intl form types. #23629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d0476b
Added locale option for intl form types.
Koc Jul 22, 2017
3a67076
tmp
Koc Jul 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/CountryType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class CountryType extends AbstractType implements ChoiceLoaderInterface
*/
private $choiceList;

private $locale;

/**
* {@inheritdoc}
*/
Expand All @@ -44,9 +46,12 @@ public function configureOptions(OptionsResolver $resolver)
return null;
}

$this->locale = $options['locale'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the use case looks weird, this may be overridden for each use of the type in the same request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does form types are shared services?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they are shared services re-used to build each of your form instances. So usage of class properties should be prohibited in most cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably deprecate the ChoiceLoaderInterface implementation in those types as done in #23648.

Copy link
Contributor Author
@Koc Koc Jul 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. It confused me before: it was so hard to find place where should I save locale from options 🤣

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's either this, either decorating the CountryType's ChoiceLoaderInterface implementation when using the locale option, so we keep a cache of base choice list (and will always be used as before when not using the option). But this cache is not worth it to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Koc: Do you have some time to finish this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, will try do it during this last week before stabilization phase.


return $this;
},
'choice_translation_domain' => false,
'locale' => \Locale::getDefault(),
));
}

Expand Down Expand Up @@ -75,7 +80,7 @@ public function loadChoiceList($value = null)
return $this->choiceList;
}

return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getRegionBundle()->getCountryNames()), $value);
return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getRegionBundle()->getCountryNames($this->locale)), $value);
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/CurrencyType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class CurrencyType extends AbstractType implements ChoiceLoaderInterface
*/
private $choiceList;

private $locale;

/**
* {@inheritdoc}
*/
Expand All @@ -44,9 +46,12 @@ public function configureOptions(OptionsResolver $resolver)
return null;
}

$this->locale = $options['locale'];

return $this;
},
'choice_translation_domain' => false,
'locale' => \Locale::getDefault(),
));
}

Expand Down Expand Up @@ -75,7 +80,7 @@ public function loadChoiceList($value = null)
return $this->choiceList;
}

return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getCurrencyBundle()->getCurrencyNames()), $value);
return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getCurrencyBundle()->getCurrencyNames($this->locale)), $value);
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/LanguageType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class LanguageType extends AbstractType implements ChoiceLoaderInterface
*/
private $choiceList;

private $locale;

/**
* {@inheritdoc}
*/
Expand All @@ -44,9 +46,12 @@ public function configureOptions(OptionsResolver $resolver)
return null;
}

$this->locale = $options['locale'];

return $this;
},
'choice_translation_domain' => false,
'locale' => \Locale::getDefault(),
));
}

Expand Down Expand Up @@ -75,7 +80,7 @@ public function loadChoiceList($value = null)
return $this->choiceList;
}

return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getLanguageBundle()->getLanguageNames()), $value);
return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getLanguageBundle()->getLanguageNames($this->locale)), $value);
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/LocaleType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class LocaleType extends AbstractType implements ChoiceLoaderInterface
*/
private $choiceList;

private $locale;

/**
* {@inheritdoc}
*/
Expand All @@ -44,9 +46,12 @@ public function configureOptions(OptionsResolver $resolver)
return null;
}

$this->locale = $options['locale'];

return $this;
},
'choice_translation_domain' => false,
'locale' => \Locale::getDefault(),
));
}

Expand Down Expand Up @@ -75,7 +80,7 @@ public function loadChoiceList($value = null)
return $this->choiceList;
}

return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getLocaleBundle()->getLocaleNames()), $value);
return $this->choiceList = new ArrayChoiceList(array_flip(Intl::getLocaleBundle()->getLocaleNames($this->locale)), $value);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/CountryTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,17 @@ public function testSubmitNull($expected = null, $norm = null, $view = null)
{
parent::testSubmitNull($expected, $norm, '');
}

public function testCustomLocale()
{
$choices = $this->factory->create(static::TESTED_TYPE, null, array('locale' => 'ru'))
->createView()->vars['choices'];

// Don't check objects for identity
$this->assertContains(new ChoiceView('DE', 'DE', 'Германия'), $choices, '', false, false);
$this->assertContains(new ChoiceView('GB', 'GB', 'Великобритания'), $choices, '', false, false);
$this->assertContains(new ChoiceView('US', 'US', 'Соединенные Штаты'), $choices, '', false, false);
$this->assertContains(new ChoiceView('FR', 'FR', 'Франция'), $choices, '', false, false);
$this->assertContains(new ChoiceView('MY', 'MY', 'Малайзия'), $choices, '', false, false);
}
}
10 changes: 10 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/CurrencyTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ public function testCurrenciesAreSelectable()
$this->assertContains(new ChoiceView('SIT', 'SIT', 'Slovenian Tolar'), $choices, '', false, false);
}

public function testCustomLocale()
{
$choices = $this->factory->create(static::TESTED_TYPE, null, array('locale' => 'ru'))
->createView()->vars['choices'];

$this->assertContains(new ChoiceView('EUR', 'EUR', 'Евро'), $choices, '', false, false);
$this->assertContains(new ChoiceView('USD', 'USD', 'Доллар США'), $choices, '', false, false);
$this->assertContains(new ChoiceView('SIT', 'SIT', 'Словенский толар'), $choices, '', false, false);
}

public function testSubmitNull($expected = null, $norm = null, $view = null)
{
parent::testSubmitNull($expected, $norm, '');
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/LanguageTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ public function testCountriesAreSelectable()
$this->assertContains(new ChoiceView('my', 'my', 'Burmese'), $choices, '', false, false);
}

public function testCustomLocale()
{
$choices = $this->factory->create(static::TESTED_TYPE, null, array('locale' => 'ru'))
->createView()->vars['choices'];

$this->assertContains(new ChoiceView('en', 'en', 'Английский'), $choices, '', false, false);
$this->assertContains(new ChoiceView('en_GB', 'en_GB', 'Британский английский'), $choices, '', false, false);
$this->assertContains(new ChoiceView('en_US', 'en_US', 'Американский английский'), $choices, '', false, false);
$this->assertContains(new ChoiceView('fr', 'fr', 'Французский'), $choices, '', false, false);
$this->assertContains(new ChoiceView('my', 'my', 'Бирманский'), $choices, '', false, false);
}

public function testMultipleLanguagesIsNotIncluded()
{
$choices = $this->factory->create(static::TESTED_TYPE, 'language')
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/LocaleTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ public function testLocalesAreSelectable()
$this->assertContains(new ChoiceView('zh_Hant_MO', 'zh_Hant_MO', 'Chinese (Traditional, Macau SAR China)'), $choices, '', false, false);
}

public function testCustomLocale()
{
$choices = $this->factory->create(static::TESTED_TYPE, null, array('locale' => 'ru'))
->createView()->vars['choices'];

$this->assertContains(new ChoiceView('en', 'en', 'English'), $choices, '', false, false);
$this->assertContains(new ChoiceView('en_GB', 'en_GB', 'English (United Kingdom)'), $choices, '', false, false);
$this->assertContains(new ChoiceView('zh_Hant_MO', 'zh_Hant_MO', 'Chinese (Traditional, Macau SAR China)'), $choices, '', false, false);
}

public function testSubmitNull($expected = null, $norm = null, $view = null)
{
parent::testSubmitNull($expected, $norm, '');
Expand Down
0