issue #218: add russian translations #245
Conversation
There was a problem hiding this comment.
Thanks for taking this up!!
I made one tiny comment, and then we also have moved away from using flags, so there's no need to add the lang-rus.png ... I know the LANGUAGE_TRANSLATION.md` still says to add a flag image so if you wanted you could also remove that step from that file :)
lizzyaustad
force-pushed
the
218_russian_translation
branch
2 times, most recently
from
da54a69
to
9a7935d
Compare
|
@amaxama thanks! I removed the image file and the image references in the new language instructions. Let me know if there were any other issues with the PR I missed |
docs/LANGUAGE_TRANSLATION.md
Outdated
| @@ -145,7 +131,8 @@ constructor() { | |||
| orm, | |||
| oji, | |||
| dak, | |||
| vie | |||
| vie, | |||
| ru | |||
There was a problem hiding this comment.
I think we've mostly been using three-letter codes for languages, could change this to rus or another three-letter code?
lizzyaustad
force-pushed
the
218_russian_translation
branch
from
9a7935d
to
862c56b
Compare
There was a problem hiding this comment.
Thanks for following up on this @lizzyaustad ! I added a couple comments for things I didn't catch the first time around.
| "moment_d": "день", | ||
| "moment_M": "месец", | ||
| "moment_y": "год" | ||
| } |
There was a problem hiding this comment.
We've had a few more values added to the translation spreadsheet, would you mind adding those here?
docs/LANGUAGE_TRANSLATION.md
Outdated
| #### Moment.js | ||
|
|
||
| We use [`moment.js`](https://momentjs.com/) to display text based on time computations ("an hour ago", "a few days ago", "in 3 days"). Some languages are already translated and supported by default. For others, we have created a translation file that will work with `moment.js` which can be found under [`src/locale`](https://github.com/Twin-Cities-Mutual-Aid/twin-cities-aid-distribution-locations/tree/master/src/locale). | ||
|
|
||
| If the language is already supported by `moment.js`, the `locale` value in the `XXX.json` file should match the one that `moment.js` uses. |
There was a problem hiding this comment.
Did you go over these steps to check whether moment.js supports russian or not?
docs/LANGUAGE_TRANSLATION.md
Outdated
| @@ -154,8 +139,6 @@ constructor() { | |||
| #### Add language checklist | |||
|
|
|||
| - [ ] Create a new translation file (`src/i18n/XXX.json`) | |||
| - [ ] Add a new flag image (`public/images/lang-XXX.png`) | |||
| - [ ] `locale` should match existing `moment.js` translation or new `moment.js` locale created (`src/locale/XXX.js`) | |||
There was a problem hiding this comment.
Is there a reason this step for the locale is being removed?
|
@amaxama it looks like my previous comment was removed, but I had mentioned that the moment.js translation did exist and was |
|
@lizzyaustad I don't think I ever saw your previous comments. Oh sorry, I was just saying to use 3 letters when naming the json file since that's what was in the language_translation file, not necessarily for the moment locale. It doesn't really matter that much about the file name, I was just thinking about consistency. Sorry if I was unclear before! But I think the moment stuff is still relevant. |
lizzyaustad
force-pushed
the
218_russian_translation
branch
from
862c56b
to
a1e2f46
Compare
What
Add support for Russian translations
Why
Ticket #218
Ensure the following interactions work as expected. Please test using a mobile form factor.
Check the app in the following web browsers: