sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Form "help" is not translated using the (default) domain of the admin. #7014

Closed gremo closed 3 years ago

gremo commented 3 years ago

Using version 3.79

Bug introduced with this commit (I suppose): https://github.com/sonata-project/SonataAdminBundle/commit/77326c4427895be3a724a876a61ea660990344a5#diff-35d3133a895a9ec1948258059dc0265ac33c1959eca60d39b3a61726dbeb6f66, somewhere after 3.65 (I can see "old" admin not showing this problem).

The block looks like this:

% block form_help -%}
    {% if help is not empty %}
        {# NEXT_MAJOR: Remove class sonata-ba-field-widget-help #}
        {% set help_attr = help_attr|merge({class: (help_attr.class|default('') ~ ' help-block sonata-ba-field-widget-help sonata-ba-field-help')}) %}
        {{ parent() }}
    {% endif %}
{%- endblock form_help %}

Which calls the symfony block:

{% block form_help -%}
    {%- if help is not empty -%}
        <p id="{{ id }}_help" class="help-text">
            {%- if translation_domain is same as(false) -%}
                {{- help -}}
            {%- else -%}
                {{- help|trans({}, translation_domain) -}}
            {%- endif -%}
        </p>
    {%- endif -%}
{%- endblock form_help %}

... where translation_domain is null, fallback to "messages" (while all my admin have "admin" as translation domain).

The right thing to do is the same as for form_label, that is using the admin domain.

Now I'm forced to do:

$form->add('name', null', [
  'help' => $this->trans('help.label_name'),
]);

or even worst:

$form->add('name', null', [
  'help' =>'help.label_name',
  'translation_domain' => $this->getTranslationDomain(),
]);

For each field of every admin I have.

franmomu commented 3 years ago

We could maybe set the translation_domain in https://github.com/sonata-project/SonataAdminBundle/blob/db09b03adee7ae24379d0462af329467b4a83e29/src/Form/Extension/Field/Type/FormTypeFieldExtension.php#L95

And there check if the translation_domain option is false and there is admin, set it to the admin translation domain and there is no need to override the blocks in twig.

PS: Thanks for reporting it!

franmomu commented 3 years ago

Can you please try to add:

if (false === $options['translation_domain']) {
    $view->vars['translation_domain'] = $sonataAdmin['admin']->getTranslationDomain();
}

after https://github.com/sonata-project/SonataAdminBundle/blob/db09b03adee7ae24379d0462af329467b4a83e29/src/Form/Extension/Field/Type/FormTypeFieldExtension.php#L155-L156

Just to check, if that's ok I'll try to make a PR, if we could avoid overriding these blocks will be great.

EDIT: for the future me or if someone is willing to do it, instead of overriding translation_domain in FormTypeFieldExtension.php, we could use a new sonata_admin_translation_domain view var and in twig override translation_domain when is needed.

gremo commented 3 years ago

@franmomu translation_domain seems null not false when not passing it.

That is, your code is not working (line 157 of SonataAdminBundle/src/Form/Extension/Field/Type/FormTypeFieldExtension.php)

if (false === $options['translation_domain']) {
    $view->vars['translation_domain'] = $sonataAdmin['admin']->getTranslationDomain();
}

On the contrary, this is working (but don't know the side effects):

if (!isset($options['translation_domain'])) {
    $view->vars['translation_domain'] = $sonataAdmin['admin']->getTranslationDomain();
}
franmomu commented 3 years ago

@franmomu translation_domain seems null not false when not passing it.

That is, your code is not working (line 157 of SonataAdminBundle/src/Form/Extension/Field/Type/FormTypeFieldExtension.php)

if (false === $options['translation_domain']) {
    $view->vars['translation_domain'] = $sonataAdmin['admin']->getTranslationDomain();
}

On the contrary, this is working (but don't know the side effects):

if (!isset($options['translation_domain'])) {
    $view->vars['translation_domain'] = $sonataAdmin['admin']->getTranslationDomain();
}

Oops, yes, I thought null and I wrote false, false is when the user explicitly says don't want label and null if not specified.

I added some tests in https://github.com/sonata-project/SonataAdminBundle/pull/7020, after that I'll create a sonata_admin_translation_domain and only use it in form_help and if that's fine I think we could remove some code of form_label in 4.x to be safe.

gremo commented 3 years ago

Thank you! You mean this will not be fixed in 3.x (that's the version I use)?

franmomu commented 3 years ago

Thank you! You mean this will not be fixed in 3.x (that's the version I use)?

No, no. I'll make the PR for 3.x.

The comment about 4.x was that I think we can remove some code using this new sonata_admin_translation_domain (in form_label) and I'd rather do it in 4.x to be safe.