-
Notifications
You must be signed in to change notification settings - Fork 75
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
Edit language list, remove dialect specificity #7134
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Review
This PR updates the list of languages supported by PFE's translation systems. It mostly matches the changes to the similar list of language on Zooniverse Translations (see zooniverse/pandora#848)
- Languages are now standardised so that, uh, (trying to see what's the most accurate way to phrase this) only the "main parent class" (e.g. English) is used instead of a specific "dialect child class" (e.g. British English), where possible.
- Exceptions continue to exist where it makes sense, e.g. there's no "main" Chinese language (zh), but there are the Chinese simplified (zh-cn) and Chinese traditional (zh-tw) dialects
Notable differences with Pandora PR 848:
- ❓ This PR doesn't remove en-gb and en-us.
Notable differences with Pandora in general (not necessarily restricted to the scope of these two PRs):
- PFE's list of languages has one label for
ts
("Xitsonga"), whereas Pandora has two labels ("Xitsonga" and "Tsonga") - PFE's list of languages has one label for
ve
("Tshivenḓa"), whereas Pandora has two labels ("Tshivenḓa" and "Venda") - This may not be relevant to this PR, and no changes to this PR are expected, but these differences are worth noting for a future follow up.
Status
Code changes look good, though I do have that question about keeping the en-gb and en-us dialects in PFE.
And as I commented in Panodra PR 848, I can't fully comment on the context of these language changes - they look fine enough to me given my knowledge of global languages.
{ value: 'en-gb', label: 'English (United Kingdom)' }, | ||
{ value: 'en-us', label: 'English (United States)' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Question: in Pandora PR 848, both 'en-gb' and 'en-us' are removed from the Zooniverse Translations website's list of "languages you can set translations for".
Is this an oversight, or are we maintaining 'en-gb' and 'en-us' on PFE for backwards compatibility? (See recent Slack message: 68 projects have en-gb and 217 projects have en-us)
If we ARE making a conscious decision to only have en-gb
and en-us
in PFE's languages list but not Pandora's languages list, we should add a comment here saying something like /* English dialects should exist only PFE's language list. Do not delete these entries from PFE nor add them to Pandora when syncing the language lists of both repos. */
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is most important to limit the language list in Pandora, so as to limit the language translations that can be created. For PFE, the list can be more inclusive, because to the best of my knowledge, it is only used to transform a language code into a language name.
In a feat of cowardice, I opt to keep en-gb and en-us in the list because there are multiple projects and workflows that have these language codes as their primary_language value. While these additional entries shouldn't be needed, there is little penalty for doing so.
Note: whether en-us and en-gb primary_language values should be allowed or cleaned up is a whole issue unto itself; out of scope for now.
I'm happy to add the comment if you feel strongly we should, but I feel it is not necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification, Cliff. 👍 I'm on board with the reasons you have.
I do very, very strongly suggest we add the comment about the PFE code's en-gb and en-us entries, because I know that in the future, a dev (e.g. me) is gonna go "I'll just copy-paste the languages.js file wholesale to/from pandora to sync the changes, it'll be fast & easy! What could go wrong? A-hyuck!", and an inline comment in the code will make it very obvious to a reviewer that they shouldn't be doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the example -- yes, I see the utility of the comment now. I'll get that in (here and on Pandora just for completeness) before merging early next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From an accessibility perspective, en-US
and en-GB
can be read and pronounced differently by screenreaders, so specifying a dialect can be useful for screenreader users, in cases where there are differences of spelling or pronunciation. I assume that's true for other dialects too eg. Mexican Spanish is pronounced very differently from European Spanish. One fairly common example, in English, is that VoiceOver/Safari with a UK voice will consistently mispronounce 'favorite' on Zooniverse project pages.
Follows zooniverse/pandora#848, moving toward a standard of using only two-character language codes (without dialect specification) in almost all cases.
Note: along with merge, a small amount of data cleanup is necessary for existing translations (i.e., fil-ph, nso-za, tn-za, xh-za, zu-za) to drop dialect codes and make compatible with newly revised language list.
Staging branch URL: https://pr-7134.pfe-preview.zooniverse.org
Required Manual Testing
Review Checklist
npm ci
and app works as expected?