feat: Localize keyboards index page 📡 #521
Conversation
_includes/locale/Locale.php
Outdated
| public static function localize($domain, $strings) { | ||
| foreach(Locale::CROWDIN_LOCALES as $l) { | ||
| if ($l == Locale::DEFAULT_LOCALE) { | ||
| // Skip English | ||
| continue; | ||
| } | ||
|
|
||
| bindtextdomain("$domain-$l", __DIR__); | ||
| } | ||
|
|
||
| $previousTextDomain = textdomain(NULL); | ||
| Locale::setTextDomain($domain); | ||
|
|
||
| $result = []; | ||
| foreach($strings as $s) { | ||
| $result[$s] = _($s); | ||
| } | ||
|
|
||
| // Restore textdomain | ||
| textdomain($previousTextDomain); | ||
| return $result; | ||
| } |
There was a problem hiding this comment.
Refactored from #500 to Locale.php.
This way, pages don't need to make all the calls to bindtextdomain() and setTextDomain()
| msgid "%skeyboards%s" | ||
| msgstr "%stescados%s" |
| msgid "Keyboard search%s" | ||
| msgstr "Recherche au clavier%s" | ||
|
|
keyboards/index.php
Outdated
| @@ -9,9 +9,35 @@ | |||
| use Keyman\Site\com\keyman\templates\Foot; | |||
| use Keyman\Site\com\keyman\Locale; | |||
|
|
|||
| // Of array of strings at top of file | |||
| // by msgid | |||
| $keyboardIndexStrings = Locale::localize('keyboards', [ | |||
There was a problem hiding this comment.
This all feels very fragile -- arrays across multiple files which have to match up. A lot of duplication of these strings -- 6 times:
- twice in keyboards-en.po
- once each in keyboards-es-ES.po, keyboards-fr-FR.po
- twice in keyboards/index.php -- here and where referenced.
This will need automatic validation to keep in sync or it's going to get very hard to maintain. (Why do we need the array defined here as well as in the keyboards-en.po file? Can't we just use the list in that file as our starting point?)
It may be better to use an identifier which is not the whole string? e.g. I see we have also 'keyboard search' 'keyman keyboard search' 'keyboard search%s', 'search', 'new search' ... lots of repetition here and risk of confusion!
There was a problem hiding this comment.
It might be sensible to establish a convention for a shorthand reference e.g. $_ rather than a long name like $keyboardIndexStrings
There was a problem hiding this comment.
Based on 🦆 -
- Replaced $keyboardIndexStrings array with
_()alias - Updated
Locale::localize()so it doesn't restore textdomain
keyboards/index.php
Outdated
|
|
||
| <div id='search-box'> | ||
| <form method='get' action='/keyboards' name='f'> | ||
| <label for="search-q">Keyboard search:</label><input id="search-q" type="text" placeholder="Enter language or keyboard" name="q" | ||
| <label for="search-q"><?= Locale::_s($keyboardIndexStrings['Keyboard search%s'], ":") ?></label><input id="search-q" type="text" placeholder="<?= $keyboardIndexStrings['Enter language or keyboard'] ?>" name="q" |
There was a problem hiding this comment.
Why have you split the : out from the string? Here and other places? Seems to make the strings much more complicated than they need to be
There was a problem hiding this comment.
ok, I've consolidated punctuation back into the strings in edd7430
Add indexes to parameters Co-authored-by: Marc Durdin <[email protected]>
* Replace array of indexes with `_()` alias
Re-implements #500 for #384
This updates Locale.php for localizing the keyboards/ index page
Changes
Locale.php
setTextDomain()to manage the localization files_s()wrapperkeyboards/index.php
Screenshots
Note: the "Search" button image asset contains English text.
French
http://keyman.com.localhost/keyboards/?lang=fr-FR
Spanish
http://keyman.com.localhost/keyboards/?lang=es-ES