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

Add i18n #27

Merged
merged 18 commits into from Sep 28, 2016
Merged

Add i18n #27

merged 18 commits into from Sep 28, 2016

Conversation

Niharika29
Copy link
Collaborator

  • Add SimpleI18n support to project
  • Extract out all message keys to en.json
  • Replace all keys in twig and js by message keys
  • Update README

"js-unauthorized": "You need to be logged in to be able to review.",
"js-dberror": "There was an error in connecting to database.",
"js-undo-own-only": "You can only undo your own reviews.",
"js-unknown-error": "An unknown error occurred when loading results. Please try again."
Copy link
Member

Choose a reason for hiding this comment

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

The only thing is I would avoid including links and markup in the messages. E.g. you get rid of footer-source and instead just have "source": "Source" and our HTML, <a href="https://github.com/wikimedia/PlagiabotWeb">{{ 'source'|message }}</a>. The markup is not translatable and there's a chance someone might unintentionally break it.

And for footer-poweredby you could use: Powered by $1 and $2 since EranBot and Turnitin are also not translatable. If they are, make a separate message for turnitin then in our HTML:
{{ 'footer-poweredby'|message | ( <a href="https://en.wikipedia.org/wiki/Wikipedia:Turnitin">{{ 'turnitin'|message }}</a> | <a href="https://en.wikipedia.org/wiki/User:EranBot">{{ 'EranBot'|message }}</a> }} (this is pseudocode, not sure how SimpleI18n works)

Copy link
Member

Choose a reason for hiding this comment

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

Also it should be https://github.com/wikimedia/CopyPatrol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. I also fixed some links in Image attribution modal. I didn't break it up into 12 messages but 8 instead.
The link in footer is https://github.com/wikimedia/CopPatrol only. Did you see otherwise anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, simpleI18n doesn't support the sort of markup you were suggesting above so I resorted to break out the text.

@MusikAnimal
Copy link
Member

Trying out these Gerrrit-wannabe features on GitHub, "Changes requested" doesn't mean I'm demanding any changes!

@MusikAnimal
Copy link
Member

MusikAnimal commented Sep 16, 2016

I pushed directly to your branch, hope that's OK...

To pass markup containing messages as a parameter to a message, I found you could use {% set variable %} followed by a block of HTML.

A few notes that I've learned doing i18n:

  • For simple words or phrases, it's probably better to not make them specific to a particular part of the interface unless you are sure the message may differ from the identical translation based on where it's located. So if we used the word "WikiProjects" somewhere else, we wouldn't have to ask for a new translation but instead use the existing "wikiprojects" message (as opposed to "form-wikiprojects" and "col-wikiprojects").
  • Avoid asking for translations of proper nouns. E.g. "EranBot" is a username and would be the same in every language
  • Finally, we need to add qqq documentation, like this. For direct translations of words, such as "login", append "\n{{Identical}}" to the end of qqq message.

Sorry if I'm being anal! I just think we should try to make it easy as possible for the translatewiki folks, because they are awesome.

Other not important things:

  • I used spaces to separate the pipes in the twig templates. I found this easier to read, but purely a nitpick. Maybe I should find a Twig syntax highlighter.
  • JavaScript variables are by convention camelCased
  • For public-facing attribution I prefer to go by MusikAnimal :)

That is the extent of my stupid quibbles!

@Niharika29
Copy link
Collaborator Author

A few notes that I've learned doing i18n:

For simple words or phrases, it's probably better to not make them specific to a particular part of the interface unless you are sure the message may differ from the identical translation based on where it's located. So if we used the word "WikiProjects" somewhere else, we wouldn't have to ask for a new translation but instead use the existing "wikiprojects" message (as opposed to "form-wikiprojects" and "col-ewikiprojects")

For Copypatrol, I think I agree. When I did i18n for Wikimania scholarships app and Grants app, it was very prone to interface changes from time to time and hence separate keys were encouraged.

Avoid asking for translations of proper nouns. E.g. "EranBot" is a username and would be the same in every language

Got it.

Finally, we need to add qqq documentation, like this. For direct translations of words, such as "login", append "\n{{Identical}}" to the end of qqq message.

Thanks for reminding me to do this!

Sorry if I'm being anal! I just think we should try to make it easy as possible for the translatewiki folks, because they are awesome.

Agreed. :)

Other not important things:

I used spaces to separate the pipes in the twig templates. I found this easier to read, but purely a nitpick. Maybe I should find a Twig syntax highlighter.

I've worked with twig before for quite a while (here, for example) and never used spaces with the pipe separator. If you don't mind, I'll like to continue to have it that way.

JavaScript variables are by convention camelCased

Oh, right.

For public-facing attribution I prefer to go by MusikAnimal :).

Will remember in future.

safe - dont fail on missing metadata
"phabricator": "Phabricator",
"footer-imageattr": "Image attribution",
"lboard-title": "Leaderboard",
"lboard-week": "Cases closed in the<br/> last 7 days",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hardcoded linebreaks don't really work for translation, since the translators won't know where to put the linebreaks to make sure things wrap correctly. Danny originally wanted this done so that all the columns lined up right, but we can probably figure out a better way to do this in a future commit. For now, let's just remove the line breaks and let the lines wrap naturally (even if they don't all line up perfectly).

"form-mine": "My reviews",
"form-drafts": "Drafts only",
"wikiprojects": "Wikiprojects",
"form-wikiprojects-placeholder": "Type Wikiproject names...",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"WikiProject" is always camelcase.

@Niharika29
Copy link
Collaborator Author

I've made the fixes and rebased the patch. I took this opportunity to split out the header, footer and modal templates from the base template. The code was nearly incomprehensible.
I've let the twig pipe spaces be because I don't mind them if that's what MusikAnimal prefers. We're nearly done with this app anyway.

Copy link
Member

@bd808 bd808 left a comment

Choose a reason for hiding this comment

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

A few nits inline. Mostly about making the qqq descriptions more useful for translators.

$editor = null;
if ( isset( $record['diff'] ) && isset( $editors[$record['diff']] ) ) {
$editor = $editors[$record['diff']];
}
Copy link
Member

@bd808 bd808 Sep 20, 2016

Choose a reason for hiding this comment

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

Block overindented?

"documentation": "Link to documentation in header\n{{Identical|Documentation}}",
"feedback": "Link to feedback in header\n{{Identical|Feedback}}",
"leaderboard": "Link to leaderboard in header",
"login": "Butten label\n{{Identical|login}}",
Copy link
Member

Choose a reason for hiding this comment

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

"Button"

"record-editcount": "Label to indicate number of edits a user has made",
"compare": "Button label to open a panel for text comparison between two pages",
"record-ithenticate": "Button label for a button to go to the original plagiarism report generated by Turnitin",
"record-url-text": "Label text to show amount of plagiarism.\n Parameters: $1 - Percentage of edit that was plagirized.\n $2 - Number of words plagiarized.",
Copy link
Member

Choose a reason for hiding this comment

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

"plagiarized"

"compare-source-loading": "Temporary text shown to user while copyvio text source loads",
"footer-loadmore": "Label for a button to load more records.",
"footer-nomore": "Label to indicate no more records in database.",
"footer-broughtby": "Footer text to indicate authors. \n Parameters: $1 - String.",
Copy link
Member

Choose a reason for hiding this comment

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

$1 - List of names?

"footer-nomore": "Label to indicate no more records in database.",
"footer-broughtby": "Footer text to indicate authors. \n Parameters: $1 - String.",
"footer-commtech": "Footer text - name of team.",
"footer-poweredby": "Footer text to highlight sources used for development. \n Parameters: $1 - String. $2 - String.",
Copy link
Member

Choose a reason for hiding this comment

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

"String" is not descriptive at all for a translator.

"public-domain": "Text as part of the attributions box",
"open-font-license": "Text as part of the attributions box",
"imageattr-paragraph1": "Text as part of the attributions box. \n Parameters: $1, $2, $3, $4 - Strings.",
"imageattr-paragraph2": "Text as part of the attributions box. \n Parameters: $1, $2, $3 - Strings.",
Copy link
Member

Choose a reason for hiding this comment

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

Again, the "Strings" description won't help a translator understand the resulting sentence/phrase.

Copy link
Member

Choose a reason for hiding this comment

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

We might want to make it clear the imageattr messages should be gender-neutral (User:Xander89 hasn't specified their gender, other proper nouns are not human) and that {{GENDER}} should not be used, since the parameters are links. In Intuition a PHP error is thrown if GENDER is used on markup, I think, not sure if the same is true for Simplei18n

Copy link
Collaborator

@kaldari kaldari left a comment

Choose a reason for hiding this comment

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

A couple qqq suggestions.

"Ryan Kaldari"
]
},
"name": "Title of the application/tool",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may want to mark this one as optional to translate, since it's a proper noun and a translation isn't strictly needed. To mark something as optional to translate, add {{Optional}}\n to the beginning of it.

"name": "Title of the application/tool",
"documentation": "Link to documentation in header\n{{Identical|Documentation}}",
"feedback": "Link to feedback in header\n{{Identical|Feedback}}",
"leaderboard": "Link to leaderboard in header",
Copy link
Collaborator

@kaldari kaldari Sep 20, 2016

Choose a reason for hiding this comment

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

"Leaderboard" may be tricky to understand. You might want to explain this better, like "Link to a list of the top reviewers in header".

Copy link
Member

@bd808 bd808 left a comment

Choose a reason for hiding this comment

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

LGTM

@kaldari kaldari merged commit 22d401b into wikimedia:master Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants