Pass menu label attributes to translation

[pavol-tuka]

Subject

Allow to use parameters/placeholders in (sidebar) menu label.

translations/messages.en.yaml

item_label_translation: Translatable label with %placeholder%.

Menu builder:

$group->addChild('item', [
    'label' => 'item_label_translation',
    'labelAttributes' => ['%placeholder%' => 'dynamic_value']
]);

[phansys]

This change should target 3.x, since there is no BC break in this addition.

[VincentLanglet]

IMHO If I understand correctly the option, the best name is label_translation_parameters.

This would fit with symfony option: https://symfony.com/doc/current/reference/forms/types/choice.html#label-translation-parameters

Edit: Seems like this was an already documented option, https://symfony.com/doc/master/bundles/SonataAdminBundle/reference/advanced_configuration.html#translations

But when I looked in the code, there is no use of translation_parameters. I would prefer to rename the option and update the documentation.

[phansys]

But when I looked in the code, there is no use of translation_parameters. I would prefer to rename the option and update the documentation.

I agree. We should deprecate the existing option in order to be consistent with "label_translation_parameters".

[VincentLanglet]

This is not fully related, but, seems like the translation_domain is defined this way in the block linkElement and the block spanElement:

{% set translation_domain = item.extra('label_catalogue', 'messages') %}

First of all, the option used should be translation_domain and not label_catalogue.

Secondly, seems like the variable translation_domain is not defined in the block label, so it always default to messages.

I would recommend to use:

{# NEXT_MAJOR: Remove the deprecated option 'label_catalogue' #}
{% set translation_domain = item.extra('label_catalogue', item.extra('translation_domain', 'messages')) %}

line 25, 39, and adding this variable inside the block label.

[VincentLanglet]

Does someone use the label_catalogue option (or translation_domain one ?). Which one works ATM ? Do you know @pavol-tk ?

When I look at the doc, it say:

The translation parameters and domain can be customised by using the
``translation_domain`` and ``translation_parameters`` keys of the extra array
of data associated with the item, respectively::

    $menuItem->setExtras([
        'translation_parameters' => ['myparam' => 'myvalue'],
        'translation_domain' => 'My domain',
    ]);

You can also set the translation domain on the menu root, and children will
inherit it::

    $menu->setExtra('translation_domain', 'My domain');

But when I look at the code, all I see is label_catalogue and default_label_catalogue options. For a total of 19 occurences.

If the actual working options are label_catalogue and default_label_catalogue, we should:

• update the documentation to describe the actual working option.
• create an issue to rename these options to translation_domain and default_translation_domain.

This will avoid to start changing 19 occurences, adding a deprecation note, etc, in a non-related small PR which was created to add a missing option.

[pavol-tuka]

I have installed sonata-project/admin-bundle 3.66.0 and try this:

addChild($year, [
                'label' => 'year',
                'default_label_catalogue' => 'OPTION_default_label_catalogue',
                'label_catalogue' => 'OPTION_label_catalogue',
            ])->setExtras([
                'translation_domain' => 'EXTRA_translation_domain'
            ])

and I see (in translation panel of profiler) that its completly ignored. It uses domain messages (as you said - there is no tranlation_domain so it always fallback to messages) and also no translation parameres are passed. You are right, documented options are unused. IMHO we should:

• add {% set translation_domain = item.extra('label_catalogue') %} to label block
• use item.extra('translation_parameters') in label block
• fix documentation
• create another issue to rename label_catalogue to translation_domain and default_label_catalogue to default_translation_domain as @VincentLanglet mentioned.

If you agree, I can do it :)

[VincentLanglet]

'default_label_catalogue' => 'OPTION_default_label_catalogue',

I think the default_label_catalogue is set in the yaml.

• add {% set translation_domain = item.extra('label_catalogue') %} to label block

I agree, {% set translation_domain = item.extra('label_catalogue', 'messages') %} is needed.

• use item.extra('translation_parameters') in label block

You mean label_translation_parameters right ?

• fix documentation

Yes

• create another issue to rename label_catalogue to translation_domain and default_label_catalogue to default_translation_domain as @VincentLanglet mentioned.

Yes

If you agree, I can do it :)

Great :)

[VincentLanglet]

I'm not sure of the impact of adding - in all the tag of block root, block item, etc.

Maybe it's better to limit the change to block label in order to avoid regression.
But I let the choice to other reviewers.

[VincentLanglet]

@pavol-tk Can you make this PR on 3.x instead of master ?

A way to do this is

git checkout 3.x
git checkout -b newBranch
git cherry-pick cec3866
git cherry-pick ...

With every commit hash of your PR.

[pavol-tuka]

@pavol-tk Can you make this PR on 3.x instead of master ?

@VincentLanglet thank you for your steps to do it. #6112