Skip to content

Translation handling (#142)#143

Merged
benoit74 merged 1 commit intoopenzim:mainfrom
adiprathapa:feat/enhance-translation-handling
Mar 30, 2026
Merged

Translation handling (#142)#143
benoit74 merged 1 commit intoopenzim:mainfrom
adiprathapa:feat/enhance-translation-handling

Conversation

@adiprathapa
Copy link
Copy Markdown
Contributor

@adiprathapa adiprathapa commented Mar 24, 2026

Closes #142

  • Added a YAML file defining which locale keys the UI uses and their expected placeholders
  • Added scrape time validation that checks locale files against the YAML and fails the build if anything seems off
  • Added a CI check that scans UI code for translation keys not listed in the YAML
  • Centralized all static translation lookups into a t() function in the main store

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 9.64912% with 103 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.33%. Comparing base (874fa90) to head (67fdcc8).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
scraper/src/fcc2zim/locale_validation.py 13.69% 63 Missing ⚠️
scraper/src/fcc2zim/check_ui_keys.py 0.00% 35 Missing ⚠️
scraper/src/fcc2zim/prebuild.py 16.66% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #143      +/-   ##
==========================================
- Coverage   40.77%   33.33%   -7.44%     
==========================================
  Files           9       11       +2     
  Lines         363      477     +114     
  Branches       53       92      +39     
==========================================
+ Hits          148      159      +11     
- Misses        210      313     +103     
  Partials        5        5              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adiprathapa adiprathapa force-pushed the feat/enhance-translation-handling branch from 544d735 to 01a4f8c Compare March 24, 2026 12:42
@adiprathapa
Copy link
Copy Markdown
Contributor Author

Updated the PR with the remaining parts: CI check for UI translation keys and a centralized t() in the store so all static lookups go through one place

@adiprathapa adiprathapa force-pushed the feat/enhance-translation-handling branch from 01a4f8c to 3f89d54 Compare March 24, 2026 12:57
@adiprathapa
Copy link
Copy Markdown
Contributor Author

lk still needs a bit more work, will comment again when it looks good

@adiprathapa adiprathapa force-pushed the feat/enhance-translation-handling branch 3 times, most recently from 421ad12 to e4038e4 Compare March 24, 2026 13:12
@adiprathapa
Copy link
Copy Markdown
Contributor Author

Tested locally with a Spanish locale build. Screen below to show:

  • All static translation keys rendering correctly in Spanish
  • Placeholder interpolation working in the reset dialog

CI status:

  • All GitHub Actions checks pass
  • CodeFactor flags validate_locales as a complex method but the function validates three different things so the nesting is expected

Should be good for review.

Screen.Recording.2026-03-24.at.1.08.59.PM.mov

Copy link
Copy Markdown
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

It is important to note that check_ui_keys.py does not ensure placeholders are matching what we expect, which is fine to me because it is too hard to implement / too risky to break every now and then.

It is also important to note that locale_validation.py does check placeholders for non-dynamic keys, and does check dynamic keys are all present. This is good.

I feel like we should

  • extend locale_validation.py to check that dynamic keys in addition to being present are not using any placeholders (since we do not expect them to)
  • also use main.t(...) for dynamic keys (or is there a blocker for that?) so that we have a consistent code in the whole app
  • make localesIntro and others of the like "private" (prefixed with _) in main store so that it is clearer we should avoid to use them (or really private if you have some idea of how to do that in a simple way)

WDYT?

@adiprathapa
Copy link
Copy Markdown
Contributor Author

Makes sense, all three seem doable. I'll add the placeholder check for dynamic keys to locale_validation.py since the regex logic is alr there, and extend t() to handle dynamic paths. For the private state, I'll prefix the locale properties with _ since Pinia doesn't have true private state support.

One question on the motivation data though, compliments.random() and motivationalQuotes.random() are array based, not keyed string lookups, so they don't really fit the t() pattern. Should I add separate getter methods for those or leave them as is?

@benoit74
Copy link
Copy Markdown
Collaborator

One question on the motivation data though, compliments.random() and motivationalQuotes.random() are array based, not keyed string lookups, so they don't really fit the t() pattern. Should I add separate getter methods for those or leave them as is?

What about passing 'compliment' or 'motivationalQuote' to t() and perform the random selection in t()? Not that clean, but at least it allows to centralize a bit more translations handling. WDYT?

@adiprathapa
Copy link
Copy Markdown
Contributor Author

For dynamic intro keys I went with a separate tIntro() instead of routing them through t() since the intro lookups return string | string[] since the paragraphs come as arrays, so they don't really fit t()'s string return type

Copy link
Copy Markdown
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

LGTM, please squash your commits and I will merge. Well done.

Re TS7056 hack: OMG ... nice

@adiprathapa adiprathapa force-pushed the feat/enhance-translation-handling branch from aa9173e to 67fdcc8 Compare March 27, 2026 11:43
@adiprathapa
Copy link
Copy Markdown
Contributor Author

good to go

Copy link
Copy Markdown
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

Thank you!

@benoit74 benoit74 merged commit a2eae2f into openzim:main Mar 30, 2026
9 of 10 checks passed
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.

Enhance handling of translations / strings from freecodecamp

2 participants