Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add internationalization (i18n) support with react-i18next #58

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ztjhz
Copy link

@ztjhz ztjhz commented Dec 12, 2024

This PR only lays out the i18n foundation (only some text have been translated to English).

Translations are organized in JSON files located at public/locales/{{lang}}/translation.json, where {{lang}} represents the language code (e.g., en, ja, es).

To support a new language:

  • Create a new directory: Inside public/locales, create a folder named with the language code of the new language. For example, for Japanese, create public/locales/ja.
  • Add the translation file: Inside the newly created language directory, add a translation.json file. This file will contain the key-value pairs for the translated strings.

Usage Examples

Inside Components:

import { useTranslation } from "react-i18next";

const Component = () => {
  const { t } = useTranslation();

  return <h1>{t('sampleKey')}</h1>;
};

Outside Components:

import i18n from './i18n'; // Assuming your i18n setup is in i18n.ts or i18n.js

const translatedText = i18n.t('sampleKey');

@ayaka14732 ayaka14732 linked an issue Dec 12, 2024 that may be closed by this pull request
Copy link
Member

@graphemecluster graphemecluster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I would prefer a library that sticks on ICU MessageFormat since the Intl.MessageFormat proposal will eventually be a part of the ES spec. What do @syimyuzya @untunt @ayaka14732 think?

Additionally, before we can move on, we will need to decide on whether:

  • to remove the English translations temporarily and merge anyway; or
  • to merge into an alternative branch, nk2028:i18n

since English users will see messages in two languages at the same time as not all messages are translated.


i18n
.use(Backend)
// detect user language
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this line since it’s self-explanatory

escapeValue: false, // not needed for react as it escapes by default
},
backend: {
loadPath: "/tshet-uinh-autoderiver/locales/{{lng}}/translation.json",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to follow assetLocation in vite.config.ts since we also have vite.cos.config.ts for production

@@ -3,6 +3,8 @@ import { createRoot } from "react-dom/client";

import App from "./Components/App";

import "./i18n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary, i18n.ts would have already been imported in App.ts

Comment on lines +575 to +580
document.documentElement.lang = i18n.language;
i18n.on("languageChanged", lng => {
document.documentElement.lang = lng;
});

document.title = t("tshetUinhAutoderiver");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be put into i18n.ts since nothing is related to the App component.

Suggested change
document.documentElement.lang = i18n.language;
i18n.on("languageChanged", lng => {
document.documentElement.lang = lng;
});
document.title = t("tshetUinhAutoderiver");
document.documentElement.lang = i18n.language;
document.title = i18n.t("tshetUinhAutoderiver");
i18n.on("languageChanged", lng => {
document.documentElement.lang = lng;
document.title = i18n.t("tshetUinhAutoderiver");
});

Comment on lines 198 to +199
}, []);
const onHideTooltip = useCallback(() => setCopyTooltipText("複製到剪貼簿"), []);
const onHideTooltip = useCallback(() => setCopyTooltipText(t("copyToClipboard")), []);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t will be a dependency in these cases

@@ -204,13 +205,16 @@ export const evaluateOption: Record<Option, Handler> = {
return result.length ? (
<>
<Title>
找到 {result.length} 個相異項目。
{i18n.t("schemaCompareDifferent", { count: result.length })}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tricky, we will need a functional component like

function Translation({ message, ...options }) {
  const { t } = useTranslation();
  return t(message, options);
}

to get the messages sync with the language chosen (I am new to the i18next library and I haven’t think of how to type the component correctly yet, but I believe this should be what react-i18next provide. I wonder if there are alternative i18n libraries that provide such components.)

"copyToClipboard": "複製到剪貼簿",
"copySuccess": "成功複製到剪貼簿",
"copyFailed": "無法複製到剪貼簿",
"tshetUinhAutoderiver": "切韻音系自動推導器"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be at the top of the file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Members and I will need to decide on how the messages should be organised into groups

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@syimyuzya @untunt @ayaka14732 Perhaps let’s have a meeting soon?

@@ -11,8 +11,9 @@ import Table from "./Components/Table";
import TooltipChar from "./Components/TooltipChar";
import { noop } from "./consts";

import i18n from "./i18n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be at least one empty line between import groups - eslint(import/order)
This line should be in the previous group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multilingual UI
2 participants