Skip to content

Comments

chore: rework i18n for js to avoid async and duplication#668

Open
mcdurdin wants to merge 1 commit intofeat/search-i18nfrom
chore/search-i18n-structural-tweaks
Open

chore: rework i18n for js to avoid async and duplication#668
mcdurdin wants to merge 1 commit intofeat/search-i18nfrom
chore/search-i18n-structural-tweaks

Conversation

@mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Feb 24, 2026

Moves the responsibility for determining locale fallback server-side, so that we can avoid async pollution and have just a single responsible module for locale fallback.

Strings are injected into a <script> data element as JSON, reducing the number of network round trips.

@darcywong00 happy to discuss this further as required

Test-bot: skip

Moves the responsibility for determining locale fallback server-side, so
that we can avoid async pollution and have just a single responsible
module for locale fallback.

Strings are injected into a <script> data element as JSON, reducing the
number of network round trips.

Test-bot: skip
@keymanapp-test-bot
Copy link

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot added this to the A19S23 milestone Feb 24, 2026
@mcdurdin mcdurdin requested a review from darcywong00 February 24, 2026 09:11
@github-project-automation github-project-automation bot moved this to Todo in Keyman Feb 24, 2026
@github-actions github-actions bot added the chore label Feb 24, 2026
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

nice - this does make things cleaner!

$localization = '';
foreach($locales as $locale) {
if($localization != '') $localization .= ",\n";
$localization .= "{ \"locale\": \"$locale\", \"strings\": " . file_get_contents(__DIR__ . "/../../locale/strings/$domain/$locale.json") . "}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wanted to confirm the folder structure for all the localized strings will be:

PHP (existing)

.../locale/strings/keyboards/ (for the main keyboards/ page)

.../locale/strings/keyboards/details/
.../locale/strings/keyboards/install/
.../locale/strings/keyboards/share/

JS (so far)

.../locale/strings/keyboard-search/

So we don't want to have the JS strings somewhere in /locale/strings/keyboards/?

Copy link
Contributor

Choose a reason for hiding this comment

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

The JS paths in crowdin.yml would need to be updated.
I can do here or back on my base PR

  # JS files
  - source: /_includes/locale/strings/keyboard-search/en.json
    dest: /js/search/en.json
    translation: /_includes/locale/strings/keyboard-search/%locale%.json
    languages_mapping:
      locale:

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a lot of munging on the "embed json i18n strings" for loop.
Would it be cleaner to wrap it in a Util.php function?

Comment on lines +26 to +27
* @param {string} domain
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param {string} domain
*/
* @param {string} domain
* @return {boolean} Status if domain was successfully loaded
*/

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

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants