8000 [2.7][Form][choice] added choice_translation_domain to avoid trans options. by aitboudad · Pull Request #13651 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[2.7][Form][choice] added choice_translation_domain to avoid trans options. #13651

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.

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

Merged
merged 1 commit into from
Apr 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
246 changes: 141 additions & 105 deletions UPGRADE-2.7.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ public function configureOptions(OptionsResolver $resolver)
'choice_name' => $choiceName,
'choice_value' => $choiceValue,
'id_reader' => null, // internal
'choice_translation_domain' => false,
));

$resolver->setRequired(array('class'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@
{%- block choice_widget_options -%}
{% for group_label, choice in options %}
{%- if choice is iterable -%}
<optgroup label="{{ group_label|trans({}, translation_domain) }}">
<optgroup label="{{ choice_translation_domain is sameas(false) ? group_label : group_label|trans({}, choice_translation_domain) }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

just for the record .. this change breaks things for users of earlier Form component versions. maybe we should therefore adjust the composer dependency when we do such changes in the future

{% set options = choice %}
{{- block('choice_widget_options') -}}
</optgroup>
{%- else -%}
{% set attr = choice.attr %}
<option value="{{ choice.value }}" {{ block('attributes') }}{% if choice is selectedchoice(value) %} selected="selected"{% endif %}>{{ choice.label|trans({}, translation_domain) }}</option>
<option value="{{ choice.value }}" {{ block('attributes') }}{% if choice is selectedchoice(value) %} selected="selected"{% endif %}>{{ choice_translation_domain is sameas(false) ? choice.label : choice.label|trans({}, choice_translation_domain) }}</option>
{%- endif -%}
{% endfor %}
{%- endblock choice_widget_options -%}
Expand Down
Original file line number Diff line number Diff line change
8000 Expand Up @@ -2,12 +2,12 @@

$translatorHelper = $view['translator']; // outside of the loop for performance reasons! ?>
<?php $formHelper = $view['form']; ?>
<?php foreach ($choices as $index => $choice): ?>
<?php foreach ($choices as $group_label => $choice): ?>
<?php if (is_array($choice) || $choice instanceof ChoiceGroupView): ?>
<optgroup label="<?php echo $view->escape($translatorHelper->trans($index, array(), $translation_domain)) ?>">
<optgroup label="<?php echo $view->escape(false !== $choice_translation_domain ? $translatorHelper->trans($group_label, array(), $choice_translation_domain) : $group_label) ?>">
<?php echo $formHelper->block($form, 'choice_widget_options', array('choices' => $choice)) ?>
</optgroup>
<?php else: ?>
<option value="<?php echo $view->escape($choice->value) ?>" <?php echo $view['form']->block($form, 'attributes', array('attr' => $choice->attr)) ?><?php if ($is_selected($choice->value, $value)): ?> selected="selected"<?php endif?>><?php echo $view->escape($translatorHelper->trans($choice->label, array(), $translation_domain)) ?></option>
<option value="<?php echo $view->escape($choice->value) ?>" <?php echo $view['form']->block($form, 'attributes', array('attr' => $choice->attr)) ?><?php if ($is_selected($choice->value, $value)): ?> selected="selected"<?php endif?>><?php echo $view->escape(false !== $choice_translation_domain ? $translatorHelper->trans($choice->label, array(), $choice_translation_domain) : $choice->label) ?></option>
<?php endif ?>
<?php endforeach ?>
3 changes: 2 additions & 1 deletion src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
2.7.0
-----

* added option "choice_translation_domain" to ChoiceType.
8000 * deprecated option "precision" in favor of "scale"
* deprecated the overwriting of AbstractType::setDefaultOptions() in favor of overwriting AbstractType::configureOptions().
* deprecated the overwriting of AbstractTypeExtension::setDefaultOptions() in favor of overwriting AbstractTypeExtension::configureOptions().
Expand All @@ -16,7 +17,7 @@ CHANGELOG
* deprecated ChoiceToBooleanArrayTransformer and ChoicesToBooleanArrayTransformer
* deprecated FixCheckboxInputListener and FixRadioInputListener
* deprecated the "choice_list" option of ChoiceType
* added new options to ChoiceType:
* added new options to ChoiceType:
* "choices_as_values"
* "choice_loader"
* "choice_label"
Expand Down
17 changes: 17 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ public function buildForm(FormBuilderInterface $builder, array $options)
*/
public function buildView(FormView $view, FormInterface $form, array $options)
{
$choiceTranslationDomain = $options['choice_translation_domain'];
if ($view->parent && null === $choiceTranslationDomain) {
$choiceTranslationDomain = $view->vars['translation_domain'];
}
Copy link
Member

Choose a reason for hiding this comment

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

We can use one if here:

if ($view->parent && null === $choiceTranslationDomain) {
    // ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


/** @var ChoiceListView $choiceListView */
$choiceListView = $form->getConfig()->hasAttribute('choice_list_view')
? $form->getConfig()->getAttribute('choice_list_view')
Expand All @@ -170,6 +175,7 @@ public function buildView(FormView $view, FormInterface $form, array $options)
'choices' => $choiceListView->choices,
'separator' => '-------------------',
'placeholder' => null,
'choice_translation_domain' => $choiceTranslationDomain,
));

// The decision, whether a choice is selected, is potentially done
Expand Down Expand Up @@ -295,6 +301,14 @@ public function configureOptions(OptionsResolver $resolver)
return $options['expanded'];
};

$choiceTranslationDomainNormalizer = function (Options $options, $choiceTranslationDomain) {
if (true === $choiceTranslationDomain) {
return $options['translation_domain'];
}

return $choiceTranslationDomain;
};

$resolver->setDefaults(array(
'multiple' => false,
'expanded' => false,
Expand All @@ -317,14 +331,17 @@ public function configureOptions(OptionsResolver $resolver)
// is manually set to an object.
// See https://github.com/symfony/symfony/pull/5582
'data_class' => null,
'choice_translation_domain' => true,
));

$resolver->setNormalizer('choice_list', $choiceListNormalizer);
$resolver->setNormalizer('empty_value', $placeholderNormalizer);
$resolver->setNormalizer('placeholder', $placeholderNormalizer);
$resolver->setNormalizer('choice_translation_domain', $choiceTranslationDomainNormalizer);

$resolver->setAllowedTypes('choice_list', array('null', 'Symfony\Component\Form\ChoiceList\ChoiceListInterface'));
$resolver->setAllowedTypes('choices', array('null', 'array', '\Traversable'));
$resolver->setAllowedTypes('choice_translation_domain', array('null', 'bool', 'string'));
Copy link
Contributor

Choose a reason for hiding this comment

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

this would also allow true.

Copy link
Contributor
< F438 div hidden="hidden" data-view-component="true" class="js-comment-show-on-error flash flash-error flash-full">

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

maybe the easiest would be to add a normalizer that return null when it is true so they mean the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

or you set an setAllowedValues with a closure that returns false in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the normalizer.

$resolver->setAllowedTypes('choices_as_values', 'bool');
$resolver->setAllowedTypes('choice_loader', array('null', 'Symfony\Component\Form\ChoiceList\Loader\ChoiceLoaderInterface'));
$resolver->setAllowedTypes('choice_label', array('null', 'callable', 'string', 'Symfony\Component\PropertyAccess\PropertyPath'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'choices' => Intl::getRegionBundle()->getCountryNames(),
'choice_translation_domain' => false,
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'choices' => Intl::getCurrencyBundle()->getCurrencyNames(),
'choice_translation_domain' => false,
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'choices' => Intl::getLanguageBundle()->getLanguageNames(),
'choice_translation_domain' => false,
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'choices' => Intl::getLocaleBundle()->getLocaleNames(),
'choice_translation_domain' => false,
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'choices' => self::getTimezones(),
'choice_translation_domain' => false,
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ public function testCountry()
'/select
[@name="name"]
[@class="my&class form-control"]
[./option[@value="AT"][@selected="selected"][.="[trans]Austria[/trans]"]]
[./option[@value="AT"][@selected="selected"][.="Austria"]]
[count(./option)>200]
'
);
Expand All @@ -858,7 +858,7 @@ public function testCountryWithPlaceholder()
[@name="name"]
[@class="my&class form-control"]
[./option[@value=""][not(@selected)][not(@disabled)][.="[trans]Select&Country[/trans]"]]
[./option[@value="AT"][@selected="selected"][.="[trans]Austria[/trans]"]]
[./option[@value="AT"][@selected="selected"][.="Austria"]]
[count(./option)>201]
'
);
Expand Down Expand Up @@ -1388,7 +1388,7 @@ public function testLanguage()
'/select
[@name="name"]
[@class="my&class form-control"]
[./option[@value="de"][@selected="selected"][.="[trans]German[/trans]"]]
[./option[@value="de"][@selected="selected"][.="German"]]
[count(./option)>200]
'
);
Expand All @@ -1402,7 +1402,7 @@ public function testLocale()
'/select
[@name="name"]
[@class="my&class form-control"]
[./option[@value="de_AT"][@selected="selected"][.="[trans]German (Austria)[/trans]"]]
[./option[@value="de_AT"][@selected="selected"][.="German (Austria)"]]
[count(./option)>200]
'
);
Expand Down Expand Up @@ -1826,8 +1826,8 @@ public function testTimezone()
[@class="my&class form-control"]
[not(@required)]
[./optgroup
[@label="[trans]Europe[/trans]"]
[./option[@value="Europe/Vienna"][@selected="selected"][.="[trans]Vienna[/trans]"]]
[@label="Europe"]
[./option[@value="Europe/Vienna"][@selected="selected"][.="Vienna"]]
]
[count(./optgroup)>10]
[count(.//option)>200]
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/Form/Tests/AbstractLayoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ public function testCountry()
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/select
[@name="name"]
[./option[@value="AT"][@selected="selected"][.="[trans]Austria[/trans]"]]
[./option[@value="AT"][@selected="selected"][.="Austria"]]
[count(./option)>200]
'
);
Expand All @@ -1033,7 +1033,7 @@ public function testCountryWithPlaceholder()
'/select
[@name="name"]
[./option[@value=""][not(@selected)][not(@disabled)][.="[trans]Select&Country[/trans]"]]
[./option[@value="AT"][@selected="selected"][.="[trans]Austria[/trans]"]]
[./option[@value="AT"][@selected="selected"][.="Austria"]]
[count(./option)>201]
'
);
Expand Down Expand Up @@ -1557,7 +1557,7 @@ public function testLanguage()
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/select
[@name="name"]
[./option[@value="de"][@selected="selected"][.="[trans]German[/trans]"]]
[./option[@value="de"][@selected="selected"][.="German"]]
[count(./option)>200]
'
);
Expand All @@ -1570,7 +1570,7 @@ public function testLocale()
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/select
[@name="name"]
[./option[@value="de_AT"][@selected="selected"][.="[trans]German (Austria)[/trans]"]]
[./option[@value="de_AT"][@selected="selected"][.="German (Austria)"]]
[count(./option)>200]
'
);
Expand Down Expand Up @@ -1936,8 +1936,8 @@ public function testTimezone()
[@name="name"]
[not(@required)]
[./optgroup
[@label="[trans]Europe[/trans]"]
[./option[@value="Europe/Vienna"][@selected="selected"][.="[trans]Vienna[/trans]"]]
[@label="Europe"]
[./option[@value="Europe/Vienna"][@selected="selected"][.="Vienna"]]
]
[count(./optgroup)>10]
[count(.//option)>200]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,51 @@ public function testPassExpandedToView()
$this->assertTrue($view->vars['expanded']);
}

public function testPassChoiceTranslationDomainToView()
{
$form = $this->factory->create('choice', null, array(
'choices' => $this->choices,
));
$view = $form->createView();

$this->assertNull($view->vars['choice_translation_domain']);
}

public function testChoiceTranslationDomainWithTrueValueToView()
{
$form = $this->factory->create('choice', null, array(
'choices' => $this->choices,
'choice_translation_domain' => true,
));
$view = $form->createView();

$this->assertNull($view->vars['choice_translation_domain']);
}

public function testDefaulChoiceTranslationDomainIsSameAsTranslationDomainToView()
{
$form = $this->factory->create('choice', null, array(
'choices' => $this->choices,
'translation_domain' => 'foo',
));
$view = $form->createView();

$this->assertEquals('foo', $view->vars['choice_translation_domain']);
}

public function testInheritChoiceTranslationDomainFromParent()
{
$view = $this->factory
->createNamedBuilder('parent', 'form', null, array(
'translation_domain' => 'domain',
))
->add('child', 'choice')
->getForm()
->createView();

$this->assertEquals('domain', $view['child']->vars['choice_translation_domain']);
}

public function testPlaceholderIsNullByDefaultIfRequired()
{
$form = $this->factory->create('choice', null, array(
Expand Down
0