fix: translation strings (DHIS2-8658) #590

turban · 2020-04-21T08:17:11Z

Fixes: https://jira.dhis2.org/browse/DHIS2-8658

The PR wraps non-translatable strings in i18n.t(). It is also a larger refactor to avoid using variables directly in i18n.t(), as these strings are not identified when the localisation files are generated.

This regex can be used to identify translation strings in Visual Studio Code which are not properly formatted: i18n.t\([^'"]

martinkrulltott

LGTM. Minor comments, happy to approve if they are addressed. Massive change, I've only looked through the diff itself, not the complete files or tested locally (would take ages, but happy to do it if necessary).

martinkrulltott · 2020-04-22T07:41:32Z

i18n/en.pot

-msgstr ""
-
-msgid "is saved"
+msgid "Map \"{{name}}\" is saved."


Hm, I was considering if the wrapping quotes should be in the name variable instead, to keep the translation string cleaner, but I think this is better anyway since it gives the translator the indication that name will indeed be within quotes.

martinkrulltott · 2020-04-22T07:52:06Z

src/components/orgunits/SelectedOrgUnits.js


-const modes = {
+const getModes = () => ({
    SELECTED: i18n.t('in'),


This translation string doesnt give any context which makes it impossible to translate properly. Could you either a) pass a the prop to getModes(myProp) and pass that on to the translation string? Or b) use an empty dummy prop in the translation string just to give context, like i18n.t('{myProp} in', {myProp: ''}). Same would go for the other 2 modes, even though the string is longer the context is missing.

ping @janhenrikoverland and @amcgee for input

I agree these are hard to translate without the context, I've changed to:

const getModeString = props => ({ SELECTED: i18n.t('{{units}} in {{orgunits}}', props), CHILDREN: i18n.t('{{units}} in and right below {{orgunits}}', props), DESCENDANTS: i18n.t('{{units}} in and all below {{orgunits}}', props), });

Great success! Tiny note, I would prefer const getModeString = (units, orgunits) => ({ to avoid having to pre-construct the props object like so:

maps-app/src/components/orgunits/SelectedOrgUnits.js

Line 44 in 4bdf785

selected = getModeString({

. But I guess this is still readable enough 👍

martinkrulltott · 2020-04-22T07:56:56Z

src/constants/periods.js

+import i18n from '@dhis2/d2-i18n';
+
+export const getPeriodTypes = () => [
+    { id: 'relativePeriods', name: i18n.t('Relative') },


why is relativePeriods camelCase when everything else is PascalCase? 🙂

Not changing now as it's not part of the PR, and changing it might break other parts :-)

martinkrulltott · 2020-04-22T07:58:37Z

src/constants/periods.js

-    { id: 'LAST_FINANCIAL_YEAR', name: 'Last financial year' },
-    { id: 'LAST_5_FINANCIAL_YEARS', name: 'Last 5 financial years' },
+export const getRelativePeriods = () => [
+    { id: 'TODAY', name: i18n.t('Today') },


And here everything is SCREAMING_SNAKE 🐍 😄

These are constants used in the Web API.

ah ok, just curious why they were all different

martinkrulltott · 2020-04-22T08:02:53Z

src/epics/favorites.js

-                              'is saved'
-                          )}.`
-                      ),
+                      setMessage(i18n.t('Map "{{name}}" is saved.', { name })),


This message is used twice, keep it as a const instead? Like const getSavedMessage = name => i18n.t('Map "{{name}}" is saved.', { name }) and setMessage(getSavedMessage(name)). Will reward 100 DRY points 😃

martinkrulltott · 2020-04-22T08:08:01Z

src/reducers/layers.js

@@ -101,22 +102,24 @@ const defaultLayers = [
    },
 ];

-const layers = (state = defaultLayers, action) => {
+const layers = (state, action) => {
+    const prevState = state || defaultLayers();


Would state = state || defaultLayers() / if(!state){state = defaultLayers()} work or is that an anti-pattern?

I think to reassign a function parameter is an anti-pattern. The problem assigning defaultLayers() in the function definition is that it's evaluated before the translation configuration is ready.

amcgee

@turban @martinkrulltott so the approach here, if I understand it correctly, is to use functions to fetch translations because when the module loads it hasn't yet selected the user's locale. Is that correct?

I think a much simpler approach (which we use in the platform) is to dynamically load the majority of the application using react.lazy AFTER initializing the i18n singleton. This way, those modules can use i18n strings even as consts, and they will have the correct locale. I think it makes it much cleaner than needing to think about when the i18n.t function is called everywhere it's used, and would make this change much smaller.

This also has the added benefit of a much faster initial page load, which can then show a nice loading spinner while the main app component is fetched.

If you want I can help look into that alternative approach, otherwise this approach should work as well.

amcgee · 2020-04-22T10:33:41Z

(Note: the above comment isn't about using string literals instead of variables, that's very much still important :-) )

turban · 2020-04-22T11:38:21Z

I feel more confident if we avoid switching to lazy loading for released versions of the Maps app, so I've added this issue to remember to remove the function wrappers after we've switched to the app plattform(#410): https://jira.dhis2.org/browse/DHIS2-8689

# 1.0.0 (2020-12-02) ### Bug Fixes * **translations:** sync translations from transifex (master) ([351ec3c](351ec3c)) * **translations:** sync translations from transifex (master) ([805277c](805277c)) * **translations:** sync translations from transifex (master) ([28ec1c7](28ec1c7)) * **translations:** sync translations from transifex (master) ([d581dae](d581dae)) * **translations:** sync translations from transifex (master) ([8e6d182](8e6d182)) * **translations:** sync translations from transifex (master) ([5bf64d2](5bf64d2)) * **translations:** sync translations from transifex (master) ([1ef8af4](1ef8af4)) * **translations:** sync translations from transifex (master) ([8ff7983](8ff7983)) * **translations:** sync translations from transifex (master) ([6481261](6481261)) * **translations:** sync translations from transifex (master) ([4ad0501](4ad0501)) * **translations:** sync translations from transifex (master) ([7c2ce65](7c2ce65)) * **translations:** sync translations from transifex (master) ([da60d2b](da60d2b)) * **translations:** sync translations from transifex (master) ([7858354](7858354)) * **translations:** sync translations from transifex (master) ([0ca2d82](0ca2d82)) * **translations:** sync translations from transifex (master) ([d96fa9f](d96fa9f)) * **translations:** sync translations from transifex (master) ([5dd74d3](5dd74d3)) * **translations:** sync translations from transifex (master) ([2b4d5f2](2b4d5f2)) * **translations:** sync translations from transifex (master) ([f095534](f095534)) * **translations:** sync translations from transifex (master) ([efd94da](efd94da)) * add missing translations (DHIS2-8658) ([#697](#697)) ([1563ae0](1563ae0)) * authentication header ([#434](#434)) ([11b646d](11b646d)) * avoid app crash if missing period id ([#1054](#1054)) ([b6d0b26](b6d0b26)) * backport changes ([#477](#477)) ([9a74886](9a74886)), closes [#474](#474) [#476](#476) * backport changes from master ([#475](#475)) ([916f574](916f574)), closes [#474](#474) * Bing Maps download and more compact select lists ([#474](#474)) ([4d4ac0c](4d4ac0c)) * build ([48358f2](48358f2)) * build ([c06b383](c06b383)) * catch error for invalid org unit selection for facility layer (DHIS2-8474) ([#495](#495)) ([bcc95d6](bcc95d6)) * change fullscreen control container and period state handling (DHIS2-8524) ([#522](#522)) ([4a71c8f](4a71c8f)) * clean ou dimension before app switching ([#582](#582)) ([2782cc6](2782cc6)) * clear conflicting settings when changing TEI type ([#171](#171)) ([4ed657b](4ed657b)) * clear data and legend when data loading fails ([#564](#564)) ([9b437ac](9b437ac)) * clear dimension items array when filter is changed ([#263](#263)) ([5d6ac53](5d6ac53)) * context menu positioning ([#457](#457)) ([24c904e](24c904e)) * context menu styles ([c0693c4](c0693c4)) * correctly look up lowercase option code ([#137](#137)) ([8e542ca](8e542ca)) * dashboard maps ([#441](#441)) ([83b1dc6](83b1dc6)) * delete serverCluster option if previously set ([#528](#528)) ([62334c1](62334c1)) * event layer styling and no data alert ([#121](#121)) ([a15cd4f](a15cd4f)) * format coordinate values in event popups ([#178](#178)) ([8fb9b96](8fb9b96)) * Handle error if org unit selection is invalid for thematic layers (DHIS2-8479) ([#496](#496)) ([8ec724e](8ec724e)) * keep dynamic filters when switching from DV ([#565](#565)) ([9ff76ac](9ff76ac)) * latest maps-gl ([bad25cb](bad25cb)) * latest maps-gl ([#516](#516)) ([3068de1](3068de1)) * layer dialog column max width ([85d9363](85d9363)) * layer name translations and d2-ui upgrades ([#657](#657)) ([dab62d4](dab62d4)) * loading indicator for new layers ([#181](#181)) ([3d3b07e](3d3b07e)) * map context menu positioning and refactoring ([#447](#447)) ([5baa1bb](5baa1bb)) * map resize on panel change ([#438](#438)) ([96e8511](96e8511)) * map state ([#386](#386)) ([4feae28](4feae28)) * maps gl upgrade and increased node memory for travis ([#452](#452)) ([9c7058b](9c7058b)) * maps-gl latest ([d7d6b9c](d7d6b9c)) * maps-gl latest ([be035df](be035df)) * maps-gl latest ([d1c7185](d1c7185)) * maps-gl latest ([6110828](6110828)) * maps-gl latest ([ebb4c5a](ebb4c5a)) * maps-gl latest and map container class name ([#456](#456)) ([05cc2b2](05cc2b2)) * maps-gl upgrade ([28058bd](28058bd)) * new temperature layer from google earth engine ([#140](#140)) ([6ca1cd9](6ca1cd9)) * no data color and split view layer ordering ([76b8a4a](76b8a4a)) * node version ([0bc1334](0bc1334)) * only set default org unit level if no other org units are selected ([#585](#585)) ([aefe5d9](aefe5d9)) * only show org units with data ([#584](#584)) ([13c2d5f](13c2d5f)) * plugin rendering class names ([#755](#755)) ([0bf154a](0bf154a)) * prop types checks ([#136](#136)) ([41e2512](41e2512)) * re-export cypress fixtures ([#157](#157)) ([66d0825](66d0825)) * refactor popup handling ([#182](#182)) ([eb5bcd1](eb5bcd1)) * remove and onReady event ([#454](#454)) ([b227c7c](b227c7c)) * remove doble scrollbars for dimension select ([#499](#499)) ([5c4b2d8](5c4b2d8)) * remove unsupported filter operators for event layers (DHIS2-7089) ([#586](#586)) ([b29059c](b29059c)) * rename period names import ([#630](#630)) ([99126a7](99126a7)) * resize handling for map plugins ([#168](#168)) ([adf1a9f](adf1a9f)) * revert maps-gl upgrade ([6156596](6156596)) * rxjs import ([5442944](5442944)) * rxjs import bug ([9ff52ac](9ff52ac)) * set alerts to undefined ([#440](#440)) ([d7ab7e6](d7ab7e6)) * set default legend set from data item ([#494](#494)) ([ef2946b](ef2946b)) * set single map rendering strategy as default (DHIS2-9543) ([#1060](#1060)) ([e407411](e407411)) * set the default classification/legend for selected data item (DHIS2-8485) ([#498](#498)) ([d030be5](d030be5)) * set width and height for line legend items in ff ([#125](#125)) ([33a3860](33a3860)) * set width of fullscreen container ([#601](#601)) ([e887617](e887617)) * show an alert when a load error occurs ([#134](#134)) ([edb3f30](edb3f30)) * show feature count in brackets for thematic layers ([#265](#265)) ([d2f2b70](d2f2b70)) * start/end dates check ([#1132](#1132)) ([5fa5e81](5fa5e81)) * **translations:** sync translations from transifex (master) ([b0f4f8f](b0f4f8f)) * round legend numbers ([#1119](#1119)) ([c9ed1f0](c9ed1f0)) * **translations:** sync translations from transifex (master) ([a7fd5b7](a7fd5b7)) * d2-ui-interpretations upgrade ([#1105](#1105)) ([8fa73af](8fa73af)) * **translations:** sync translations from transifex (master) ([7741043](7741043)) * **translations:** sync translations from transifex (master) ([d67469f](d67469f)) * **translations:** sync translations from transifex (master) ([6c8fac5](6c8fac5)) * **translations:** sync translations from transifex (master) ([42a1f56](42a1f56)) * ee api upgrade ([#1067](#1067)) ([d467dd4](d467dd4)) * maps-gl upgrade with style fix ([#1075](#1075)) ([7cb2506](7cb2506)) * only remove date field if undefined ([#1080](#1080)) ([9b98e10](9b98e10)) * only remove date field if undefined ([#1082](#1082)) ([54d5bf3](54d5bf3)) * proptype change ([#1061](#1061)) ([d5c5359](d5c5359)) * start/end dates ([#1053](#1053)) ([18b6add](18b6add)) * style and default legend ([#476](#476)) ([6ccde2b](6ccde2b)) * switch from google maps to bing maps ([#450](#450)) ([bad559d](bad559d)) * tei checkbox label ([#1074](#1074)) ([39485b4](39485b4)) * translation strings (DHIS2-8658) ([#590](#590)) ([a28b9a1](a28b9a1)) * unique class name prefix for each map plugin item (DHIS2-9593) ([#1083](#1083)) ([761e7a5](761e7a5)) * **translations:** sync translations from transifex (master) ([06f2ff8](06f2ff8)) * **translations:** sync translations from transifex (master) ([f6085a7](f6085a7)) * **translations:** sync translations from transifex (master) ([67f5848](67f5848)) * trigger map redraw when data table is dragged (DHIS2-9522) ([#1056](#1056)) ([ab4fce5](ab4fce5)) * **translations:** sync translations from transifex (master) ([be9be9f](be9be9f)) * additional checks for data existence ([#1046](#1046)) ([6e9aa0a](6e9aa0a)) * **translations:** sync translations from transifex (master) ([8347703](8347703)) * d2-ui dependency upgrades ([#1034](#1034)) ([dd93ca3](dd93ca3)) * scroll zoom handling for split view maps on dashboard ([#1032](#1032)) ([ec7e21e](ec7e21e)) * **translations:** sync translations from transifex (master) ([4baeea5](4baeea5)) * map plugin style handling ([#995](#995)) ([162a788](162a788)) * TEI visibility toggle ([#998](#998)) ([32cab9b](32cab9b)) * **translations:** sync translations from transifex (master) ([8d2a48c](8d2a48c)) * **translations:** sync translations from transifex (master) ([49b3b35](49b3b35)) * allow uid to idenitfy org unit levels (TECH-427) ([#984](#984)) ([8a4ac43](8a4ac43)) * require legend set selection (DHIS2-9314) ([#985](#985)) ([03c01a8](03c01a8)) * **translations:** sync translations from transifex (master) ([80219fd](80219fd)) * **translations:** sync translations from transifex (master) ([7864318](7864318)) * **translations:** sync translations from transifex (master) ([3e8aa2c](3e8aa2c)) * **translations:** sync translations from transifex (master) ([2e00d48](2e00d48)) * **translations:** sync translations from transifex (master) ([63e3ffb](63e3ffb)) * **translations:** sync translations from transifex (master) ([0b7e092](0b7e092)) * **translations:** sync translations from transifex (master) ([72ed6ca](72ed6ca)) * add missing translation strings ([#786](#786)) ([8ac176b](8ac176b)) * keep created and lastUpdated map properties ([#868](#868)) ([277970e](277970e)) * missing translation ([fbf920c](fbf920c)) * **translations:** sync translations from transifex (master) ([81c0a1b](81c0a1b)) * **translations:** sync translations from transifex (master) ([ae6587a](ae6587a)) * **translations:** sync translations from transifex (master) ([f7dd46d](f7dd46d)) * **translations:** sync translations from transifex (master) ([0a373e5](0a373e5)) * **translations:** sync translations from transifex (master) ([d51f984](d51f984)) * **translations:** sync translations from transifex (master) ([4ac2f63](4ac2f63)) * **translations:** sync translations from transifex (master) ([4d9d524](4d9d524)) * **translations:** sync translations from transifex (master) ([d8dccd9](d8dccd9)) * **translations:** sync translations from transifex (master) ([84437fd](84437fd)) * **translations:** sync translations from transifex (master) ([e9c7aa8](e9c7aa8)) * **translations:** sync translations from transifex (master) ([82b75f4](82b75f4)) * **translations:** sync translations from transifex (master) ([d5b6308](d5b6308)) * **translations:** sync translations from transifex (master) ([d0227ad](d0227ad)) * **translations:** sync translations from transifex (master) ([cea51a3](cea51a3)) * **translations:** sync translations from transifex (master) ([3fe207d](3fe207d)) * **translations:** sync translations from transifex (master) ([c0e0d17](c0e0d17)) * localize ([6683049](6683049)) * **translations:** sync translations from transifex (master) ([e6b6ec9](e6b6ec9)) * persist publicAccess on map save ([#765](#765)) ([c4c8e71](c4c8e71)) * **translations:** sync translations from transifex (master) ([31c03f3](31c03f3)) * update headerbar to latest and greatest ([#405](#405)) ([7c8353f](7c8353f)) * upgrade gis-api to 32.0.21 ([#124](#124)) ([2bdeeb5](2bdeeb5)) * upgrade interpretations components DHIS2-7249 ([#163](#163)) ([e79b333](e79b333)) * upgrade to v34 of backend api ([#446](#446)) ([e022516](e022516)) * upgrade to version 32 of the web api ([#129](#129)) ([0b44b02](0b44b02)) * use first period as default ([#583](#583)) ([7447e33](7447e33)) * use JssProvider for maps plugin ([#451](#451)) ([b83d1e2](b83d1e2)) * version bump ([bf4b52c](bf4b52c)) * version bump ([6020fd4](6020fd4)) * version bump ([5278708](5278708)) * version bump ([a388d4c](a388d4c)) * version bump ([01ebc23](01ebc23)) * version bump ([99419d5](99419d5)) * version bump ([f500d9d](f500d9d)) * version bump ([e07a4e9](e07a4e9)) * version bump ([7f00b6a](7f00b6a)) * **externallayers:** handle network error correctly ([#102](#102)) ([74ceb68](74ceb68)) ### Features * allow all program statuses to be selected for TEI layers (DHIS2-8166) ([#894](#894)) ([c162d59](c162d59)) * bubble map support for thematic layers (DHIS2-8572) ([#805](#805)) ([e5354f2](e5354f2)) * data table for event layer (DHIS2-3210) ([#862](#862)) ([f353f52](f353f52)) * dynamic dimension filtering for thematic layers ([#86](#86)) ([057c7f0](057c7f0)) * event polygons ([#114](#114)) ([39b79d6](39b79d6)) * fullscreen map control ([#411](#411)) ([62bcb8e](62bcb8e)) * hideTitle option ([#571](#571)) ([01e5179](01e5179)) * improved period selection for google earth engine layers ([#139](#139)) ([bbcb30d](bbcb30d)) * map loading spinner ([#177](#177)) ([cbdca45](cbdca45)) * mapping interface locale ([#412](#412)) ([742fb7c](742fb7c)) * maps gl download ([#432](#432)) ([57cb9fb](57cb9fb)) * new event status select for event layers ([#893](#893)) ([cb51d70](cb51d70)) * new relative periods ([#978](#978)) ([57358f9](57358f9)) * no data color for thematic layers (DHIS2-8642) ([#935](#935)) ([61d185f](61d185f)) * split view maps ([#154](#154)) ([2a81b0d](2a81b0d)) * support for bing maps and donut clusters ([#404](#404)) ([9ab5ad6](9ab5ad6)) * support saving and loading of TEI relationship maps ([#130](#130)) ([38b9581](38b9581)) * switch to maps gl ([#439](#439)) ([fa9cf81](fa9cf81)) * tei relationships ([#117](#117)) ([c8d22d3](c8d22d3)) * timeline support for thematic layers ([#155](#155)) ([6d46dba](6d46dba)) * Use default relative period defined in system settings (DHIS2-8841) ([#706](#706)) ([222c397](222c397)) * use new favicon, cleanup static file handling ([#120](#120)) ([378dafa](378dafa))

dhis2-bot · 2020-12-02T12:56:54Z

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

turban added 3 commits April 16, 2020 21:13

fix: i18n strings for display periods

9f6e70a

fix: i18n strings for user org units

7c89e6e

fix: i18n strings

4268e42

turban marked this pull request as draft April 21, 2020 08:17

turban added the WIP label Apr 21, 2020

turban changed the title * WIP * Translation strings fix: translation strings (DHIS2-8658) Apr 21, 2020

turban added 4 commits April 21, 2020 11:40

fix: translation strings

4ad94d5

fix: redux layers translations

0852e1f

chore: localize

c80647b

fix: earth engine translation strings

4d12b5a

turban removed the WIP label Apr 21, 2020

turban requested a review from jenniferarnesen April 21, 2020 11:09

turban marked this pull request as ready for review April 21, 2020 11:25

martinkrulltott approved these changes Apr 22, 2020

View reviewed changes

turban added 2 commits April 22, 2020 11:27

chore: saved map message

f389a9c

chore: refactor org units selection message

4bdf785

amcgee reviewed Apr 22, 2020

View reviewed changes

amcgee mentioned this pull request Apr 22, 2020

fix: set d2-i18n to peer dependency dhis2/d2-ui#591

Merged

chore: localize

7994719

turban merged commit a28b9a1 into master Apr 23, 2020

turban deleted the fix/DHIS2-8658 branch April 23, 2020 09:17

This was referenced Apr 29, 2020

fix: make all strings translatable 2.33 (DHIS2-8658) #629

Merged

fix: rename period names import #630

Merged

fix: make all strings translatable 2.32 (DHIS2-8658) #639

Merged

dhis2-bot added the released label Dec 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: translation strings (DHIS2-8658) #590

fix: translation strings (DHIS2-8658) #590

turban commented Apr 21, 2020 •

edited

martinkrulltott left a comment

martinkrulltott Apr 22, 2020

martinkrulltott Apr 22, 2020

martinkrulltott Apr 22, 2020

turban Apr 22, 2020

martinkrulltott Apr 22, 2020

martinkrulltott Apr 22, 2020

turban Apr 22, 2020

martinkrulltott Apr 22, 2020

turban Apr 22, 2020

martinkrulltott Apr 22, 2020

martinkrulltott Apr 22, 2020

turban Apr 22, 2020

martinkrulltott Apr 22, 2020

turban Apr 22, 2020

amcgee left a comment

amcgee commented Apr 22, 2020

turban commented Apr 22, 2020 •

edited

dhis2-bot commented Dec 2, 2020

fix: translation strings (DHIS2-8658) #590

fix: translation strings (DHIS2-8658) #590

Conversation

turban commented Apr 21, 2020 • edited

martinkrulltott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcgee left a comment

Choose a reason for hiding this comment

amcgee commented Apr 22, 2020

turban commented Apr 22, 2020 • edited

dhis2-bot commented Dec 2, 2020

turban commented Apr 21, 2020 •

edited

turban commented Apr 22, 2020 •

edited