Conversation
User Test ResultsTest specification and instructions User tests are not required |
There was a problem hiding this comment.
Rename to .mjs and then we can treat it as a module by file ext.
_includes/2020/templates/Head.php
Outdated
| if (str_contains($jsFile, '/js/i18n/') || str_contains($jsFile, 'search.js')) { ?> | ||
| <script src='<?=$jsFile?>' type='module' ></script> | ||
| <?php } else { ?> | ||
| <script src='<?=$jsFile?>'></script> | ||
| <?php } ?> | ||
| <?php } | ||
| } ?> |
There was a problem hiding this comment.
I really don't like this kind of specialization -- it is hard to maintain because it is putting test for specific files into the framework code.
It'd be better to make that determination on the basis of the file extension being .mjs:
if (str_ends_with($jsFile, '.mjs')) { ?>
But overall, better to just use echo:
| if (str_contains($jsFile, '/js/i18n/') || str_contains($jsFile, 'search.js')) { ?> | |
| <script src='<?=$jsFile?>' type='module' ></script> | |
| <?php } else { ?> | |
| <script src='<?=$jsFile?>'></script> | |
| <?php } ?> | |
| <?php } | |
| } ?> | |
| $jsFileType = str_ends_with($jsFile, '.mjs') ? "type='module'" : ""; | |
| echo "<script src='$jsFile' $jsFileType></script> | |
| } | |
| ?> |
There was a problem hiding this comment.
got it. GH had an error trying to apply the suggestion, so I manually updated in e87862c
| { | ||
| "resultOne": { | ||
| "text": "resultado", | ||
| "crowdinContext": "1 keyboard result found" |
There was a problem hiding this comment.
I don't like the crowdinContext being repeated for each language -- this is a lot of busy noise.
Couldn't we just have an object map?
{
"resultOne": "resultado",
...
}| import translationEN from './en.json' with { type: 'json' }; | ||
| import translationES from './es.json' with { type: 'json' }; | ||
| import translationFR from './fr.json' with { type: 'json' }; |
There was a problem hiding this comment.
This is not scalable to many translations. We need to use dynamic imports, and see also my comment on en.json - the translations need to be scoped.
| @@ -0,0 +1,54 @@ | |||
| { | |||
There was a problem hiding this comment.
This won't scale, because it's just one json file, so strings are going to get all muddled together. We are going to need to split per .js file I think?
There was a problem hiding this comment.
We are going to need to split per .js file I think?
So the strings relating to search.mjs would go in a folder like this?
/cdn/dev/js/i18n/search/en.json, es.json, etc?
There was a problem hiding this comment.
Are we wanting to dynamically load strings in a pattern similar to PHP (Locale.php)
where strings = [domain][language][key].... ?
There was a problem hiding this comment.
So the strings relating to
search.mjswould go in a folder like this?
/cdn/dev/js/i18n/search/en.json, es.json, etc?
Yes
Are we wanting to dynamically load strings in a pattern similar to PHP (Locale.php)
where strings = [domain][language][key].... ?
Something like that yes, with dynamic imports
cdn/dev/js/i18n/i18n.mjs
Outdated
| static objNavigate(obj, path){ | ||
| var aPath = path.split('.'); | ||
| try { | ||
| return aPath.reduce((a, v) => a[v].text, obj); |
There was a problem hiding this comment.
This isn't going to work with a.b.c is it?
There was a problem hiding this comment.
good catch. Updated to fix the nesting so this now works
objNavigate({a: {b: {c: {text:123}}}}, "a.b.c") // returns 123| if (!I18n.translations[language].hasOwnProperty(key)) { | ||
| // key is missing for current language | ||
| console.warn(`key '${key}' missing in '${language}' translations`); | ||
| return ''; |
There was a problem hiding this comment.
Fallback to en string would be better because we know we have those.
Co-authored-by: Marc Durdin <marc@durdin.net>
pagination strings now handled by search.mjs
| </div> | ||
|
|
||
| <script src="<?= cdn('legacy-keyboard-search/search.js'); ?>"></script> No newline at end of file | ||
| <script src="<?= cdn('legacy-keyboard-search/search.mjs'); ?>" type="module"></script> No newline at end of file |
There was a problem hiding this comment.
Hmm, I don't think we should touch the legacy keyboard search should we? Just the current version.
3 participants
For #384
Most of the PHP keyboard search pages can be localized.
This follows on by allowing the JS keyboard search to also be localized.
Off-the-shelf, i18n-next looked really good, but I opted for something that didn't need a framework:
https://medium.com/@mihura.ian/translations-in-vanilla-javascript-c942c2095170
Some of the changes
The JSON files make it easy to
For example:
I had to add search.js and i18n.js as modules in order to import the JSON files.
Sample screenshots
Test-bot: skip