-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feature/multilingual support #498
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
base: main
Are you sure you want to change the base?
Feature/multilingual support #498
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.
Thanks for the contribution!
So this is quite interesting, and I can see the value in it. That said, I am not sure if we should merge this. Main concerns:
- The readme in chinese -> we don't have the capacity to maintain it, so this we would certainly skip. Maybe at some point in the future.
- While internationalization is nice, it does make things more complex. Harder to see in the code what are the actual messages. Could we have some of them embeeded directly in the place of usage? For example the main language, let's say english, is always there? But yeah it makes development harder somewhat, and if somebody wants to get rid of this because they don't need it, that is also hard. How are other starter tempaltes out there going about this? How many come with localization? How many let you choose if you want it or not? If they come with it, how hard is it to get out of it?
- I wonder if it is indeed best practice to put it all in one place. One huge file for each language. I kind of get the appeal of that, but on the other hand I always valued organization of features by the "vertical". So maybe, this could be split into multiple files, or even embedded above/in the React components, when the values are very localized (used only there, in that React component). So then if I am changing that React component, I don't have to change that component + this file, but only the file in which that React component is.
- I haven't used this library -> how is it type-wise? What kind of type level safety do we have here? If I forget to define something in chinese, but I have it defined in english, do I get a type error?
Also a bit of overall story would help! How do you feel about the PR, do you usually do it like this, convince us a bit :D. Also, I am guessing you used AI to help with this? How did that go, how confident are you in the output, how much of it did you check manually?
@@ -9,6 +9,73 @@ import searchcraft from '../client/static/examples/searchcraft.webp'; | |||
import { BlogUrl, DocsUrl } from '../shared/common'; | |||
import type { GridFeature } from './components/FeaturesGrid'; | |||
|
|||
// 使用函数来获取翻译后的功能列表 | |||
export const getFeatures = (t: (key: string) => string): GridFeature[] => [ |
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.
Hm this was added, but what was deleted? Also, why chinese comment at the top?
@@ -18,6 +19,29 @@ interface PaymentPlanCard { | |||
features: string[]; | |||
} | |||
|
|||
// 使用函数来获取翻译后的计划卡片 |
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.
Same question as above: why chinese comment here, and if this was added here, what was removed? I guess this was moved from somewhere?
@@ -1,6 +1,32 @@ | |||
import { LayoutDashboard, Settings, Shield } from 'lucide-react'; | |||
import { routes } from 'wasp/client/router'; | |||
|
|||
// 使用函数来获取翻译后的用户菜单项 | |||
export const getUserMenuItems = (t: (key: string) => string) => [ |
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.
Why was this added + why chinese comment?
Description
Contributor Checklist