-
Notifications
You must be signed in to change notification settings - Fork 20
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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
document.documentElement.lang = i18n.language; | ||
i18n.on("languageChanged", lng => { | ||
document.documentElement.lang = lng; | ||
}); | ||
|
||
document.title = t("tshetUinhAutoderiver"); |
There was a problem hiding this comment.
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.
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"); | |
}); |
}, []); | ||
const onHideTooltip = useCallback(() => setCopyTooltipText("複製到剪貼簿"), []); | ||
const onHideTooltip = useCallback(() => setCopyTooltipText(t("copyToClipboard")), []); |
There was a problem hiding this comment.
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 })} |
There was a problem hiding this comment.
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": "切韻音系自動推導器" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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
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:
Usage Examples
Inside Components:
Outside Components: