Closed rogup closed 6 days ago
No problems seen in the code. Just ran my eyes over the commits - can't see much to comment on on a quick pass as I don't have a deep understanding of the code being modified at the moment. Seems sensible just to thumbs-up quickly.
The final commit seems like it belongs as a sole commit on main - unrelated.
I note the CI checks seem to be failing though? This probably needs to be resolved before it can be deployed.
What? Why?
Related to issue #29
This PR adds localisation of strings, so that they are translated based on the URL 'lang' parameter.
Code-specific details (for Reviewer)
I used the react-i18next library to do translation of the UI strings in React components. See the setup in the
i18n.ts
file. This file actually fetches the dataset config.json itself (separately to configSlice.ts) and then transforms theui
vocab into a format that can be consumed by i18next.Perhaps it would have been more in line with the rest of the app to share the vocabs state in Redux that is used elsewhere (e.g. for displaying filter input labels), and create our own method to place translations into the UI. But I chose to use i18next since it's a widely-used, standard library and we get a lot of nice things with it for free (e.g. the useTranslation hook, context-dependent string interpolation such as translating plurals to different languages). And in the future, we'll probably want to split out UI translations that are common to all maps from individual dataset vocabs anyway.
Checklist
docs/
What should we test? (for QA)
?lang=
URL paramater to a language supported by the dataset (e.g. 'fr' with dataset-A)Deployment notes