-
Notifications
You must be signed in to change notification settings - Fork 377
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
✨ LLM - Thai language added #9345
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Skipped Deployments
|
Mobile Bundle Checks
Desktop Bundle Checks✅ Previous issues have all been fixed. |
bd966cb
to
223eb40
Compare
223eb40
to
fd12fb4
Compare
export const useSupportedLocales = () => { | ||
const thaiFeature = useFeature("llmThai"); | ||
|
||
const supportedLocalesFiltered = supportedLocales.filter(locale => { |
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.
Don't we want to use a memo here?
fd12fb4
to
ede6db0
Compare
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 we can think a little more generic?
In fact, if we add another language in the future
it might be a bit overkill at the moment, but something along the lines of
export const useSupportedLocales = () => {
const thaiFeature = useFeature("llmThai");
const greekFeature = useFeature("llmGreek");
const arabicFeature = useFeature("llmArabic");
const lithuanianFeature = useFeature("llmLithuanian");
const features = {
th: thaiFeature,
el: greekFeature,
ar: arabicFeature,
lt: lithuanianFeature,
};
const supportedLocalesFiltered = useMemo(
() =>
supportedLocales.filter(locale => {
const feature = features[locale];
if (feature && !feature.enabled) {
return false;
}
return true;
}),
[thaiFeature?.enabled, greekFeature?.enabled, arabicFeature?.enabled, lithuanianFeature?.enabled],
);
return supportedLocalesFiltered;
};
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.
Not mandatory btw ;)
|
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.
LGTM
✨ LLM - Thai language added
✅ Checklist
npx changeset
was attached.📝 Description
Thai language added
❓ Context
🧐 Checklist for the PR Reviewers