-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Translation] Translatable objects #37670
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
Conversation
I like this implementation, primarily because you can add parameters to the message without having to define that elsewhere. Meaning you can return this object rather than having to return and array where the first key is the message, or a custom data container etc. In the past I've built something similar where a process would create a similar object and then it translate this in another request: {{ translatableMessage.key | trans(translatableMessage.parameters) }} I like this idea because you push the dependency of translating to where it belongs; the view. I suspect this would also benefit translations within the form component where you can easily pass parameters if needed. |
im wondering what type of objects would be "translatable", esp. in direct relation to a single message. To me it hints for a value object instead, rather then a contract.
does this mean only the |
You'll need the contract to get the message key and parameters, possibly domain and language.
This is automatically the case if the translator can deal with it. |
i meant interface. |
Can do that, though I can also see advantages of (serializable) custom objects. |
I like the idea of putting One other example I'd like to provide, is at the end, it would be great to be able to use |
do you have any other implementation in mind? I dont see polymorphism being a requirement here. Im not sure i follow your "addFlash" example, but it could account for Translatables as a feature. Not sure about the |
I don't have any in mind at the moment, but potentially someone else might have a reason for them. We could just add a concrete class only, but though it might be a good to allow flexibility there. Users could also just use their own translator service to get around this limitation as well. Let me know if you want me to remove the interface.
In a basic CRUD setup, typically we'd add a flash message saying what was done and display it as a flash message on the next page. Using a translatable object would be great to avoid having to pass an already translated string and being able to just display on the next page without special handling or having Symfony think we're missing a translation, as we would like to just always translate the message in the view.
I'm open to either way of dealing with this. Let me know what direction you'd like this to take. |
We have built basically the same feature here: https://github.com/Becklyn/rad/blob/8.x/src/Translation/DeferredTranslation.php We have some additional functionality, where not all of it makes sense here:
The point is: from our POV the whole "translate this without using the translator" is a core feature and without these additional use cases it is really cumbersome to use. PS: in terms of our |
I don't see the added benefit of this. There's a very specific dataset you put in the data object. Handling these custom translations are not a part of the data object, nor should they be. If you need to do something custom, you can use composition (if the interface stays). |
If the interface stays, that would be fine as well. Yeah, but for example things like merging parameters is (imo) a core feature. So if it's decided that this doesn't make it to the There were quite a few of these in the past, so I'm just trying to chime in to avoid that here. Sorry for the noise. |
Merging parameters y/n would be a Translator implementation detail :) |
Hey @ro0NL, can you take another look at this? Once you're happy with where things are at, I think it would make sense to add a shorthand function (currently leaning towards |
/** | ||
* @return string The message id (may also be an object that can be cast to string) | ||
*/ | ||
public function getMessageId(): string |
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.
I would use getMessage()
as a message can be a plain English sentence or an id.
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.
@fabpot i proposed messageId. I think it's semantically more clear, as ultimately this is the ID value (even if it's a plain English sentence).
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.
message
is what is used everywhere in the component, we don't use message id anywhere, so I still think that message should be used here.
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.
I agree with this. Updated the PR.
65ac028
to
f6370da
Compare
src/Symfony/Component/Translation/Resources/functions/translatable.php
Outdated
Show resolved
Hide resolved
Can you rebase on current master (to remove the merge commit)? |
1b11bf8
to
e08aebb
Compare
@fabpot rebased onto master |
e08aebb
to
dc6b3bf
Compare
Thank you @natewiebe13. |
This PR was squashed before being merged into the master branch. Discussion ---------- Translatable objects Goal: Add docs for translatable objects. Feature: symfony/symfony#37670 Fixes #14145 Unsure how in-depth the docs should go. Open to feedback and collaboration. Commits ------- a2ad274 Translatable objects
Scenario
When writing business logic, this code typically lives within services. With complex situations, there can be lots of business logic exceptions or messages being created. In order to have the best user experience, we try to display detailed messages to the end user so if it's an error with some data, they know what they need to do to correct the situation. Sometimes this is through a message on a page, an email, etc. In some cases a generic message might be displayed and we log the message instead or send the message in an email to a 3rd party.
The service shouldn't need to care how the message is going to be handled, but just needs to create it in a way that it can be translated if needed.
Options
Translating when the message is created. There are two issues with this approach. Autowiring the translator interface to services that will display an error isn't ideal as it will increase the dependencies the service has. The larger issue is that because we would be throwing an exception that contains a string or passing a message that's a string (what the translator returns), if we have generic twig templates that run the translator function on a message, we're doing a double translation and the debug toolbar will indicate that there is a missing message and turn red. This makes finding missing translations much more difficult as we have to determine if there are any false positives and turns the red indicator into white noise.
Translating when displaying on a template. This option is possible, but adds complexity to our templates. Each time we display a message, we need to have some kind of logic that checks to see if it should be ran through the translator as-is or we need to get the data from the object and pass into the translator.
Proposed Solution
Adding a Phrase object that we can create and pass around reduces the template complexity and adds a nice way to create objects that can be translated at any point. This something that will be very familiar with Magento developers and other platforms also have similar solutions.
TODO
Phrase
asMessage
will likely get confused with Messenger__()
in MagentoHappy to get as much feedback as possible. I know a feature like this would be very welcomed by my team.