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

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

add i18n support #6006

wants to merge 5 commits into from

Conversation

gaecoli
Copy link
Member

@gaecoli gaecoli commented Apr 26, 2023

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • New Query Runner (Data Source)
  • New Alert Destination
  • Other

Description

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

@gaecoli gaecoli changed the title add i18n support add i18n support #933 Apr 26, 2023
@gaecoli
Copy link
Member Author

gaecoli commented Apr 26, 2023

I add i18n module but i need help to translate English, Chinese and French so on. So i need help language json file.

@gaecoli
Copy link
Member Author

gaecoli commented Apr 26, 2023

If someone support language file, the language pack switching function on the UI should be added later, and the js-cookie package should be used to record the language pack used last time.

@gaecoli gaecoli changed the title add i18n support #933 add i18n support Apr 26, 2023
@gaecoli gaecoli mentioned this pull request Apr 26, 2023
@lucasfcnunes
Copy link
Member

I can translate to Portuguese :)
Is it gonna be possible to integrate to services like https://hosted.weblate.org/projects/nekox/#languages?

@gaecoli
Copy link
Member Author

gaecoli commented May 5, 2023

I can translate to Portuguese :) Is it gonna be possible to integrate to services like https://hosted.weblate.org/projects/nekox/#languages?

Yes

Copy link
Member

@gabrieldutra gabrieldutra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding it! Only thing I'd probably do is to keep it disabled until we have more internationalized strings not to have random translated words in the UI


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)

Comment on lines +61 to +62
"i18next": "^22.4.15",
"i18next-browser-languagedetector": "^7.0.1",
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.

client/app/index.js Outdated Show resolved Hide resolved
@gabrieldutra
Copy link
Member

Btw @gaecoli how are you doing the string extraction? Is it manually in this PR for now and still needs to be configured?

@gaecoli
Copy link
Member Author

gaecoli commented May 10, 2023

Btw @gaecoli how are you doing the string extraction? Is it manually in this PR for now and still needs to be configured?

There is no good idea for this, for me it can only be added manually.

@gabrieldutra
Copy link
Member

There is no good idea for this, for me it can only be added manually.

Sure, adding something like https://www.i18next.com/how-to/extracting-translations#2-using-an-extraction-tool to either automate the English strings extraction, or to have yarn i18n:extract and yarn i18n:check scripts (the later to be executed on CI)

I haven't used react-i18next myself, so I don't really know if that's required

@gaecoli
Copy link
Member Author

gaecoli commented Jun 7, 2023

@gabrieldutra Hello, I would like to ask, what should I do after I modify the source code, so that I can achieve the same code style as the original? Which front-end commands should I run?

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

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

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!

@myonlylonely
Copy link
Contributor

myonlylonely commented Aug 18, 2023

I could help translating to Chinese. How could I contribute?
Another open source project Sentry (https://github.com/getsentry/sentry) also written in Python & React uses transifex (https://explore.transifex.com/getsentry/sentry/) to translate.

@guidopetri
Copy link
Collaborator

@gaecoli hey, any interest in continuing this PR? I think we need to split zh.json into the simplified/traditional Chinese format, as well as disabling i18n by default for now until we have more translations in (which we should open a separate issue for, I think).

We also need to merge master in + resolve conflicts in the yarn lock file.

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

Successfully merging this pull request may close these issues.

None yet

6 participants