feat: i18n for react #9

siljec · 2023-09-19T10:27:51Z

This PR adds the localisation files needed to handle i18n and adjusts the react script to add translated titles for the React Icons.

Important files to take a closer look at:

scripts/i18n.js
scripts/output/react.js

The i18n.js script generates the following .po and .mjs files:

However, we are only checking in the English .po files, because they are needed to make the Crowdin integration work. (To be fixed in this ticket).

The i18n.js script also generates the lingui.config.

To make i18n work, we do need the compiled translations (the .mjs files) in the React/Vue/Elements icons. Therefore, we need to make sure that we are extracting the messages and compiling the messages before we can run the scripts to generate the icons for the frameworks. That's why I've added lingui extract && lingui compile to the build:react, build:vue and build:elements scripts in package.json. (In theory we only need to do it once).

I've modified the react script to output files to handle i18n, e.g:

import React from 'react';
import { i18n } from '@lingui/core';
import { messages as nbMessages} from '../../src/raw/active-ads/locales/nb/messages.mjs';
import { messages as enMessages} from '../../src/raw/active-ads/locales/en/messages.mjs';
import { messages as fiMessages} from '../../src/raw/active-ads/locales/fi/messages.mjs';
import { activateI18n } from '../../src/utils/i18n';
activateI18n(enMessages, nbMessages, fiMessages);
const title = i18n.t({ message: `Sheet with image and headline with highlighted checkmark`, id: 'icon.title.active-ads', comment: 'Title for active ads icon' });
export const IconActiveAds24 = (attrs) => React.createElement('svg', { xmlns: 'http://www.w3.org/2000/svg', width: '24', height: '24', fill: 'none', viewBox: '0 0 24 24', dangerouslySetInnerHTML: { __html: `<title>${title}</title><path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="1.5" d="M18 12H6V6h5M6 17h12"></path><path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="1.5" d="M16 2H3.5A1.5 1.5 0 0 0 2 3.5v17A1.5 1.5 0 0 0 3.5 22h17a1.5 1.5 0 0 0 1.5-1.5V11"></path><path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="1.5" d="M16.5 5.5 18 7l4-4.5"></path>` }, ...attrs, });

The default-icon-descriptions.js is based on the Icon-description.yml file. I've used ChatGPT to generate this file, so I've informed Ruth Rostrup that they will need to look over the English translations at some point.

Tested that it works in another project:

siljec · 2023-09-19T10:28:54Z

scripts/build.js

@@ -66,7 +66,6 @@ files.forEach((filePath) => {
    const dataAsHTMLElement = getElement({ selector: 'div', htmlString: `<div>${rawData}</div>` })
    const { name } = getNameAndSize(filePath)
    const title = descriptions[pascalCase(name)]
-    if (title) dataAsHTMLElement.querySelector('svg').prepend(`<title>${title}</title>`)


Need to remove it here because we are adding it when building the react. (Will be handled for vue and elements in separate PRs)

With this fix the icons that are built in dist, the optimized ones, will loose title. I've seen that those icons were used in FINN directly. SO those will loose title. Can it be good to at least keep the english one? I know that those should not be used directly but in case they are, maybe we can keep english titles to be on the safe side?

I'll try to find examples of those usages

https://github.schibsted.io/finn/new-construction-frontend/blob/457066bdf756d2aaa189382741661649b8b95a88/packages/ad/src/components/keyInfo/index.jsx this looks like one case of those usages

Hmmm good point. We can wait to merge this until we also handle Vue and Elements... Or wdyt?
Because if I remove this change, then the React icons will have two titles.

package.json

AnnaRybkina · 2023-09-20T09:41:51Z

package.json

+    "i18n:clear": "node scripts/i18n-helpers/clear-locales.js",
+    "i18n:get-sorted-locales": "node scripts/i18n-helpers/sort-locales.js",


When is it time to run those scripts?

I've added some comments in the script files:

/** * Helper to remove locales folder * Test that localization files in raw folder are made correctly with the i18n script */

This one is nice to keep when we are making changes to i18n, and want to test the the locales folder will be made correctly.

/** * Helper script to clean up and get a sorted default icon descriptions */

This one is useful when we are adding new translations, and want to sort the long file that holds all default English translations. I also used chatGPT to generate this file, and chatgpt (or I 😛) did some mistakes (so there were some duplicates that I wanted to remove.

But, maybe I should remove it from the scripts in package.json. Because they wont be used that much. Wdyt?

Can we sort that long file automatically when we generate it? As I assume we always want to have it sorted

This long file is not automatically generated 😭 I had to go several rounds with chatGPT to generate it based on the Norwegian titles. So when we add a new icon, we need to add the default English translation to default-icon-description.js. Then we can run the script to sort the file.

AnnaRybkina

I approve this one but I'm still not clear on why we do have some scripts and in what cases I could use them. But it will be clear I think as we are starting to use them. SO let's merge and improve if it is needed.

I haven't tested the functionality, only scanned the code changes

# [1.1.0-next.1](v1.0.1-next.1...v1.1.0-next.1) (2023-09-26) ### Features * i18n for react ([#9](#9)) ([10e41bc](10e41bc))

# [1.1.0](v1.0.0...v1.1.0) (2023-10-19) ### Bug Fixes * add 9BA8BA to exchange with currentColor ([#17](#17)) ([bea742a](bea742a)) * add leading slash to dest in crowdin config ([#23](#23)) ([d1170b6](d1170b6)) * add more missing titles ([#35](#35)) ([91f2b78](91f2b78)) * element icons names ([#16](#16)) ([0d550c8](0d550c8)) * Fix bundling ([#20](#20)) ([6ea9874](6ea9874)) * make sure finnish translations aren't gitignored ([#40](#40)) ([5c49b2b](5c49b2b)) * pnpm lockfile ([6529014](6529014)) * set next as default prerelease branch ([#12](#12)) ([bb0be72](bb0be72)) * update crowdin.yaml with leading slash ([bd09d68](bd09d68)) * update Readme text ([156d4ed](156d4ed)) * update script with improved destination in crowdin ([#34](#34)) ([30f4c92](30f4c92)) * use proper language placeholder in translations ([#22](#22)) ([4425b66](4425b66)) ### Features * Generate individual icons ([#39](#39)) ([dd74394](dd74394)) * Handle i18n for elements ([#19](#19)) ([8a27850](8a27850)) * Handle i18n for vue ([#14](#14)) ([2cf4fdb](2cf4fdb)) * i18n for react ([#9](#9)) ([10e41bc](10e41bc)) ### Reverts * Revert "feat:Handle i18n for elements (#15)" (#18) ([6bfee40](6bfee40)), closes [#15](#15) [#18](#18)

siljec added 8 commits September 17, 2023 15:33

chore: add lingui to devDependencies

52675fb

feat: add i18n script and translate react icon descriptions

773aeaf

chore: update .gitignore

be005c6

fix: update scripts

4f9578e

chore: add auto generateed lingui config

0f6831b

chore: update scripts in package.json

f121ed8

fix: add english .po files to raw

c250aba

fix: clean up in some files

5e77d17

siljec commented Sep 19, 2023

View reviewed changes

siljec changed the title ~~Feat/i18n for react~~ feat: i18n for react Sep 19, 2023

siljec marked this pull request as ready for review September 19, 2023 10:33

AnnaRybkina reviewed Sep 20, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

AnnaRybkina reviewed Sep 20, 2023

View reviewed changes

chore: update script in package.json

3aaba8f

AnnaRybkina approved these changes Sep 22, 2023

View reviewed changes

siljec changed the title ~~feat: i18n for react~~ feat: i18n for react (DO NOT MERGE until we have a fix for vue and elements too) Sep 22, 2023

siljec changed the title ~~feat: i18n for react (DO NOT MERGE until we have a fix for vue and elements too)~~ feat: i18n for react Sep 26, 2023

siljec changed the base branch from alpha to next September 26, 2023 06:23

siljec merged commit 10e41bc into next Sep 26, 2023
4 checks passed

siljec deleted the feat/i18n-for-react branch September 26, 2023 06:24

github-actions bot pushed a commit that referenced this pull request Sep 26, 2023

chore(release): 1.1.0-next.1 [skip ci]

4a07ee0

# [1.1.0-next.1](v1.0.1-next.1...v1.1.0-next.1) (2023-09-26) ### Features * i18n for react ([#9](#9)) ([10e41bc](10e41bc))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: i18n for react #9

feat: i18n for react #9

siljec commented Sep 19, 2023 •

edited

Loading

siljec Sep 19, 2023

AnnaRybkina Sep 20, 2023

AnnaRybkina Sep 20, 2023

siljec Sep 21, 2023

AnnaRybkina Sep 20, 2023 •

edited

Loading

siljec Sep 21, 2023

siljec Sep 21, 2023

AnnaRybkina Sep 21, 2023

siljec Sep 22, 2023

AnnaRybkina left a comment

		"i18n:clear": "node scripts/i18n-helpers/clear-locales.js",
		"i18n:get-sorted-locales": "node scripts/i18n-helpers/sort-locales.js",

feat: i18n for react #9

feat: i18n for react #9

Conversation

siljec commented Sep 19, 2023 • edited Loading

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

AnnaRybkina Sep 20, 2023 • edited Loading

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

AnnaRybkina left a comment

Choose a reason for hiding this comment

siljec commented Sep 19, 2023 •

edited

Loading

AnnaRybkina Sep 20, 2023 •

edited

Loading