I18n

[billybonks]

Initial pull request for translations. let me know what you think.

i added some comments about things we can clean, in commit descriptions

[jglovier]

Curious what is the purpose of this? And if there is an obvious design change to the UI, please post a screencap.

[jglovier]

Nm, just realized the value is capitalize and not uppercase. 👍

[jglovier]

@billybonks thanks for opening this one!! Really excited for internationalization. 😍 ⚡

So to clarify, it looks like the general approach is that with these changes if someone wants a different language they are responsible for the actual translation work (which takes place in the translations.js file), correct?

[billybonks]

I capitalised the letters of every button with that CSS :). Ye they create
a folder using the countries ISO code duplicate translations.js and replace
phrases with the desired language

On Mon, 28 Dec 2015, 07:53 Joel Glovier [email protected] wrote:

@billybonks https://github.com/billybonks thanks for opening this one!!
Really excited for internationalization.

So to clarify, it looks like the general approach is that with these
changes if someone wants a different language they are responsible for the
actual translation work (which takes place in the translations.js file),
correct?

—
Reply to this email directly or view it on GitHub
#248 (comment)
.

[jkleinsc]

@billybonks looks good what you have done so far. The only thing I'm not sure about is the capitalization of ALL the buttons. For example inventory has some buttons that were intentionally lower cased (e.g. + new request). I don't know that I care one way or the other, but its a change that goes against original design. @jglovier do you have any thoughts on this?

[jkleinsc]

Also, I am thinking maybe we breakup the rest of the internationalization into separate github issues for each module unless @billybonks you want to do the whole app with this PR.

[jglovier]

@jglovier do you have any thoughts on this?

Yeah, I agree with @jkleinsc. It seems helpful but there are some cases where we don't actually want button text capitalized so lets drop that part.

Also, I am thinking maybe we breakup the rest of the internationalization into separate github issues for each module unless @billybonks you want to do the whole app with this PR.

Also agreed. Always be keeping PRs as small in scope as possible. 👍

[billybonks]

@jglovier @jkleinsc

some buttons that were intentionally lower cased (e.g. + new request).

I actually think that + New request looks better then + new request, what is the reason for it being lower case. ill remove the commit, and take it case by case.

@jkleinsc

lets break the issue down, as I'm currently running around thailand, and wouldn't have an eta on completing the entire app. but i will continue to add translations

[jglovier]

I actually think that + New request looks better then + new request, what is the reason for it being lower case.

Honestly, it was sort of an arbitrary UI decision. I think the loose rationale was that it's not a sentence so it doesn't need capitalized, but in hindsight and on further scrutiny I think that's a dumb reason and agree with you that it's more appropriate capitalized. 😄

That being said, I think (at least for now) that keeping the global capitalization out of this effort is best.

[turboMaCk]

👏 Nice PR so far!

[billybonks]

@jglovier i agree, removed the commit.

I still want to translate a few more things, to see how the current format plays out. so lets hold out on merging. i have a 2hour ferry ride tomorrow so i can make more commits

[jkleinsc]

ok sounds good @billybonks. Let me know when the PR is ready.

One thing I noticed was on the imaging requests screen (e.g. http://localhost:4200/#/imaging), there is a missing label. I am wondering if we can add a test helper that automatically checks for missing labels. Long term, I think it would be great to have so that you could verify if translations were missing labels simply by running tests.

[billybonks]

good catch, i agree, it has been something that has been worrying me. ill see if i can hook into the translations helper and add a an assertion. my only concern is it getting to work well with expect, but i have an idea of how i can do this.

can we unify

'date requested' and 'requested on'` i think they mean the same thing

[jkleinsc]

@billybonks I missed your question here. Yes we can unify 'date requested' and 'requested on' . I would go with 'date requested'

[billybonks]

I played around with the testing strategy, but didn't find an awesome solution, so lets smoke test for now. i will do the date requested and requested on in a separate pr

[jkleinsc]

@billybonks sounds good to me. If you can fix the test that is failing (Element button:contains(Sign in) not found.), I'll merge this PR.

[jkleinsc]

@billybonks There was still a test failing, but I wanted to bring this PR into master, so I fixed the issue with the test and pulled it into master. Thanks for the PR.