feat: Use associative array for strings on keyboard search page 🗺️ #604

darcywong00 · 2025-09-09T04:01:18Z

Replaces the PR chain #516 in addressing #384 of localizing the keyboard search

From the A19S11 sprint design meeting, we pivoted from using .po locale files to localizing strings in associative arrays.

This PR reuses earlier Locale.php work to go in that direction on the main keyboard search page.
For now, locale is passed with the query parameter lang= (e.g. lang=es or lang=fr).

This also adds a globe icon to the top right for selecting UI language (passing the lang parameter)

Limitation: Currently session is set in keyboards/session.php so the rest of the site is ignoring the lang query parameter

TODO's on follow-on PRs

keyboard search results (Details and Install pages) 🔗 feat: Localize keyboard details page 🗺️ #622
check that crowdin.com can sync with these .php files (previously was syncing .po files)

User Testing

Setup - Run the keyman.com and website-local-proxy sites locally with docker

TEST_ENGLISH - Verifies default strings

Navigate to http://keyman.com.localhost/keyboards
Verify English strings match the live site on https://keyman.com/keyboards

TEST_SPANISH - Verifies search page appear in Spanish

Navigate to http://keyman.com.localhost/keyboards
Hover on the globe key in the top right (next to the rest donate section) and select Spanish - Español
Verify the main keyboard search page displays in Spanish

TEST_FRENCH - Verifies search page appear in French

Navigate to http://keyman.com.localhost/keyboards?lang=fr-FR
Hover on the globe key in the top right (next to the rest donate section) and select French - Français
Verify keyboard search page is in French

keymanapp-test-bot · 2025-09-09T04:01:33Z

User Test Results

Test specification and instructions

✅ TEST_ENGLISH (PASSED) (notes)
✅ TEST_SPANISH (PASSED) (notes)
✅ TEST_FRENCH (PASSED) (notes)

mcdurdin

This is a nice clean start. But I am asking for a bunch of changes to make it more extensible long-term. Thanks!

_includes/locale/Locale.php

mcdurdin · 2025-09-09T05:15:56Z

_includes/locale/Locale.php

+    public const CROWDIN_LOCALES = array(
+      'en',
+      'es-ES',
+      'fr-FR'
+    );


I think this should be in a separate file, or perhaps determined on a per-domain basis, because we have multiple l10n domains (i.e. keyboards is one localization domain). See my comments re filenames and folder names further down

with the glob refactor below, this can go away

mcdurdin · 2025-09-09T05:16:36Z

_includes/locale/Locale.php

+    public static function validateLocale($locale) {
+      return in_array($locale, Locale::CROWDIN_LOCALES);
+    }


This may not be viable because we may not have localizations for a given l10n domain.

Can you give an example?
We can set the list of available languages on
https://crowdin.com/project/keymancom/settings#languages

(would need the sil_ltops login to add custom languages)

removed with the glob refactor.

mcdurdin · 2025-09-09T05:17:24Z

_includes/locale/Locale.php

+    private static $langArrayEn = [];
+
+    /**
+     * Return the current locale. Fallback to 'en'


Fallback should really be through a path, e.g. given es-ES, we should fallback first to es, then to en.

So should setLocale() split on dashes for determining fallback.
e.g. with es-ES we end up with

self::$currentLocales = ['es', 'es-ES', 'en'];

There are algorithms for fallback already. See https://cldr.unicode.org/development/development-process/design-proposals/language-distance-data

That might be too much for us. @srl295 may have suggestions on simpler approaches.

Moved to calculateFallbackLocales() where we can always reimplement in a TODO

mcdurdin · 2025-09-09T05:19:44Z

_includes/locale/Locale.php

+      'es-ES',
+      'fr-FR'


These should be minimal BCP-47 tags - i.e. we would use es-419 for Latin America and Caribbean, es for Spain. Similar for French, should be just fr, not fr-FR (we would use fr-CA for example).

mcdurdin · 2025-09-09T05:28:26Z

_includes/locale/Locale.php

+     * @param $s - the format string
+     * @param $args - optional remaining args to the format string
+     */ 
+    public static function _s($s, ...$args) {


Ditto on no _ needed. I would have the one public function though (and make _m into a private getString()):

public static function m($domain, $id, ...args) { $str = self::getString($domain, $id); if(count($args) == 0) return $str; return vsprintf($str, $args); }

mcdurdin · 2025-09-09T05:38:03Z

_includes/locale/Locale.php

+     * Return the current locale. Fallback to 'en'
+     * @return $currentLocale
+     */
+    public static function currentLocale() {


This should return an array of available locales in priority order, e.g. for es-ES, we'd go ['es-ES', 'es', 'en']. This way, we can support fallback more neatly in the future, even if we initially only go with one fallback to en.

mcdurdin · 2025-09-09T05:49:11Z

keyboards/index.php

  $head_options = [
-    'title' =>'Keyboard Search',
-    'description' => 'Keyman Keyboard Search',
+    'title' =>Locale::_m('page_title'),


Suggested change

'title' =>Locale::_m('page_title'),

'title' => _m('page_title'),

mcdurdin · 2025-09-09T05:49:48Z

keyboards/index.php

  use Keyman\Site\com\keyman\templates\Foot;
+  use Keyman\Site\com\keyman\Locale;
+
+  Locale::localize('keyboards');


// To make the localization inserts cleaner and DRY, we can do this:

Suggested change

Locale::localize('keyboards');

function _m($id) { return Locale::m('keyboards', $id); }

Will it get messy if every page we localize has to add?

function_m($id) {...}

or should we just move it to Localize.php()?

mcdurdin · 2025-09-09T05:51:42Z

keyboards/session.php

+  if(isset($_REQUEST['lang'])) {
+    \Keyman\Site\com\keyman\Locale::overrideCurrentLocale($_REQUEST['lang']);
+  } else if (isset($_SESSION['lang'])) {
+    \Keyman\Site\com\keyman\Locale::overrideCurrentLocale($_SESSION['lang']);
+  }
+  $_SESSION['lang'] = \Keyman\Site\com\keyman\Locale::currentLocale();
+


It would be good to look at best practices around embedding locale in query strings vs path or domain etc. We need to consider our technology stack in making this decision.

Yep, I've copied this as a bullet point on the issue to investigate

From SO: "Does the locale belong on the path or as a request parameter on the URL?
https://stackoverflow.com/a/16471122

If you need it as request parameter or part of the url depends of what you want to achieve. If you want to serve static content, you should have it be part of the path. If you want to act dynamically on the chosen locale, you should use it as request parameter, since you don't want to have your scripts replicated several times over different paths just to add different locales.

ermshiperete · 2025-09-15T12:57:54Z

_includes/locale/Locale.php

+     * @param $id - the id for the string
+     * @param $args - optional remaining args to the format string
+     */ 
+    public static function m($domain, $id, ...$args) {


Why m?

I might overlook the obvious, but currently I have a hard time seeing a connection between m and getting a localized string. I could make sense of s, t, or l, but for m I'm missing the association (which then makes it harder to use the function if I can't remember it).

I'm open to any clear name. In the previous iteration #516, we were using Locale::_s() for formatted strings.
That version was also using _ gettext, but we're not setting locales this time.

darcywong00 · 2025-09-16T03:54:54Z

_includes/locale/strings/keyboards/en.php

+
+return [
+  # Page Title
+  "page_title" => "Keyboard Search",


One drawback of not using the .po format is we lose the in-line context comments on Crowdin.

The map file is quite cryptic

Crowdin allows us to use an accompanying Markdown file to provide some context, but it covers the entire file

darcywong00 · 2025-09-17T01:06:35Z

I think this is ready for another round of review comments.

This one is still unresolved.
#604 (comment)

It would be good to look at best practices around embedding locale in query strings vs path or domain etc. We need to consider our technology stack in making this decision.

For now, I'm just using ?lang=es or ?lang=fr for testing.

ermshiperete · 2025-09-17T09:40:06Z

This one is still unresolved. #604 (comment)

It would be good to look at best practices around embedding locale in query strings vs path or domain etc. We need to consider our technology stack in making this decision.

For now, I'm just using ?lang=es or ?lang=fr for testing.

Can we use the Accept-Language header for this? (Accept-Language, qa-lang-priorities)

darcywong00 · 2025-09-18T04:20:50Z

keyboards/index.php

-    'description' => 'Keyman Keyboard Search',
+    'title' => _m('page_title'),
+    'description' => _m('page_description'),
+    'language' => isset($_SESSION['lang']) ? $_SESSION['lang'] : 'en',


This will set the lang attribute for the entire page.

Unless we need to apply it separately to all the
tags involving localized strings below...

darcywong00 · 2025-09-18T07:03:19Z

Can we use the Accept-Language header for this?

Maybe as a starting point? We do eventually need a way for:

desktop users to toggle the UI language with a dropdown selector (next to "Support" in the top right banner)
Keyman mobile apps to pass their UI locale to the embedded keyboard search. (which could be different from the device's locale)

* Remove CROWDIN_LOCALES * Update setLocale to parse the locale for the fallback tags

Moved to calculateFallbackLocales() so we can replace as TODO

_includes/autoload.php

_includes/2020/templates/Menu.php

mcdurdin

Let's go with this -- it's reasonably generalized now and we can at least move forward with some localization!

mcdurdin · 2025-11-18T08:30:35Z

_includes/2020/templates/Menu.php

+    private static function render_globe_dropdown(): void {
+?>
+          <p>
+            <div id="ui-language" class="menu-item">  


This will result in two divs with the same id, which is not really right. We need to keep them unique.

Updated to pass a div number so we can assign "id" = ui-language or ui-language1
in f1f8a62

darcywong00 · 2025-12-01T04:19:02Z

Hmm. something to chase down on why the link checker is now 7x longer
Reference GHA
https://github.com/keymanapp/keyman.com/actions/runs/19664278223/job/56317324603?pr=624

PR	Links Found	Links Excluded	Elapsed Time
#750 (baseline)	318,228	315,763	21 minutes, 24 seconds
This PR	2,377,385	2,366,016	1 hour, 50 minutes

darcywong00 · 2025-12-02T07:04:48Z

And the link checker is back to 20 min with 4fe6b0c

Meng-Heng · 2025-12-03T04:39:40Z

Prerequisites

Fetch & Pull feat/localize/keyboards-map
Run keyman.com & website-local-proxy on Chrome

Test Results

TEST_ENGLISH (PASSED):

Open http://keyman.com.localhost:8053/keyboards/
Verified: The page content is in English & match the content on keyman.com/keyboards.

TEST_SPANISH (PASSED):

Return to Home page, in English
Open http://keyman.com.localhost:8053/keyboards/, the page content remains in English.
Hover the globe key, click on Español
Verified: The page content translates to Spanish.

TEST_FRENCH (PASSED):

Return to Home page, in Spanish
Open http://keyman.com.localhost:8053/keyboards/, the page content is still in Spanish
Hover the globe key, click on Français
Verified: The page content translates to French
Return to Home page, in French
Open http://keyman.com.localhost:8053/keyboards/, the page content remains in French
Hover on the globe key, click on English
Return to Home page, open http://keyman.com.localhost:8053/keyboards/?lang=fr-Fr
Verified: the page opens up in French.

darcywong00 added this to the A19S11 milestone Sep 9, 2025

darcywong00 requested review from ermshiperete and mcdurdin September 9, 2025 04:01

keyman-status added this to Keyman Sep 9, 2025

github-actions bot added the feat label Sep 9, 2025

github-project-automation bot moved this to Todo in Keyman Sep 9, 2025

keymanapp-test-bot bot added has-user-test user-test-required labels Sep 9, 2025

mcdurdin requested changes Sep 9, 2025

View reviewed changes

darcywong00 modified the milestones: A19S11, A19S12 Sep 13, 2025

ermshiperete reviewed Sep 15, 2025

View reviewed changes

darcywong00 force-pushed the feat/localize/keyboards-map branch from 739ee14 to 70f48e6 Compare September 16, 2025 01:55

darcywong00 commented Sep 16, 2025

View reviewed changes

darcywong00 marked this pull request as ready for review September 17, 2025 01:06

darcywong00 commented Sep 18, 2025

View reviewed changes

darcywong00 requested a review from mcdurdin September 19, 2025 00:55

darcywong00 added 8 commits September 19, 2025 08:06

feat: Use associate array for strings on keyboard search page

689d15b

wip: Start implementing review comments

755a2df

refactor load strings

db8aa79

Cleanup fallback of locale

c715fb8

* Remove CROWDIN_LOCALES * Update setLocale to parse the locale for the fallback tags

refactor callback locales

d446584

Moved to calculateFallbackLocales() so we can replace as TODO

chore: more cleanup

79dbb48

fix: variadric args

faa45cf

set lang tag and fallback to English

76c9d0a

darcywong00 force-pushed the feat/localize/keyboards-map branch from 6b929d9 to 76c9d0a Compare September 19, 2025 01:06

fix: Update globe to hover

ed1c38b

darcywong00 force-pushed the feat/localize/keyboards-map branch from e687164 to ed1c38b Compare October 27, 2025 03:39

darcywong00 marked this pull request as draft October 29, 2025 03:23

darcywong00 added 3 commits October 30, 2025 08:24

fix: Parse lang string in URI query

2fd6527

refactor render_globe_dropdown()

0bc40f5

chore: Add header to render_globe_dropdown

54a0de5

darcywong00 marked this pull request as ready for review October 31, 2025 02:42

die if current locales aren't set

c1638ca

keyman-server modified the milestones: A19S15, A19S16 Nov 8, 2025

darcywong00 commented Nov 10, 2025

View reviewed changes

_includes/autoload.php Outdated Show resolved Hide resolved

darcywong00 commented Nov 10, 2025

View reviewed changes

_includes/2020/templates/Menu.php Outdated Show resolved Hide resolved

chore: Address review comments

cacc840

darcywong00 mentioned this pull request Nov 11, 2025

feat: Localize keyboard details page 🗺️ #622

Merged

darcywong00 requested a review from ermshiperete November 18, 2025 00:28

mcdurdin approved these changes Nov 18, 2025

View reviewed changes

mcdurdin mentioned this pull request Nov 18, 2025

feat: Localize keyboards share and install pages 🗺️ #623

Merged

keyman-server modified the milestones: A19S16, A19S17 Nov 22, 2025

Merge branch 'master' into feat/localize/keyboards-map

75abde9

fix: Pass div number for render_globe_dropdown()

f1f8a62

darcywong00 changed the title ~~feat: Use associate array for strings on keyboard search page 🗺️~~ feat: Use associative array for strings on keyboard search page 🗺️ Dec 1, 2025

test: exclude link-checking locales 'lang=*'

4fe6b0c

keymanapp-test-bot bot removed the user-test-required label Dec 3, 2025

darcywong00 merged commit 0e1b284 into master Dec 3, 2025
5 checks passed

darcywong00 deleted the feat/localize/keyboards-map branch December 3, 2025 04:44

github-project-automation bot moved this from Todo to Done in Keyman Dec 3, 2025

	'title' =>Locale::_m('page_title'),
	'title' => _m('page_title'),

	Locale::localize('keyboards');
	function _m($id) { return Locale::m('keyboards', $id); }

Uh oh!

feat: Use associative array for strings on keyboard search page 🗺️ #604

feat: Use associative array for strings on keyboard search page 🗺️ #604

Uh oh!

Conversation

darcywong00 commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Testing

Uh oh!

keymanapp-test-bot bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Test Results

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darcywong00 Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ermshiperete Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darcywong00 commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ermshiperete commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darcywong00 commented Sep 18, 2025

Uh oh!

Uh oh!

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darcywong00 commented Sep 9, 2025 •

edited

Loading

keymanapp-test-bot bot commented Sep 9, 2025 •

edited

Loading

darcywong00 Sep 15, 2025 •

edited

Loading

ermshiperete Sep 15, 2025 •

edited

Loading

darcywong00 commented Sep 17, 2025 •

edited

Loading

ermshiperete commented Sep 17, 2025 •

edited

Loading

darcywong00 Dec 1, 2025 •

edited

Loading

Meng-Heng commented Dec 3, 2025 •

edited

Loading