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: fall back to overall language [DHIS2-17696] #33

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

tomzemp
Copy link
Member

@tomzemp tomzemp commented Jul 5, 2024

See https://dhis2.atlassian.net/jira/software/c/projects/DHIS2/issues/DHIS2-17696

Changes in PR:

  • updates the translation wrapping so that the translations will fall back to the variant language if no specific translations (e.g. if you select Portuguese Brazilian, the translations should fall back to Portuguese if no pt_BR translation is available)
  • handles the direction inference (which is based on i18n information) by parsing from Java format (e.g. ar_EG) to the ISO format expected by i18next (e.g. ar-EG)

before
image
(no translations for Egyptian Arabic and translations do not fall back to Arabic; direction is not set to RTL)

after
image
(translations fall back to Arabic; direction is set to RTL)

Technical notes:

  • the login app was already passing the user's chosen locale from the language selector to the i18n.t functions (with lng option). (This was done because calling i18n.changeLanguage would require a page refresh to load translations, which we decided against during the initial implementation). Since i18n.t also supports passing lngs, I've updated the translation functions to pass two languages (selected code and (if applicable) language e.g. ["ar_EG", "ar"] or ["fr"])

Testing

Manual: I've tested locally (looked at all pages in French) and visually checked that overall languages fallback to the overall language. You can see this on the main page looking at the differences between Portuguese Brazilian and Portuguese (slight difference in username, and Portuguese Brazilian did not have its own translation for "Create an account"

Automated: We don't generally test if strings are wrapped with translation logic, so I haven't done that here (it's an interesting question if it's possible 🧐). I have updated an existing test that touches translations in the "Powered by DHIS2" function and have updated tests for the relevant hooks to make sure fallback language gets set appropriately on language change. Also tests for the insignificant helper parser functions I added.

@tomzemp tomzemp requested a review from a team July 5, 2024 09:18
@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2024

Codecov Report

Attention: Patch coverage is 73.33333% with 12 lines in your changes missing coverage. Please review.

Project coverage is 68.90%. Comparing base (496107a) to head (af895fa).

Files Patch % Lines
src/components/account-creation-form.js 20.00% 4 Missing ⚠️
src/pages/complete-registration.js 33.33% 2 Missing ⚠️
src/pages/create-account.js 0.00% 2 Missing ⚠️
src/components/oidc-login-options.js 0.00% 1 Missing ⚠️
src/pages/password-reset-request.js 80.00% 1 Missing ⚠️
src/pages/password-update.js 83.33% 1 Missing ⚠️
src/pages/safe-mode.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   68.75%   68.90%   +0.15%     
==========================================
  Files          33       34       +1     
  Lines         560      566       +6     
  Branches      187      192       +5     
==========================================
+ Hits          385      390       +5     
- Misses        161      162       +1     
  Partials       14       14              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@d-rita d-rita left a comment

Choose a reason for hiding this comment

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

Tested with Arabic and Portuguese, and it works as expected! 🎉

@tomzemp tomzemp merged commit 401a3ec into main Jul 15, 2024
8 checks passed
dhis2-bot added a commit that referenced this pull request Jul 15, 2024
## [100.1.1](v100.1.0...v100.1.1) (2024-07-15)

### Bug Fixes

* fall back to overall language [DHIS2-17696] ([#33](#33)) ([401a3ec](401a3ec))
@dhis2-bot
Copy link
Collaborator

🎉 This PR is included in version 100.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants