Make default notifications translatable #23885
Conversation
This uses the JSON translation-strings feature to make it easier to localize notifications and the password reset mail.
|
I also added a note to the docs. |
| ->line('You are receiving this email because we received a password reset request for your account.') | ||
| ->action('Reset Password', url(config('app.url').route('password.reset', $this->token, false))) | ||
| ->line('If you did not request a password reset, no further action is required.'); | ||
| ->line(__('You are receiving this email because we received a password reset request for your account.')) |
There was a problem hiding this comment.
I feel that these keys should be more succinct in their key naming and stored in the default lang file in resources/lang/en in the laravel/laravel repository
There was a problem hiding this comment.
@countnoobula Taylor mentioned that he'd accept a pull-request if the json translation features was used instead of translation keys (see laravel/ideas#409).
There was a problem hiding this comment.
The auth component can't use helpers defined in the foundation package.
There was a problem hiding this comment.
I actually tested this before submitting the PR and it does work as expected.
There was a problem hiding this comment.
I actually tested this before submitting the PR and it does work as expected.
Did you test using the auth component outside of the framework?
There was a problem hiding this comment.
This notification won't work without the rest of the framework anyway, will it? It can't be used in isolation.
There was a problem hiding this comment.
It should work without the foundation package, such as in Lumen. If there is genuinely code in here that doesn't work, even with every illuminate package installed, then it is in the wrong place, and should be in foundation.
There was a problem hiding this comment.
Correct me if I'm wrong: Lumen does not use the ResetPassword notification. And if one wants to use it they need to pull in all sorts of packages anyway (that are not defined in auths composer.json).
The thing is: There should be an easy way to localize password resets. Right now there isn't. If Foundation is the one package that can't be pulled in if people want this feature, then this file really should be moved to foundation.
There was a problem hiding this comment.
@GrahamCampbell Ahh.. I see. Foundation is not a seperate package that can be pulled in. Shame on me.
That said.. all the other password reset stuff is already in foundation.. so this should probably go there, too, right?
| ->line('You are receiving this email because we received a password reset request for your account.') | ||
| ->action('Reset Password', url(config('app.url').route('password.reset', $this->token, false))) | ||
| ->line('If you did not request a password reset, no further action is required.'); | ||
| ->line(__('You are receiving this email because we received a password reset request for your account.')) |
There was a problem hiding this comment.
The auth component can't use helpers defined in the foundation package.
|
I suggest using toMailUsing() method to register your own way of sending this notification. |
|
@taylorotwell Apart from that, are you okay with translating the default notification strings in https://github.com/laravel/framework/blob/5.6/src/Illuminate/Notifications/resources/views/email.blade.php? (Edited) |
This uses the JSON translation-strings feature to make it easier
to localize notifications and the password reset mail.