Added support for help_translation_parameters and label_translation_parameters #6206

tkuska · 2020-07-16T20:02:09Z

Subject

In Symfony 4.3 form translation was improved, there was added parameters like 'label_translation_parameters' and 'help_translation_parameters' but these parameters was missing in SonataAdmin templates

Changelog

### Added
- Support for 'help_translation_parameters' in form types
- Support for 'label_translation_parameters' in form types

…r last module

Co-Authored-By: tkuska <[email protected]>

…o 3.x

…anslation_parameters

jordisala1991 · 2020-07-17T06:31:09Z

Build is failing, can you rebase?

VincentLanglet

I had trouble to understand this file... 🤔

Why label_translation_parameters and help_translation_parameters would be accessible like this, when we're doing sonata_admin.field_description.translationDomain or sonata_admin.field_description.help for others params ?

Shouldn't we use something like sonata_admin.field_description.option.foo ?

franmomu · 2020-07-17T07:05:11Z

Why label_translation_parameters and help_translation_parameters would be accessible like this, when we're doing sonata_admin.field_description.translationDomain or sonata_admin.field_description.help for others params ?

https://symfony.com/blog/new-in-symfony-4-3-improved-form-translation, you can define them in form types.

VincentLanglet · 2020-07-17T07:12:25Z

Why label_translation_parameters and help_translation_parameters would be accessible like this, when we're doing sonata_admin.field_description.translationDomain or sonata_admin.field_description.help for others params ?

https://symfony.com/blog/new-in-symfony-4-3-improved-form-translation, you can define them in form types.

I agree with a classic formType. But here, we're working with FieldDescription too.

Here

<span class="help-block sonata-ba-field-help">{{ sonata_admin.field_description.help|trans({}, sonata_admin.field_description.translationDomain ?: admin.translationDomain)|raw }}</span>

You can see that we're not just using help, but sonata_admin.field_description.help. Same way we're using sonata_admin.field_description.translationDomain.

There is also

{% if sonata_admin.field_description.options is defined %}
    {% set label = sonata_admin.field_description.options.name|default(label) %}
{% endif %}

So if I want to get foo, how can I know if I need to write

foo
sonata_admin.field_description.foo
sonata_admin.field_description.options.foo
?

…r last module

Co-Authored-By: tkuska <[email protected]>

…anslation_parameters

…o 3.x

Added access to help_tranlslation_parameters and label_translation_parameters to fieldDescription

tkuska · 2020-07-17T12:53:40Z

Ok, rebase done, and i've added label_translation_parameter and help_translation_parameters as options to fieldDescription, now hey are accessible like:

help_translation_parameters (standard symfony templates approach)
sonata_admin.field_description.helpTranslationParameters
sonata_admin.field_description.options.help_translation_parameters

Is that OK?

tkuska · 2020-07-17T12:59:15Z

ooops i messed sth up during rebase.
changes in src/Resources/views/Core/add_block.html.twig file should not be done there.

VincentLanglet · 2020-07-17T13:05:05Z

src/Admin/BaseFieldDescription.php

+    public function getHelpTranslationParameters()
+    {
+        return $this->getOption('help_translation_parameters');
+    }
+
+    public function getLabelTranslationParameters()
+    {
+        return $this->getOption('label_translation_parameters');
+    }


I'm not sure we want to have quick getter for every option.

Shouldn't we remove them and only use
options. label_translation_parameters to access this kind of property ?

ok, just misunderstood your comment, i thought you want it accessible via getter

So if I want to get foo, how can I know if I need to write

foo

sonata_admin.field_description.foo

sonata_admin.field_description.options.foo
?

I never correctly understood the way this file works.

What I wanted to say is that I don't know why sometimes we're using foo and sometimes we're using sonata_admin.field_description.options.foo. Which one should we use to this work properly ? And how to know this ?

We're indeed also using sonata_admin.field_description.foo sometimes but I feel like it's not a good idea to always add getter, because we need to add them to the interface (and it's a BC-break). Working with $options is easier.

In a PR I prepare for 5.0 (so in a long long time), I will try to remove some sonata_admin.field_description.foo getter.
sonata-project/SonataDatagridBundle#239

VincentLanglet · 2020-07-17T13:06:47Z

src/Form/FormMapper.php

+            if (isset($options['label_translation_parameters'])) {
+                $fieldDescription->setOption('label_translation_parameters', $options['label_translation_parameters']);
+                unset($options['help']);
+            }


I don't think we need this, all options are set by default to the $fieldDescription.

There is special behaviour for label, template etc because the data is saved in a $label, $template property instead of the $options property (don't really know why btw...).

BTW it currently introduce a bug since you're unsetting $options['help'].

VincentLanglet · 2020-07-17T13:06:54Z

src/Form/FormMapper.php

+            if (isset($options['help_translation_parameters'])) {
+                $fieldDescription->setOption('help_translation_parameters', $options['help_translation_parameters']);
+                unset($options['help']);
+            }


VincentLanglet · 2020-07-17T13:10:41Z

sonata_admin.field_description.helpTranslationParameters

sonata_admin.field_description.options.help_translation_parameters

IMHO we should only promote sonata_admin.field_description.options.help_translation_parameters.

help_translation_parameters (standard symfony templates approach)

sonata_admin.field_description.options.help_translation_parameters

How to know which one we should use and when ? Is it always the same value ?

@sonata-project/contributors @tkuska Does someone understand something about this file ? 😅

src/Form/FormMapper.php

franmomu · 2020-07-17T14:13:43Z

Apparently, help in FileDescription was added long time ago I would say to do the job that now the help attribute does in the form type. If so, I think we could deprecate the use of help in FileDescription in favor of the form type and clean up a little bit.

VincentLanglet · 2020-07-17T14:49:42Z

Apparently, help in FileDescription was added long time ago I would say to do the job that now the help attribute does in the form type. If so, I think we could deprecate the use of help in FileDescription in favor of the form type and clean up a little bit.

If I understand correctly:

We should promote foo for formOptions (even if sonata_admin.field_description.options.foo has the same value).
Sonata-custom options are only accessible with sonata_admin.field_description.options.foo (or sonata_admin.field_description.foo if a getter is created).

This would mean that your PR was originally better without my request @tkuska, sorry. 😔

Edit: And yeah ! We should deprecate help in FileDescription.

tkuska · 2020-07-17T15:24:55Z

OK, i will roll back to use standard label_translation_parameters and help_translation_parameters instead of fieldDescription options this evening.

Should i do something with help option too?

i heve found something like this in FieldDescriptionInterface

//    NEXT_MAJOR: uncomment this method in 4.0
//    /**
//     * Defines the help message.
//     */
//    public function setHelp(string $help): void;

It looks like 'help' option is prepared to be part of field description in 4.0?

…tion_params for checkboxed and radios too (aswell as it works with standard Symfony forms)

VincentLanglet · 2020-07-17T20:36:04Z

OK, i will roll back to use standard label_translation_parameters and help_translation_parameters instead of fieldDescription options this evening.

Thanks !

Should i do something with help option too?

If you want to, it could be great. But you should do it in another PR.

We have to

Replace sonata_admin.field_description.help or sonata_admin.field_description.options.help by help (or add a TODO for next major ?)
Deprecate the BaseFieldDescription::getHelp and BaseFieldDescription::setHelp methods
Deprecate the FormMapper::setHelps and FormMapper::addHelp methods
Update the base_standard_edit_field.html.twig
Same for edit_one_to_many_inline_table.html.twig
Same for form_admin_field.html.twig

I create an issue to not forget about this #6207
If you want to try a PR it will be great !

i heve found something like this in FieldDescriptionInterface
//    NEXT_MAJOR: uncomment this method in 4.0
//    /**
//     * Defines the help message.
//     */
//    public function setHelp(string $help): void;
It looks like 'help' option is prepared to be part of field description in 4.0?

We added this because we were already calling setHelp on FieldDescriptionInterface, the interface wasn't up to date.
But it's better to remove this and stop using setHelp.

VincentLanglet · 2020-07-17T20:42:44Z

By the way, see #5820 (comment)

You could add it too.

tkuska · 2020-07-18T12:17:43Z

OK, i will create new PR for translation parameters in filter fields and another one with help option

VincentLanglet · 2020-07-19T11:58:50Z

OK, i will create new PR for translation parameters in filter fields and another one with help option

Thanks.

I approved this PR. As soon as another @sonata-project/contributors approve it too, we'll merge it.

phansys · 2020-07-19T15:45:56Z

I think this patch deserves some examples at /docs/reference/form_types.rst.

VincentLanglet · 2020-07-19T20:22:32Z

I think this patch deserves some examples at /docs/reference/form_types.rst.

Wouldn't it better to try to support every Symfony options and refer to the Symfony documentation instead ?
I feel like this doc is a outdated duplicate of Symfony form options.

franmomu · 2020-07-21T12:53:36Z

@tkuska have you started working on deprecating the help option? I could give it a try if you want to.

tkuska · 2020-07-21T13:28:01Z

@franmomu no, i fighting with passing label_translation_parameters in filter fields right now so you can take this help option.

VincentLanglet · 2020-07-22T20:23:29Z

Thanks @tkuska !

tkuska and others added 6 commits October 1, 2018 10:28

fixed display top nav menu when user does not have rights for first o…

e617172

…r last module

Update src/Resources/views/Core/add_block.html.twig

990d0df

Co-Authored-By: tkuska <[email protected]>

Update src/Resources/views/Core/add_block.html.twig

3e42049

Co-Authored-By: tkuska <[email protected]>

Merge branch '3.x' of github.com:sonata-project/SonataAdminBundle int…

b8d057e

…o 3.x

Added support for help_translation_parameter in FormMapper

966080d

refactored support for help_translation_parameters and added label_tr…

17e23de

…anslation_parameters

jordisala1991 added the patch label Jul 16, 2020

core23 previously approved these changes Jul 17, 2020

View reviewed changes

jordisala1991 closed this Jul 17, 2020

jordisala1991 reopened this Jul 17, 2020

VincentLanglet requested changes Jul 17, 2020

View reviewed changes

VincentLanglet requested a review from a team July 17, 2020 06:48

tkuska and others added 6 commits July 17, 2020 13:48

fixed display top nav menu when user does not have rights for first o…

ecea79b

…r last module

Update src/Resources/views/Core/add_block.html.twig

2557974

Co-Authored-By: tkuska <[email protected]>

Update src/Resources/views/Core/add_block.html.twig

73bce6c

Co-Authored-By: tkuska <[email protected]>

Added support for help_translation_parameter in FormMapper

7a78cb7

refactored support for help_translation_parameters and added label_tr…

71f0d66

…anslation_parameters

Merge branch '3.x' of https://github.com/tkuska/SonataAdminBundle int…

c795cbc

…o 3.x

tkuska dismissed core23’s stale review via c795cbc July 17, 2020 12:28

Restored changes after rebase...

62db01c

Added access to help_tranlslation_parameters and label_translation_parameters to fieldDescription

Fixed mess i've done during rebase

97bd742

VincentLanglet reviewed Jul 17, 2020

View reviewed changes

fixed some "unset" issues and removed getters in FieldDescription

d063a18

VincentLanglet reviewed Jul 17, 2020

View reviewed changes

src/Form/FormMapper.php Outdated Show resolved Hide resolved

reterted changes to standard symfony approach and added label_transla…

4269acc

…tion_params for checkboxed and radios too (aswell as it works with standard Symfony forms)

VincentLanglet mentioned this pull request Jul 17, 2020

Deprecate BaseFieldDescription::setHelp and BaseFieldDescription::getHelp #6207

Closed

VincentLanglet approved these changes Jul 17, 2020

View reviewed changes

VincentLanglet requested a review from a team July 17, 2020 20:38

VincentLanglet mentioned this pull request Jul 19, 2020

feat(form-theme): add label_translation_parameters support #5820

Closed

phansys approved these changes Jul 19, 2020

View reviewed changes

franmomu mentioned this pull request Jul 21, 2020

Deprecate help option in FieldDescription #6215

Merged

VincentLanglet merged commit ba76343 into sonata-project:3.x Jul 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for help_translation_parameters and label_translation_parameters #6206

Added support for help_translation_parameters and label_translation_parameters #6206

tkuska commented Jul 16, 2020

jordisala1991 commented Jul 17, 2020

VincentLanglet left a comment

franmomu commented Jul 17, 2020

VincentLanglet commented Jul 17, 2020

tkuska commented Jul 17, 2020

tkuska commented Jul 17, 2020

VincentLanglet Jul 17, 2020

tkuska Jul 17, 2020

VincentLanglet Jul 17, 2020

VincentLanglet Jul 17, 2020

VincentLanglet Jul 17, 2020

VincentLanglet Jul 17, 2020

VincentLanglet commented Jul 17, 2020

franmomu commented Jul 17, 2020

VincentLanglet commented Jul 17, 2020 •

edited

Loading

tkuska commented Jul 17, 2020

VincentLanglet commented Jul 17, 2020 •

edited

Loading

VincentLanglet commented Jul 17, 2020

tkuska commented Jul 18, 2020

VincentLanglet commented Jul 19, 2020

phansys commented Jul 19, 2020

VincentLanglet commented Jul 19, 2020

franmomu commented Jul 21, 2020

tkuska commented Jul 21, 2020

VincentLanglet commented Jul 22, 2020

Added support for help_translation_parameters and label_translation_parameters #6206

Added support for help_translation_parameters and label_translation_parameters #6206

Conversation

tkuska commented Jul 16, 2020

Subject

Changelog

jordisala1991 commented Jul 17, 2020

VincentLanglet left a comment

Choose a reason for hiding this comment

franmomu commented Jul 17, 2020

VincentLanglet commented Jul 17, 2020

tkuska commented Jul 17, 2020

tkuska commented Jul 17, 2020

VincentLanglet Jul 17, 2020

Choose a reason for hiding this comment

tkuska Jul 17, 2020

Choose a reason for hiding this comment

VincentLanglet Jul 17, 2020

Choose a reason for hiding this comment

VincentLanglet Jul 17, 2020

Choose a reason for hiding this comment

VincentLanglet Jul 17, 2020

Choose a reason for hiding this comment

VincentLanglet Jul 17, 2020

Choose a reason for hiding this comment

VincentLanglet commented Jul 17, 2020

franmomu commented Jul 17, 2020

VincentLanglet commented Jul 17, 2020 • edited Loading

tkuska commented Jul 17, 2020

VincentLanglet commented Jul 17, 2020 • edited Loading

VincentLanglet commented Jul 17, 2020

tkuska commented Jul 18, 2020

VincentLanglet commented Jul 19, 2020

phansys commented Jul 19, 2020

VincentLanglet commented Jul 19, 2020

franmomu commented Jul 21, 2020

tkuska commented Jul 21, 2020

VincentLanglet commented Jul 22, 2020

VincentLanglet commented Jul 17, 2020 •

edited

Loading

VincentLanglet commented Jul 17, 2020 •

edited

Loading