-
Notifications
You must be signed in to change notification settings - Fork 321
A model can have a default locale for translations #271
Conversation
Skipping App::setLocale() and setting a protected variable on the fly means we need to assign the locale for each different model which is a smell because it would be easy to forget to set the locale for another model. Thoughts? |
You don't need to change anything, and the user doesn't need to define the $defaultLocale for each model. As you can see in the locale() method i'm taking the $defaultLocale into consideration only if a value exists. If not we check the config values. To use the default locale the user have two options :
|
Can you please explain why you need this feature? |
So i have a DataSet model with and "fr" and "en" translations. When showing this model, i will not always use the application locale, but i'll use the locale defined in the url. The url for showing the model is this "data-sets/{id}/{locale}" What i need it to always use this syntax in my views So i have two options :
Now when it comes to the code of my pull request, it doesn't change a bit about how the package works (edit: of course except for the fact that adds the possibility to set the default locale for each model separately). People already using the package doesn't need to touch their code at all. The $defaultLocal attribute is already defined in the trait, so the users of the package don't need to define it in their models. The locale() will take the $defaultLocale into consideration only if :
This addition will make the package more flexible and most important of all it doesn't require any changes to existing application code. |
Sorry for the long delay. Pull requests cannot be merged without tests supporting the changes. Your point makes sense though and it seems to me we should add this feature. |
Ok i'll add tests |
This will allow to return the translation in the desired locale, by calling the attribute on the model without the need of changing the application locale using App::setLocale();
@dimsav Rebased and added tests Can you please confirm that all tests are passing, because i tried running them and i get this error
The problem is the same in homestead and on my windows machine |
This looks great @shadoWalker89 ! The tests work great. |
This will allow to return the translation in the desired locale, by calling the attribute on the model without the need of changing the application locale using App::setLocale();