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

Edit language list, remove dialect specificity #7134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lcjohnso
Copy link
Member

@lcjohnso lcjohnso commented Jun 26, 2024

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

  • Does the non-logged in home page render correctly?
  • Does the logged in home page render correctly?
  • Does the projects page render correctly?
  • Can you load project home pages?
  • Can you load the classification page?
  • Can you submit a classification?
  • Does talk load correctly?
  • Can you post a talk comment?

Review Checklist

  • Does it work in all major browsers: Firefox, Chrome, Edge, Safari?
  • Does it work on mobile?
  • Can you npm ci and app works as expected?
  • If the component is in coffeescript, is it converted to ES6? Is it free of eslint errors? Is the conversion its own commit?
  • Are the tests passing locally and on GitHub Actions?

@coveralls
Copy link

Coverage Status

coverage: 56.969%. remained the same
when pulling 1ea8af9 on simple-lang-list
into 5150a5e on master.

@lcjohnso lcjohnso marked this pull request as ready for review June 26, 2024 17:01
Copy link
Member

@shaunanoordin shaunanoordin left a 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.

Comment on lines 29 to 30
{ value: 'en-gb', label: 'English (United Kingdom)' },
{ value: 'en-us', label: 'English (United States)' },
Copy link
Member

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. */

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Contributor

@eatyourgreens eatyourgreens Jul 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants