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

[fixes #3833] Take locale into account in views.render() #3889

Merged
merged 3 commits into from
Nov 16, 2016

Conversation

Josebaseba
Copy link
Contributor

@Josebaseba Josebaseba commented Nov 14, 2016

Support locale as options, broken since v0.12.5

Complete info here / fixes: #3833

Tests added to this functionality too, with different languages and default values. Plase take a look if this is what you want 👍

Thanks guys

@sailsbot
Copy link

Hi @Josebaseba! It looks like your pull request title doesn’t quite conform to our guidelines. Please edit the title so that it starts with [proposal], [patch], [fixes #], or [implements #]. Once you've fixed it, post a comment below (e.g. "ok, fixed!") and we'll take a look!

@Josebaseba
Copy link
Contributor Author

ok, fixed!

@sailsbot
Copy link

Hi @Josebaseba! It looks like your pull request title doesn’t quite conform to our guidelines. Please edit the title so that it starts with [proposal], [patch], [fixes #], or [implements #]. Once you've fixed it, post a comment below (e.g. "ok, fixed!") and we'll take a look!

@Josebaseba Josebaseba changed the title Take locale into account in views.render() [fixes] Take locale into account in views.render() Nov 14, 2016
@Josebaseba
Copy link
Contributor Author

Fixed this time? Please don't hate me sailsbot... I'm doing my best 😢

@sailsbot
Copy link

Hi @Josebaseba! It looks like your pull request title doesn’t quite conform to our guidelines. Please edit the title so that it starts with [proposal], [patch], [fixes #], or [implements #]. Once you've fixed it, post a comment below (e.g. "ok, fixed!") and we'll take a look!

@Josebaseba Josebaseba changed the title [fixes] Take locale into account in views.render() [fixes #3833] Take locale into account in views.render() Nov 14, 2016
@Josebaseba
Copy link
Contributor Author

😢 😢 😢

@mikermcneil
Copy link
Member

@Josebaseba thanks! Will take a look asap (realistically later today or tomorrow morning)

var Sails = require('../../../lib').constructor;
var path = require('path');

describe('res.render() with i18n', function (){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Josebaseba minor thing: can we change the "describe" label to say sails.hooks.views.render() instead of res.render?

Copy link
Member

@mikermcneil mikermcneil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Josebaseba would you change the label of the describe and add an inline comment above the req.headers['accept-langua... bit just to explain what/why it's doing for future reference?

Thanks again!

@Josebaseba
Copy link
Contributor Author

@mikermcneil ping!

@mikermcneil
Copy link
Member

@Josebaseba thank you!

@mikermcneil mikermcneil merged commit d0fa3b6 into balderdashy:0.12 Nov 16, 2016
@mikermcneil
Copy link
Member

@Josebaseba this is now included if you do npm install sails@^0.12.10-1. Would you give that a try? If it looks good, I'll include it in the patch later today.

@Josebaseba
Copy link
Contributor Author

captura de pantalla 2016-11-16 a las 21 52 37

captura de pantalla 2016-11-16 a las 21 55 21

And so on...

Everything works perfectly! All the tests and all the pages/emails related with i18n 👍

Thanks @mikermcneil

@mikermcneil
Copy link
Member

@Josebaseba Nice! Publishing momentarily (just waiting on one other verification re sockets since this 0.12.10 includes a couple of other changes)

@karsasmus
Copy link

@Josebaseba :
2 typos in your german translation (picture):

  • second line, first word: it should be "wird", not "werden"
  • second line, after fourth word "helfen": there should be a comma.

@Josebaseba
Copy link
Contributor Author

uou! thanks @karsasmus, I'm gonna fix it right now 👍

@mikermcneil
Copy link
Member

(btw @Josebaseba if you get a chance, would you mind rebasing this on master?)

sgress454 pushed a commit that referenced this pull request Nov 22, 2016
[fixes #3833] Take locale into account in views.render()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants