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.

Sign up for GitHub

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 i18n support #6006

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

add i18n support #6006

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5eeda15
add i18n support
gaecoli Apr 25, 2023
f913457
change code style
gaecoli Apr 26, 2023
34aecf8
Update i18n index.js
gaecoli May 30, 2023
99a815f
Merge branch 'getredash:master' into add-i18n
gaecoli May 30, 2023
5b7b188
add yarn.lock
gaecoli Jun 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions client/app/i18n/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import i18n from "i18next";
import LanguageDetector from "i18next-browser-languagedetector";
import { initReactI18next } from "react-i18next";
import en from "./locales/en.json";
import zh from "./locales/zh.json";
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, with this zh I've often seen it split into two. One for "Simplified Chinese", and one for "Traditional Chinese".

Should we have this named as one of them, to leave room for the other one at some future point?

Copy link
Member Author

Choose a reason for hiding this comment

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

Out of curiosity, with this zh I've often seen it split into two. One for "Simplified Chinese", and one for "Traditional Chinese".

Should we have this named as one of them, to leave room for the other one at some future point?

OK. You are right.

Copy link
Member

@gabrieldutra gabrieldutra Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

One note is that importing languages like this will add all of them into the bundle, which may not be nice 😅.

It's possibly better to do an async import for those cases. https://www.i18next.com/how-to/add-or-load-translations had a bit of content on this.

Separate thing I was thinking about these days: Should we plan ahead how we will do the translations end-to-end, before committing anything? I don't know much about react-i18next, I've used mostly react-intl, the main difference I saw so far is that the later also has a description for each unformatted string. Considering Redash as an open-source / community effort and how LLMs are today, it may be a good thing to plan a flow which includes automatic generation of the translation files on every release. Then would count on the community only for reviewing/fixing eventual mistakes, in this case the description would be useful for providing context and/or fixing the prompt.

Copy link
Member Author

Choose a reason for hiding this comment

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

One note is that importing languages like this will add all of them into the bundle, which may not be nice 😅.

It's possibly better to do an async import for those cases. https://www.i18next.com/how-to/add-or-load-translations had a bit of content on this.

Separate thing I was thinking about these days: Should we plan ahead how we will do the translations end-to-end, before committing anything? I don't know much about react-i18next, I've used mostly react-intl, the main difference I saw so far is that the later also has a description for each unformatted string. Considering Redash as an open-source / community effort and how LLMs are today, it may be a good thing to plan a flow which includes automatic generation of the translation files on every release. Then would count on the community only for reviewing/fixing eventual mistakes, in this case the description would be useful for providing context and/or fixing the prompt.

Cool. It's a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

One note is that importing languages like this will add all of them into the bundle, which may not be nice 😅.

It's possibly better to do an async import for those cases. https://www.i18next.com/how-to/add-or-load-translations had a bit of content on this.

Separate thing I was thinking about these days: Should we plan ahead how we will do the translations end-to-end, before committing anything? I don't know much about react-i18next, I've used mostly react-intl, the main difference I saw so far is that the later also has a description for each unformatted string. Considering Redash as an open-source / community effort and how LLMs are today, it may be a good thing to plan a flow which includes automatic generation of the translation files on every release. Then would count on the community only for reviewing/fixing eventual mistakes, in this case the description would be useful for providing context and/or fixing the prompt.

I haven't figured out which platform to host these mapping tables for the time being.

Copy link
Member

@justinclift justinclift Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As a data point - for a project many years ago - we had the project hosted on Launchpad, so it could make use of the translation infrastructure there.

One really cool benefit of that platform at the time was that if anyone, anywhere had translated the same string for any project on Launchpad (with compatible license) then the string was made available as a potential option.

It saved a lot of time for people, and made it easy to get pretty far with new language translations.

That being said, the company behind Launchpad (Canonical) have done some shady stuff in the past so I'm not really a fan of theirs... 😉

Copy link
Member

Choose a reason for hiding this comment

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

The possible use of LLM's for generating "first attempt" translations sounds like a useful idea too.

Copy link
Member Author

Choose a reason for hiding this comment

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

The possible use of LLM's for generating "first attempt" translations sounds like a useful idea too.

Cool!


i18n
.use(initReactI18next)
.use(LanguageDetector)
Copy link
Member

Choose a reason for hiding this comment

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

Does this automatically enables the translation based on the browser? I think it may be better to leave it disabled until we have more strings translated (or under some feature flag / global settings)

.init({
resources: {
en: { translation: en },
zh: { translation: zh },
},
fallbackLng: "zh",
preload: ["en", "zh"],
interpolation: {
escapeValue: false,
},
});

export default i18n;
3 changes: 3 additions & 0 deletions client/app/i18n/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"dashboard": "dashboard"
}
4 changes: 4 additions & 0 deletions client/app/i18n/locales/zh.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"dashboard": "看板",
"Favorite Dashboards": "收藏的看板"
}
1 change: 1 addition & 0 deletions client/app/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import "@/config";

import ApplicationArea from "@/components/ApplicationArea";
import offlineListener from "@/services/offline-listener";
import "./i18n/index";
gaecoli marked this conversation as resolved.
Show resolved Hide resolved

ReactDOM.render(<ApplicationArea />, document.getElementById("application-root"), () => {
offlineListener.init();
Expand Down
5 changes: 4 additions & 1 deletion client/app/pages/home/components/FavoritesList.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import LoadingOutlinedIcon from "@ant-design/icons/LoadingOutlined";

import { Dashboard } from "@/services/dashboard";
import { Query } from "@/services/query";
import { useTranslation } from "react-i18next";

export function FavoriteList({ title, resource, itemUrl, emptyState }) {
const [items, setItems] = useState([]);
Expand Down Expand Up @@ -53,13 +54,15 @@ FavoriteList.propTypes = {
FavoriteList.defaultProps = { emptyState: null };

export function DashboardAndQueryFavoritesList() {
const { t } = useTranslation();

return (
<div className="tile">
<div className="t-body tb-padding">
<div className="row home-favorites-list">
<div className="col-sm-6 m-t-20">
<FavoriteList
title="Favorite Dashboards"
title={t("Favorite Dashboards")}
resource={Dashboard}
itemUrl={dashboard => dashboard.url}
emptyState={
Expand Down
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
"font-awesome": "^4.7.0",
"history": "^4.10.1",
"hoist-non-react-statics": "^3.3.0",
"i18next": "^22.4.15",
"i18next-browser-languagedetector": "^7.0.1",
Comment on lines +61 to +62
Copy link
Member

Choose a reason for hiding this comment

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

From CI failure it seems it's missing the yarn.lock changes, so you may need to re-run yarn and push it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

From CI failure it seems it's missing the yarn.lock changes, so you may need to re-run yarn and push it :)

I pushed yarn.lock file.

"markdown": "0.5.0",
"material-design-iconic-font": "^2.2.0",
"moment": "^2.19.3",
Expand All @@ -71,6 +73,7 @@
"react-ace": "^9.1.1",
"react-dom": "^16.14.0",
"react-grid-layout": "^0.18.2",
"react-i18next": "^12.2.2",
"react-resizable": "^1.10.1",
"react-virtualized": "^9.21.2",
"sql-formatter": "git+https://github.com/getredash/sql-formatter.git",
Expand Down