-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[WEB-2870]feat: language support #6215
base: preview
Are you sure you want to change the base?
Changes from 7 commits
607ca5a
254db2d
2dab6c0
f311264
194003d
07c5147
e1fede3
28c5e3f
8800b2f
23dc6f5
4d13b0b
c1fbe98
c17f43f
fa4af3c
00f5a5c
7002474
ed5ff98
fdcc233
fd5b829
7c4d548
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
export const ORGANIZATION_SIZE = [ | ||
"Just myself", | ||
"Just myself", // TODO: translate | ||
"2-10", | ||
"11-50", | ||
"51-200", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
build/* | ||
dist/* | ||
out/* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/** @type {import("eslint").Linter.Config} */ | ||
module.exports = { | ||
root: true, | ||
extends: ["@plane/eslint-config/library.js"], | ||
parser: "@typescript-eslint/parser", | ||
parserOptions: { | ||
project: true, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
.turbo | ||
out/ | ||
dist/ | ||
build/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"printWidth": 120, | ||
"tabWidth": 2, | ||
"trailingComma": "es5" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
{ | ||
"name": "@plane/i18n", | ||
"version": "0.24.1", | ||
"description": "I18n shared across multiple apps internally", | ||
"private": true, | ||
"main": "./src/index.ts", | ||
"types": "./src/index.ts", | ||
"scripts": { | ||
"lint": "eslint src --ext .ts,.tsx", | ||
"lint:errors": "eslint src --ext .ts,.tsx --quiet" | ||
}, | ||
"dependencies": { | ||
"@plane/utils": "*" | ||
}, | ||
"devDependencies": { | ||
"@plane/eslint-config": "*", | ||
"@types/node": "^22.5.4", | ||
"typescript": "^5.3.3" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import React, { createContext, useEffect } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { observer } from "mobx-react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { TranslationStore } from "./store"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Language, languages } from "../config"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Create the store instance | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const translationStore = new TranslationStore(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Create Context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const TranslationContext = createContext<TranslationStore>(translationStore); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const TranslationProvider = observer(({ children }: any) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Handle storage events for cross-tab synchronization | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const handleStorageChange = (event: StorageEvent) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (event.key === "userLanguage" && event.newValue) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const newLang = event.newValue as Language; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (languages.includes(newLang)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
translationStore.setLanguage(newLang); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
window.addEventListener("storage", handleStorageChange); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return () => window.removeEventListener("storage", handleStorageChange); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, []); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+14
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add initial language loading and improve error handling. The useEffect only handles cross-tab synchronization but doesn't load the initial language from storage. Also, invalid language values should be handled gracefully. useEffect(() => {
+ // Load initial language from storage
+ const storedLanguage = window.localStorage.getItem("userLanguage");
+ if (storedLanguage) {
+ try {
+ const initialLang = storedLanguage as Language;
+ if (languages.includes(initialLang)) {
+ translationStore.setLanguage(initialLang);
+ } else {
+ console.warn(`Invalid language value in storage: ${storedLanguage}`);
+ }
+ } catch (error) {
+ console.error("Error loading initial language:", error);
+ }
+ }
const handleStorageChange = (event: StorageEvent) => {
if (event.key === "userLanguage" && event.newValue) {
- const newLang = event.newValue as Language;
- if (languages.includes(newLang)) {
- translationStore.setLanguage(newLang);
+ try {
+ const newLang = event.newValue as Language;
+ if (languages.includes(newLang)) {
+ translationStore.setLanguage(newLang);
+ } else {
+ console.warn(`Invalid language value in storage event: ${event.newValue}`);
+ }
+ } catch (error) {
+ console.error("Error processing language change:", error);
}
}
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return <TranslationContext.Provider value={translationStore}>{children}</TranslationContext.Provider>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,38 @@ | ||||||||||||||||||||||||||
import { makeObservable, observable } from "mobx"; | ||||||||||||||||||||||||||
import { Language, fallbackLng, languages, translations } from "../config"; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export class TranslationStore { | ||||||||||||||||||||||||||
currentLocale: Language = fallbackLng; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
constructor() { | ||||||||||||||||||||||||||
makeObservable(this, { | ||||||||||||||||||||||||||
currentLocale: observable.ref, | ||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||
this.initializeLanguage(); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
get availableLanguages() { | ||||||||||||||||||||||||||
return languages; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
t(key: string) { | ||||||||||||||||||||||||||
return translations[this.currentLocale]?.[key] || translations[fallbackLng][key] || key; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+18
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling and logging for missing translations. The current implementation silently falls back to the key when translations are missing. Consider:
Example implementation: t(key: string) {
+ const translation = translations[this.currentLocale]?.[key] || translations[fallbackLng][key];
+ if (!translation && process.env.NODE_ENV === 'development') {
+ console.warn(`Missing translation for key: ${key} in locale: ${this.currentLocale}`);
+ }
- return translations[this.currentLocale]?.[key] || translations[fallbackLng][key] || key;
+ return translation || key;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
setLanguage(lng: Language) { | ||||||||||||||||||||||||||
localStorage.setItem("userLanguage", lng); | ||||||||||||||||||||||||||
this.currentLocale = lng; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+22
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for localStorage operations. Direct localStorage access could throw in various scenarios (quota exceeded, private browsing, etc.). Apply this fix: setLanguage(lng: Language) {
- localStorage.setItem("userLanguage", lng);
+ try {
+ localStorage.setItem("userLanguage", lng);
+ } catch (error) {
+ console.error('Failed to save language preference:', error);
+ }
this.currentLocale = lng;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
initializeLanguage() { | ||||||||||||||||||||||||||
if (typeof window === "undefined") return; | ||||||||||||||||||||||||||
const savedLocale = localStorage.getItem("userLanguage") as Language; | ||||||||||||||||||||||||||
if (savedLocale && languages.includes(savedLocale)) { | ||||||||||||||||||||||||||
this.setLanguage(savedLocale); | ||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||
const browserLang = navigator.language.split("-")[0] as Language; | ||||||||||||||||||||||||||
const newLocale = languages.includes(browserLang as Language) ? (browserLang as Language) : fallbackLng; | ||||||||||||||||||||||||||
this.setLanguage(newLocale); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+27
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider edge cases in language initialization. The language initialization logic could be more robust:
Suggested implementation: interface LanguageState {
isLoading: boolean;
error: Error | null;
}
initializeLanguage = async () => {
if (typeof window === "undefined") return;
try {
this.isLoading = true;
const savedLocale = localStorage.getItem("userLanguage") as Language;
if (savedLocale && languages.includes(savedLocale)) {
this.setLanguage(savedLocale);
return;
}
const [browserMainLang] = navigator.language.toLowerCase().split("-");
const newLocale = languages.includes(browserMainLang as Language)
? (browserMainLang as Language)
: fallbackLng;
this.setLanguage(newLocale);
} catch (error) {
console.error('Language initialization failed:', error);
this.setLanguage(fallbackLng);
} finally {
this.isLoading = false;
}
} |
||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import en from "../locales/en/translations.json"; | ||
import fr from "../locales/fr/translations.json"; | ||
import es from "../locales/es/translations.json"; | ||
import ja from "../locales/ja/translations.json"; | ||
|
||
export type Language = (typeof languages)[number]; | ||
export type Translations = { | ||
[key: string]: { | ||
[key: string]: string; | ||
}; | ||
}; | ||
sriramveeraghanta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export const fallbackLng = "en"; | ||
export const languages = ["en", "fr", "es", "ja"] as const; | ||
export const translations: Translations = { | ||
en, | ||
fr, | ||
es, | ||
ja, | ||
}; | ||
|
||
export const SUPPORTED_LANGUAGES = [ | ||
{ | ||
label: "English", | ||
value: "en", | ||
}, | ||
{ | ||
label: "French", | ||
value: "fr", | ||
}, | ||
{ | ||
label: "Spanish", | ||
value: "es", | ||
}, | ||
{ | ||
label: "Japanese", | ||
value: "ja", | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./use-translation"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { useContext } from "react"; | ||
import { TranslationContext } from "../components"; | ||
import { Language } from "../config"; | ||
|
||
export function useTranslation() { | ||
const store = useContext(TranslationContext); | ||
if (!store) { | ||
throw new Error("useTranslation must be used within a TranslationProvider"); | ||
} | ||
|
||
return { | ||
t: (key: string) => store.t(key), | ||
currentLocale: store.currentLocale, | ||
changeLanguage: (lng: Language) => store.setLanguage(lng), | ||
languages: store.availableLanguages, | ||
}; | ||
sriramveeraghanta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export * from "./config"; | ||
export * from "./components"; | ||
export * from "./hooks"; |
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.
🛠️ Refactor suggestion
Add proper TypeScript types for props.
The component uses
any
type for children props, which bypasses TypeScript's type checking benefits.📝 Committable suggestion
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.
@mathalav55 check this.
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.