-
-
Notifications
You must be signed in to change notification settings - Fork 345
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
Translation is broken in oneToMany edit form #657
Conversation
@@ -23,7 +23,7 @@ file that was distributed with this source code. | |||
style="display:none;" | |||
{% endif %} | |||
> | |||
{{ nested_field.vars.label|trans({}, nested_field.vars.translation_domain) }} | |||
{{ nested_field.vars.label|trans({}, nested_field.vars['sonata_admin'].admin.translationDomain|default(nested_field.vars.translation_domain)) }} |
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.
Please linebreak this.
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.
This looks funny… what is vars['sonata_admin']
? You're calling .admin
on it…
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.
This was the old implementation
https://github.com/sonata-project/SonataDoctrineORMAdminBundle/pull/651/files#diff-53ecac26c8f66fe65ba20a64eaf055b8L26
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.
Had a deeper look, I think that sonata_admin
is an instance Sonata\AdminBundle\Form\Type\AdminType
here, or something like that… so it must be fine (I hope)
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.
No idea how to solve this in 80 chars per line without destroying readability :/
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.
How about this :
{{ nested_field.vars.label
|trans({}, nested_field.vars['sonata_admin'].admin.translationDomain
|default(nested_field.vars.translation_domain)) }}
The last line is indented twice to make it clear what is nested in what, exactly like we would do in php.
e4fd8ab
to
03c3165
Compare
Can you approve this @greg0ire ? |
LGTM 👍 |
I am targetting this branch, because this a bugfix.
Closes #656
Changelog
Subject
The PR #651 breaks the translation of oneToMany form fields in the table view.