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

Mark and remove some translations #5110

Merged
merged 4 commits into from Oct 4, 2017

Conversation

@pafcu
Copy link
Contributor

pafcu commented Sep 23, 2017

Mark some strings as translatable in the source code, and remove some obsolete strings. Depends on matrix-org/matrix-react-sdk#1421

The following strings were found to be unnecessary:

Welcome page
Room directory
 from room
You are Rioting as a guest. <a>Register</a> or <a>sign in</a> to access more rooms and features!
Create new room
Please Register
Drop here %(toAction)s
This room is inaccessible to guests. You may be able to join if you register.
Failed to join the room
Guest users can't invite users. Please register to invite.
Start chat
Failed to
 to room
Settings
pafcu added 2 commits Sep 23, 2017
Signed-off-by: Stefan Parviainen <pafcu@iki.fi>
Signed-off-by: Stefan Parviainen <pafcu@iki.fi>
@pafcu pafcu changed the title Mark and remove translations Mark and remove some translations Sep 23, 2017
Signed-off-by: Stefan Parviainen <pafcu@iki.fi>
Copy link
Contributor

lukebarnard1 left a comment

This awesome, thanks @pafcu

@@ -6,10 +6,16 @@
- Be able to understand English
- Be able to understand the language you want to translate riot-web into

## Translating strings vs. marking strings for translation

Translating strings are done with the `_t()` function found in matrix-react-sdk/lib/languageHandler.js. It is recommended to call this function wherever you introduce a string constant which should be translated. However, translating can not be performed until after the translation system has been initialized. Thus, sometimes translation must be performed at a different location in the source code than where the string is introduced. This breaks some tooling and makes it difficult to find translatable strings. Therefore, there is the alternative `_td()` function which is used to mark strings for translation, without actually performig the translation (which must still be performed separately, and after the translation system has been initialized).

This comment has been minimized.

Copy link
@lukebarnard1

lukebarnard1 Oct 3, 2017

Contributor

typo: "performig"

This comment has been minimized.

Copy link
@lukebarnard1

lukebarnard1 Oct 3, 2017

Contributor

I think it would be useful to provide a small code-snippet example of a string being defined as a module constant, and then later translating at the point of rendering.

@@ -21,6 +27,8 @@

## Things to know/Style Guides

- Do not use it inside ``getDefaultProps`` at the point where ``getDefaultProps`` is initialized the translations aren't loaded yet and it causes missing translations.
- If using translated strings as constants, translated strings can't be in constants loaded at class-load time since the translations won't be loaded.
- Do not use `_t()` inside ``getDefaultProps`` at the point where ``getDefaultProps`` is initialized when the translations aren't loaded yet and it causes missing translations. Use `_td()` instead and perform the actual translation later.

This comment has been minimized.

Copy link
@lukebarnard1

lukebarnard1 Oct 3, 2017

Contributor

Can I suggest, "Do not use _t() inside getDefaultProps: the translations aren't loaded when getDefaultProps is called, leading to missing translations. Use _td to indicate that _t will be called on the string later "

- If a string is presented in the UI with punctuation like a full stop, include this in the translation strings, since punctuation varies between languages too.
- Avoid "translation in parts", i.e. concatenating translated strings or using translated strings in variable substitutions. Context is important for translations, and translating partial strings this way is simply not always possible.
- Concatenating strings often also introduces an implicit assumption about word order (e.g. that the subject of the sentence comes first), which is incorrect for many languages.

This comment has been minimized.

Copy link
@lukebarnard1

lukebarnard1 Oct 3, 2017

Contributor

Some very good advice, thanks for this 😃

@lukebarnard1

This comment has been minimized.

Copy link
Contributor

lukebarnard1 commented Oct 4, 2017

So apparently _td is undefined:

HeadlessChrome 0.0.0 (Ubuntu 0.0.0) ERROR
  Uncaught TypeError: (0 , _languageHandler._td) is not a function
  at test/src/notifications/VectorPushRulesDefinitions.js:70:21 <- test/all-tests.js:1939
@pafcu

This comment has been minimized.

Copy link
Contributor Author

pafcu commented Oct 4, 2017

It depends on matrix-org/matrix-react-sdk#1421 which has not been merged. Perhaps that is the problem?

@lukebarnard1

This comment has been minimized.

Copy link
Contributor

lukebarnard1 commented Oct 4, 2017

Ah, yep that will be it. I'll merge that and then rerun the tests here.

Copy link
Contributor

lukebarnard1 left a comment

LGTM 🌟

@lukebarnard1 lukebarnard1 merged commit ec41414 into vector-im:develop Oct 4, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pafcu pafcu deleted the pafcu:mark-and-remove-translations branch Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.