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

i18n round 2 #974

Closed
wants to merge 4 commits into from
Closed

i18n round 2 #974

wants to merge 4 commits into from

Conversation

vencax
Copy link
Contributor

@vencax vencax commented Dec 30, 2017

Hi, this is continuation of #403. I have made another PR because the I was not sucessfull with rebase. This could be easier to merge IMHO.
Cheers, and happy New Year!

@verythorough
Copy link
Contributor

verythorough commented Dec 30, 2017

Deploy preview for netlify-cms-www ready!

Built with commit 1ceff1c

https://deploy-preview-974--netlify-cms-www.netlify.com

@verythorough
Copy link
Contributor

verythorough commented Dec 30, 2017

Deploy preview for cms-demo ready!

Built with commit 1ceff1c

https://deploy-preview-974--cms-demo.netlify.com

@vencax
Copy link
Contributor Author

vencax commented Dec 30, 2017

#403 (comment) mentions "expose i18n to widget authors, so that they can provide an object of translation objects with the widget". IMHO this could be added later. The core needs to be i18n-ed IMHO asap.

@andrevandal
Copy link
Contributor

Hi, when I run npm run extract --lang pt-br on your PL, I got nothing.
The code stops on

  if (file === enFile || file === 'index.js') {
    return
  }

@vencax could u tell me how I should extract strings?

@vencax
Copy link
Contributor Author

vencax commented Jan 13, 2018

@derevandal this was undone, thank for pointing at it. See the last commit for details. Cheers!

@andrevandal
Copy link
Contributor

@vencax your code didn't work here.
i'm using node v8.9.4.
I change your code to

const LangParam = process.argv[2];
if (LangParam) {
  const filePath = path.join(i18nFolder, `${ LangParam }.json`);
  const translations = {};
  keys.map(i => translations[i.key] = i.key);
  writeTranslations(filePath, translations);
}

and it works.
see doc

@andrevandal
Copy link
Contributor

@vencax could u tell me how do i change lang of cms by extracted lang?

@vencax
Copy link
Contributor Author

vencax commented Jan 16, 2018

@derevandal
It works as it was. At least for me. Why it didn't work for you? HAve you run it --lang pt-br ??
And you can set the lang with putting into your config.yam:
lang: pt-br

@andrevandal
Copy link
Contributor

@vencax when i used npm run extract --lang didn't work, but it works with node extract.js --lang pt-br.
i'm going to test the cms with pt-br translation and i'll post a review.

@andrevandal
Copy link
Contributor

why don't u use languageCode instead of lang if config.yaml already had languageCode?
and i extract pt-br lang, add lang: "pt-br" in config.yaml, transtale some parts and I didn't found tranlated entries. could u help me?

pt-br.json

  "Draft": "Draft",
  "New Post": "Nova Publicação",

@erquhart
Copy link
Contributor

@derevandal did you ever get this working?

@vencax I do want to get this in, just wanted at least one more person to review, especially someone that has a translation use case.

That said, I think the only real blockers are:

  • the issue outlined by @derevandal
  • the string keys are the full current english text of the strings they replace rather than just being short, camel cased descriptors.

This change has a big impact on the codebase, as the translation function is in almost every UI file, so I want to make sure it's maintainable. Thanks again for your work on this!

@andrevandal
Copy link
Contributor

@erquhart nope. nor was I able to extract or use the extracted file manually.

@tech4him1 tech4him1 mentioned this pull request Mar 29, 2018
@erquhart
Copy link
Contributor

Closing as stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants