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

Fix #12180 Add inline translation tips to translation cards. #12339

Merged
merged 11 commits into from
Apr 3, 2021

Conversation

jimbyo
Copy link
Contributor

@jimbyo jimbyo commented Mar 26, 2021

Overview

  1. This PR fixes or fixes part of #[12180].
  2. This PR does the following: [Adds translation tips to Chinese, Arabic, Bangla, and Hindi]

Essential Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The linter/Karma presubmit checks have passed locally on your machine.
  • "Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
    • This lets reviewers restart your CircleCI tests for you.
  • The PR is made from a branch that's not called "develop".

Proof that changes are correct

Arabic tips

Screen Shot 2021-03-31 at 10 38 06 AM

Chinese tips

Screen Shot 2021-03-31 at 10 38 21 AM

Bangla tips

Screen Shot 2021-03-31 at 10 38 33 AM

Hindi Tips

Screen Shot 2021-03-31 at 10 38 54 AM

PR Pointers

  • Make sure to follow the instructions for making a code change.
  • Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
  • For what code owners will expect, see the Code Owner's wiki page.
  • Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
  • Never force push. If you do, your PR will be closed.
  • Oppiabot can assign anyone for review/help if you leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL"
  • Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

@jimbyo jimbyo requested a review from sagangwee as a code owner March 26, 2021 20:11
@oppiabot
Copy link

oppiabot bot commented Mar 26, 2021

Hi @jimbyo, the karma and linter checklist has not been checked, please make sure to run the frontend tests and lint tests before pushing. Thanks!

@oppiabot
Copy link

oppiabot bot commented Mar 26, 2021

Hi, @jimbyo, this pull request does not have a "CHANGELOG: ..." label as mentioned in the PR checkbox list. Assigning @ jimbyo to add the required label. PRs without this label will not be merged. If you are unsure of which label to add, please ask the reviewers for guidance. Thanks!

@jimbyo
Copy link
Contributor Author

jimbyo commented Mar 26, 2021

@sagangwee I think adding some indented bullet points could be useful, any idea on how to structure the list to do so?

@seanlip
Copy link
Member

seanlip commented Mar 26, 2021

I think adding some indented bullet points could be useful, any idea on how to structure the list to do so?

I think that might be tricky tbh. We might not want to do it for now...

Two questions btw:

(a) Would it be possible to keep the original and translated content together, with the tips shown in a light coloured-background box just below the two?

(b) Do you think it's possible for the Arabic text to read right-to-left? No worries about it if it would be a big change, though.

@oppiabot
Copy link

oppiabot bot commented Mar 27, 2021

Hi @jimbyo, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

@sagangwee
Copy link
Contributor

@jimbyo In the future, can you make sure the PR title is of the format "Fix #12180: ..."? I think that's why the PR was not linked in the issue.

@jimbyo jimbyo changed the title Fixes #12180 Add inline translation tips to translation cards. Fix #12180 Add inline translation tips to translation cards. Mar 29, 2021
@jimbyo
Copy link
Contributor Author

jimbyo commented Mar 29, 2021

(a) Would it be possible to keep the original and translated content together, with the tips shown in a light coloured-background box just below the two?

Yes, that should be simple

(b) Do you think it's possible for the Arabic text to read right-to-left? No worries about it if it would be a big change, though.

I think it is right to left but I'm not a 100% sure. I'll check with the arabic translator.

in the future, can you make sure the PR title is of the format "Fix #12180: ..."? I think that's why the PR was not linked in the issue.

Ok got it.

@seanlip
Copy link
Member

seanlip commented Mar 29, 2021

(b) Do you think it's possible for the Arabic text to read right-to-left? No worries about it if it would be a big change, though.

I think it is right to left but I'm not a 100% sure. I'll check with the arabic translator.

Ah, to be clear -- it is supposed to read right-to-left. But in your mocks it currently doesn't. It should look like this I think: https://scripts.sil.org/cms/scripts/page.php?site_id=nrsi&id=IWS-Chapter09#49327afb

@oppiabot oppiabot bot added the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Mar 29, 2021
@oppiabot
Copy link

oppiabot bot commented Mar 29, 2021

Hi @jimbyo, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

@jimbyo
Copy link
Contributor Author

jimbyo commented Mar 31, 2021

@sagangwee This PR has been updated PTAL!

(a) Would it be possible to keep the original and translated content together, with the tips shown in a light coloured-background box just below the two?

Done and mocks have been updated

(b) Do you think it's possible for the Arabic text to read right-to-left? No worries about it if it would be a big change, though.

Tips have been changed to English as per Anu's instructions, so this is point is no longer relevant.

@oppiabot oppiabot bot removed the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Mar 31, 2021
Copy link
Contributor

@sagangwee sagangwee left a comment

Choose a reason for hiding this comment

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

@jimbyo Took a pass. Thanks!

@@ -1,6 +1,6 @@
// Copyright 2020 The Oppia Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// Licensed under the Apache License, Version 2.0 (the 'License');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not, I used replace all to fix some of the string quotations for the translation dict :P. Removed!

Comment on lines 73 to 153
$scope.TRANSLATION_TIPS = {
zh:
[
'Write fractions or numbers as they are, unless they are written out' +
'in words. For instance, one-fifth would be (五分之一)',
'When referring to Mr. Baker' +
'(or, in general, Mr./Ms. followed by an occupation), ' +
'leave it as Baker先生, since in certain cases Baker is the last name.',
'Make sure to use the correct punctuation:',
'Period = 。',
'Comma for compound sentences or translation phrases = ,',
'Comma for list of numbers or objects = 、',
'Preserve bolding. If the original content has bold text,' +
'make sure it is bold in Chinese as well.',
'Make sure that you have selected the correct words' +
'(e.g. words such as 再 and 在 ).'
],
hi:
[
'Prefer simple Hindi words that are used in daily communication.' +
' Note that common English words (pen, paper, cake, etc.) can be' +
' written as transliterations (पेन, पेपर, केक). For harder words,' +
' include the English word in parentheses, e.g. अंश (Numerator),' +
' हर (Denominator), भिन्न (Fraction).',
'Use respectful pronouns (like “आप” instead of “तुम/तू ”) and a' +
' corresponding respectful tone like “करिये, करेंगे”.',
'Use the same voice (active or passive)' +
' as in the original English text.',
'Preserve punctuation and bolding. If the original content has' +
' bold text, make sure it is bold in Hindi as well. If there' +
' are bullet points, double quotes, etc., make sure that the' +
' translated content also has bullet points and double quotes.',
'If the original card has “components” (such as pictures,' +
' links, and equations), these need to be added to the' +
' translated content. You can use the “Copy tool” for this' +
' -- click on the Copy tool and then click on the component you want' +
' to carry over. Also, double-click on the image and translate the ' +
' alt text (and caption, if any).'
],
bn:
[
'Use simple Bangla words that are used in daily communication.' +
' Note that common English words (pencil, etc.) can be written as' +
' transliterations (e.g পেন্সিল ).',
'Use proper punctuation.',
'Full stop = |',
'Use the same voice (active or passive)' +
' as in the original English text.',
'Preserve punctuation and bolding. If the original' +
' content has bold text, make sure it is bold in' +
' Bangla as well. If there are bullet points, double quotes,' +
' etc., make sure that the translated content also has bullet' +
' points and double quotes.'
],
ar:
[
'In Oppia, we prefer to use simple words that can be easily' +
' understood by children. For example, we use “تابع قائلًا”' +
' instead of “أردف قائلًا”. Furthermore, the English words that' +
' are used in the Arab society regularly can be translated as' +
' follows; Arabic word (The regularly used English word). For' +
' example, we can translate the word cupcakes this way;' +
' كعك القوالب الصغيرة (cupcakes). ',
'Use respectful ways and formal prefixes to address people.' +
' For example, use “سيدي” and “سيدتي”. ',
'If the name has a meaning in Arabic, or in English, such' +
' as Baker or Crumb, always use words that indicate that they' +
' are names before writing the name itself. For example,' +
' you can use one of the following words depending on the' +
' context; “السيد، السيدة، العم، الجد، الجدة، الآنسة.”',
'Use the same voice (active or passive) as in the' +
' original English Text',
'Preserve punctuation and bolding. If the original' +
' content has bold text, make sure it is bold in Arabic' +
' as well. If there are bullet points, double quotes, etc.,' +
' make sure that the translated content also has' +
' bullet points and double quotes.',
'Use the hyperlinks to different cards as shown in' +
' the original English Text.',
]
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this dict to assets/constants.ts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -40,6 +40,13 @@
</schema-based-editor>
</div>
</div>
<div class="oppia-translation-tips-container">
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this look like for a language that does not have translation tips? Can you post a screenshot? Ideally, we would hide the entire container in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After moving the ng-if="TRANSLATION_TIPS[activeLanguageCode]" to the <div> It shows nothing now. Posted a screenshot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Catalan Example
Screen Shot 2021-04-01 at 9 32 50 AM

@@ -99,6 +106,12 @@
margin: 15px 0;
padding: 20px;
}
.oppia-translation-tips-container {
background-color: #ccc;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make the background color slightly lighter? I think the light gray used for the text box editing toolbox would work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, New color posted below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2021-04-01 at 9 35 11 AM

@oppiabot
Copy link

oppiabot bot commented Mar 31, 2021

Unassigning @sagangwee since the review is done.

@oppiabot
Copy link

oppiabot bot commented Apr 1, 2021

Hi @jimbyo, there is a new change in develop which needs to be in your PR. Please update your branch with the latest changes in develop. For instructions, refer to this link. Thanks!

Copy link
Contributor Author

@jimbyo jimbyo left a comment

Choose a reason for hiding this comment

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

@sagangwee PTAL!

@@ -1,6 +1,6 @@
// Copyright 2020 The Oppia Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// Licensed under the Apache License, Version 2.0 (the 'License');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not, I used replace all to fix some of the string quotations for the translation dict :P. Removed!

Comment on lines 73 to 153
$scope.TRANSLATION_TIPS = {
zh:
[
'Write fractions or numbers as they are, unless they are written out' +
'in words. For instance, one-fifth would be (五分之一)',
'When referring to Mr. Baker' +
'(or, in general, Mr./Ms. followed by an occupation), ' +
'leave it as Baker先生, since in certain cases Baker is the last name.',
'Make sure to use the correct punctuation:',
'Period = 。',
'Comma for compound sentences or translation phrases = ,',
'Comma for list of numbers or objects = 、',
'Preserve bolding. If the original content has bold text,' +
'make sure it is bold in Chinese as well.',
'Make sure that you have selected the correct words' +
'(e.g. words such as 再 and 在 ).'
],
hi:
[
'Prefer simple Hindi words that are used in daily communication.' +
' Note that common English words (pen, paper, cake, etc.) can be' +
' written as transliterations (पेन, पेपर, केक). For harder words,' +
' include the English word in parentheses, e.g. अंश (Numerator),' +
' हर (Denominator), भिन्न (Fraction).',
'Use respectful pronouns (like “आप” instead of “तुम/तू ”) and a' +
' corresponding respectful tone like “करिये, करेंगे”.',
'Use the same voice (active or passive)' +
' as in the original English text.',
'Preserve punctuation and bolding. If the original content has' +
' bold text, make sure it is bold in Hindi as well. If there' +
' are bullet points, double quotes, etc., make sure that the' +
' translated content also has bullet points and double quotes.',
'If the original card has “components” (such as pictures,' +
' links, and equations), these need to be added to the' +
' translated content. You can use the “Copy tool” for this' +
' -- click on the Copy tool and then click on the component you want' +
' to carry over. Also, double-click on the image and translate the ' +
' alt text (and caption, if any).'
],
bn:
[
'Use simple Bangla words that are used in daily communication.' +
' Note that common English words (pencil, etc.) can be written as' +
' transliterations (e.g পেন্সিল ).',
'Use proper punctuation.',
'Full stop = |',
'Use the same voice (active or passive)' +
' as in the original English text.',
'Preserve punctuation and bolding. If the original' +
' content has bold text, make sure it is bold in' +
' Bangla as well. If there are bullet points, double quotes,' +
' etc., make sure that the translated content also has bullet' +
' points and double quotes.'
],
ar:
[
'In Oppia, we prefer to use simple words that can be easily' +
' understood by children. For example, we use “تابع قائلًا”' +
' instead of “أردف قائلًا”. Furthermore, the English words that' +
' are used in the Arab society regularly can be translated as' +
' follows; Arabic word (The regularly used English word). For' +
' example, we can translate the word cupcakes this way;' +
' كعك القوالب الصغيرة (cupcakes). ',
'Use respectful ways and formal prefixes to address people.' +
' For example, use “سيدي” and “سيدتي”. ',
'If the name has a meaning in Arabic, or in English, such' +
' as Baker or Crumb, always use words that indicate that they' +
' are names before writing the name itself. For example,' +
' you can use one of the following words depending on the' +
' context; “السيد، السيدة، العم، الجد، الجدة، الآنسة.”',
'Use the same voice (active or passive) as in the' +
' original English Text',
'Preserve punctuation and bolding. If the original' +
' content has bold text, make sure it is bold in Arabic' +
' as well. If there are bullet points, double quotes, etc.,' +
' make sure that the translated content also has' +
' bullet points and double quotes.',
'Use the hyperlinks to different cards as shown in' +
' the original English Text.',
]
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -40,6 +40,13 @@
</schema-based-editor>
</div>
</div>
<div class="oppia-translation-tips-container">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After moving the ng-if="TRANSLATION_TIPS[activeLanguageCode]" to the <div> It shows nothing now. Posted a screenshot

@@ -40,6 +40,13 @@
</schema-based-editor>
</div>
</div>
<div class="oppia-translation-tips-container">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Catalan Example
Screen Shot 2021-04-01 at 9 32 50 AM

@@ -99,6 +106,12 @@
margin: 15px 0;
padding: 20px;
}
.oppia-translation-tips-container {
background-color: #ccc;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, New color posted below

@@ -99,6 +106,12 @@
margin: 15px 0;
padding: 20px;
}
.oppia-translation-tips-container {
background-color: #ccc;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2021-04-01 at 9 35 11 AM

@jimbyo jimbyo requested a review from sagangwee April 1, 2021 16:37
@jimbyo jimbyo assigned sagangwee and unassigned jimbyo Apr 1, 2021
@jimbyo jimbyo requested a review from nithusha21 as a code owner April 1, 2021 17:25
Copy link
Contributor

@sagangwee sagangwee left a comment

Choose a reason for hiding this comment

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

@jimbyo LGTM, thanks!

@oppiabot oppiabot bot unassigned sagangwee Apr 1, 2021
@oppiabot
Copy link

oppiabot bot commented Apr 1, 2021

Unassigning @sagangwee since they have already approved the PR.

@oppiabot
Copy link

oppiabot bot commented Apr 1, 2021

Assigning @nithusha21 for code owner reviews. Thanks!

@nithusha21 nithusha21 merged commit ecf60ce into oppia:develop Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Contributor Dashboard] Add inline translation tips to translation cards.
4 participants