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

Issue 55 suggestions 3 #395

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

thehanimo
Copy link

Rebases changes from #252.
Fixes reCAPTCHA re-render on locale change.
Fixes suggestions modal tooltip errors

Related Issues: #55, #390

@thehanimo thehanimo mentioned this pull request Mar 24, 2021
@sushain97
Copy link
Member

The build failure indicates that you need to run make in assets/strings.

@thehanimo
Copy link
Author

thehanimo commented Mar 27, 2021

Thanks @sushain97! Btw, I had to change this:

cleanup: $(shell find . -regextype sed -regex ".*/[a-z]\{3\}\.json" -printf "%f ")

to:

cleanup: $(shell find . -regex ".*/[a-z]\{3\}\.json")

Don't think -regextype and -printf are supported on my machine (Darwin Kernel Version 20.3.0)

I haven't committed this change though.

@TinoDidriksen
Copy link
Member

That would be BSD find vs. GNU find. A portable solution is something like find . -name '*.json' | egrep '/[a-z]{3}\.json' but should be tested.

localizeReacaptchaAlternativeLanguages has a typo in both places it is used.

But asides from that the code looks ok to me.

@thehanimo
Copy link
Author

Nice catch!

var iso639Codes = {'abk': 'ab', 'aar': 'aa', 'afr': 'af', 'aka': 'ak', 'sqi': 'sq', 'amh': 'am', 'ara': 'ar', 'arg': 'an', 'hye': 'hy', 'asm': 'as', 'ava': 'av', 'ave': 'ae', 'aym': 'ay', 'aze': 'az', 'bam': 'bm', 'bak': 'ba', 'eus': 'eu', 'bel': 'be', 'ben': 'bn', 'bih': 'bh', 'bis': 'bi', 'bos': 'bs', 'bre': 'br', 'bul': 'bg', 'mya': 'my', 'cat': 'ca', 'cha': 'ch', 'che': 'ce', 'nya': 'ny', 'zho': 'zh', 'chv': 'cv', 'cor': 'kw', 'cos': 'co', 'cre': 'cr', 'hrv': 'hr', 'ces': 'cs', 'dan': 'da', 'div': 'dv', 'nld': 'nl', 'dzo': 'dz', 'eng': 'en', 'epo': 'eo', 'est': 'et', 'ewe': 'ee', 'fao': 'fo', 'fij': 'fj', 'fin': 'fi', 'fra': 'fr', 'ful': 'ff', 'glg': 'gl', 'kat': 'ka', 'deu': 'de', 'ell': 'el', 'grn': 'gn', 'guj': 'gu', 'hat': 'ht', 'hau': 'ha', 'heb': 'he', 'her': 'hz', 'hin': 'hi', 'hmo': 'ho', 'hun': 'hu', 'ina': 'ia', 'ind': 'id', 'ile': 'ie', 'gle': 'ga', 'ibo': 'ig', 'ipk': 'ik', 'ido': 'io', 'isl': 'is', 'ita': 'it', 'iku': 'iu', 'jpn': 'ja', 'jav': 'jv', 'kal': 'kl', 'kan': 'kn', 'kau': 'kr', 'kas': 'ks', 'kaz': 'kk', 'khm': 'km', 'kik': 'ki', 'kin': 'rw', 'kir': 'ky', 'kom': 'kv', 'kon': 'kg', 'kor': 'ko', 'kur': 'ku', 'kua': 'kj', 'lat': 'la', 'ltz': 'lb', 'lug': 'lg', 'lim': 'li', 'lin': 'ln', 'lao': 'lo', 'lit': 'lt', 'lub': 'lu', 'lav': 'lv', 'glv': 'gv', 'mkd': 'mk', 'mlg': 'mg', 'msa': 'ms', 'mal': 'ml', 'mlt': 'mt', 'mri': 'mi', 'mar': 'mr', 'mah': 'mh', 'mon': 'mn', 'nau': 'na', 'nav': 'nv', 'nob': 'nb', 'nde': 'nd', 'nep': 'ne', 'ndo': 'ng', 'nno': 'nn', 'nor': 'no', 'iii': 'ii', 'nbl': 'nr', 'oci': 'oc', 'oji': 'oj', 'chu': 'cu', 'orm': 'om', 'ori': 'or', 'oss': 'os', 'pan': 'pa', 'pli': 'pi', 'fas': 'fa', 'pol': 'pl', 'pus': 'ps', 'por': 'pt', 'que': 'qu', 'roh': 'rm', 'run': 'rn', 'ron': 'ro', 'rus': 'ru', 'san': 'sa', 'srd': 'sc', 'snd': 'sd', 'sme': 'se', 'smo': 'sm', 'sag': 'sg', 'srp': 'sr', 'gla': 'gd', 'sna': 'sn', 'sin': 'si', 'slk': 'sk', 'slv': 'sl', 'som': 'so', 'sot': 'st', 'azb': 'az', 'spa': 'es', 'sun': 'su', 'swa': 'sw', 'ssw': 'ss', 'swe': 'sv', 'tam': 'ta', 'tel': 'te', 'tgk': 'tg', 'tha': 'th', 'tir': 'ti', 'bod': 'bo', 'tuk': 'tk', 'tgl': 'tl', 'tsn': 'tn', 'ton': 'to', 'tur': 'tr', 'tso': 'ts', 'tat': 'tt', 'twi': 'tw', 'tah': 'ty', 'uig': 'ug', 'ukr': 'uk', 'urd': 'ur', 'uzb': 'uz', 'ven': 've', 'vie': 'vi', 'vol': 'vo', 'wln': 'wa', 'cym': 'cy', 'wol': 'wo', 'fry': 'fy', 'xho': 'xh', 'yid': 'yi', 'yor': 'yo', 'zha': 'za', 'zul': 'zu', 'hbs': 'sh', 'pes': 'fa'};
var localizeRecaptchaLanguages = ['ar', 'af', 'am', 'hy', 'az', 'eu', 'bn', 'bg', 'ca', 'zh-HK', 'zh-CN', 'zh-TW', 'hr', 'cs', 'da', 'nl', 'en-GB', 'en', 'et', 'fil', 'fi', 'fr', 'fr-CA', 'gl', 'ka', 'de', 'de-AT', 'de-CH', 'el', 'gu', 'iw', 'hi', 'hu', 'is', 'id', 'it', 'ja', 'kn', 'ko', 'lo', 'lv', 'lt', 'ms', 'ml', 'mr', 'mn', 'no', 'fa', 'pl', 'pt', 'pt-BR', 'pt-PT', 'ro', 'ru', 'sr', 'si', 'sk', 'sl', 'es', 'es-419', 'sw', 'sv', 'ta', 'te', 'th', 'tr', 'uk', 'ur', 'vi', 'zu'];
var localizeRecaptchaAlternativeLanguages = {'zh': 'zh-TW', 'hrv': 'hr', 'srp': 'sr', 'msa': 'ms', 'cy': 'en', 'ga': 'en', 'gv': 'en', 'gd': 'en', 'rn': 'en', 'lg': 'en', 'mt': 'en', 'sq': 'fr', 'br': 'fr', 'se': 'no', 'eo': 'en', 'fo': 'da', 'ia': 'en', 'xh': 'af', 'mfe': 'fr', 'nb': 'no', 'nn': 'no', 'uz': 'ru', 'rm': 'en', 'tg': 'ru', 'ba': 'ru', 'be': 'ru', 'os': 'ru', 'kum': 'ru', 'ky': 'ru', 'mk': 'en', 'tt': 'ru', 'kk': 'ru', 'he': 'en', 'ne': 'hi', 'as': 'hi', 'pa': 'ur', 'mrj': 'ru', 'myv': 'ru', 'oc': 'es', 'cv': 'ru', 'arg': 'es', 'ast': 'es', 'hbs': 'en', 'bos': 'en', 'nog': 'ru', 'sah': 'ru', 'uig': 'zh-TW', 'tyv': 'ru'};
Copy link
Member

Choose a reason for hiding this comment

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

Do these lists need to be updated manually? If so, where from, and how often?

Copy link
Author

Choose a reason for hiding this comment

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

localizeRecaptchaLanguages is the list of languages reCAPTCHA supports.
localizeRecaptchaAlternativeLanguages provides a fallback language in case the locale (current website language) is not supported by reCAPTCHA.

We would need to update either/both when we support new locales or when reCAPTCHA supports more/less languages

Copy link
Member

Choose a reason for hiding this comment

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

Could you put a comment to that effect here?

"Suggest_Title": "Improve Apertium's translation",
"Suggest_Button": "Suggest",
"Suggest_Sentence": "How would you suggest we translate {{targetWordCode}}?",
"Suggest_Placeholder": "New Word",
Copy link
Member

Choose a reason for hiding this comment

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

It won't always be a new word; in fact, often the problem could be something more having to do with grammar.

Copy link
Author

Choose a reason for hiding this comment

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

How about "Suggested word" or "Your suggestion" instead?

Copy link
Member

Choose a reason for hiding this comment

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

I like "Your suggestion"!

Copy link
Author

Choose a reason for hiding this comment

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

#252 had a Suggest_Placeholder value in Russian as well (Новое слово). Changed that to "Ваше предложение" based on Google Translate and IRC chat.

@thehanimo
Copy link
Author

thehanimo commented Apr 14, 2021

To highlight individual words and to make them clickable,

<textarea class="form-control bg-light" id="translatedText" rows="15" spellcheck="false"
aria-label="Translated Text" readonly></textarea>

was replaced with

<div class="form-control bg-light" id="translatedText" rows="15" spellcheck="false"
aria-label="Translated Text" readonly></div>

in #252

The only disadvantage I see with this is the missing resize-handle in the translated text container, which can be fixed by setting style to resize: vertical; overflow: auto;. But hovering over the handle still shows a text cursor (set for the div), rather than a resize one.

Apart from that, I'm not sure if this has far-reaching effects.

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.

None yet

4 participants