Skip to content
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

[5.6] i18n of the default notification email view #23903

Merged
merged 2 commits into from
Apr 25, 2018
Merged

[5.6] i18n of the default notification email view #23903

merged 2 commits into from
Apr 25, 2018

Conversation

dakira
Copy link
Contributor

@dakira dakira commented Apr 16, 2018

This uses json translations through the blade directive to allow easy
translations of mail notifications.

In contrast to my previous PR this doesn't change the ResetPassword notification as that can be easily changed with the toMailUsing() method. It also doesn't use the __() helper so no extra dependencies are introduced.

This uses json translations through the blade directive to allow easy
translations of mail notifications.
@GrahamCampbell GrahamCampbell changed the title i18n of the default notification email view [5.6] i18n of the default notification email view Apr 16, 2018
@tillkruss
Copy link
Collaborator

Could you post the exact generated markdown before and after?

@dakira
Copy link
Contributor Author

dakira commented Apr 17, 2018

@tillkruss do you have a hint on getting that? I couldn't find anything in the docs. Is the generated markdown saved in the storage folder somewhere?

@tillkruss
Copy link
Collaborator

No, I'd probably send two emails and look that their raw data.

I'm asking for this because whitespace is not ignored in that template and I don't know how the @lang is replaced exactly.

@dakira
Copy link
Contributor Author

dakira commented Apr 17, 2018

There was one expected whitespace difference in the end because the original markdown has a hard linebreak. There's no difference in the html view of the mail but there is one in the text view. I pushed another commit to fix that. Here's a gist with before and after. For the test I created a de.json with a translation of "Hello!" to see if the translation works.

@taylorotwell taylorotwell merged commit 0b4a5b7 into laravel:5.6 Apr 25, 2018
@dakira dakira deleted the feature/i18n-notifications branch April 26, 2018 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants