Use next-translate for i18n#192
Conversation
|
@mmiszy is attempting to deploy a commit to the giscus Team on Vercel. A member of the Team first needs to authorize it. |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/giscus/giscus/6UL1WFagdcNGn411w3e5ghtFiVUD |
laymonage
left a comment
There was a problem hiding this comment.
Fantastic work, thank you very much! I left some comments. I've tried fixing some of them locally but I can't seem to push the changes to your fork.
I'll look into the |
|
Updated @laymonage |
There was a problem hiding this comment.
Hey @mmiszy, thanks for the update!
Unfortunately it seems to use internal APIs of next-translate, which is not very ideal.
I managed to fix the useEffect issue by moving it to the Widget component (in components) instead of the widget page (in pages). You can see it in this commit. You can see the i18n branch on this repository for more details.
However, I noticed that your approach loads the translation server-side, so there's no flash of English language on the initial render, which is great. But, it's achieved through more code & the use of internal APIs. I'm not sure which one is better... what's your opinion on this? 😄
|
Looking at the source of That's why I've decided to do a bit of more manual work, load proper locales on the server and avoid changing the URL of the widget. |
laymonage
left a comment
There was a problem hiding this comment.
Hey, there's only minor issues left. Normally I would just fix these for you, squash the commits, and merge the PR right now, but I cannot because I don't have the permission. Maybe you left the "Allow edits by maintainers" unchecked or maybe because it's from your organization's fork.
Could you please check whether you could enable the following?
If the option is available and not enabled, please enable it and let me know, and ignore the rest of this comment.
Otherwise, could you:
git pull upstream main
git rebase upstream/main -i
And squash the commits into a single commit, and then
git push origin i18n -f
(I need this so that the diff view on GitHub is up to date and doesn't contain my existing changes for the Supabase config.)
If it's too much hassle, then don't worry, I will just use GitHub's squash merge option.
a6d423a to
4307322
Compare
|
@laymonage done with the rebase. I can't enable edits by maintainers because it's my organisation's fork. In the future I'll create PRs from my private account :D |
laymonage
left a comment
There was a problem hiding this comment.
Whoops, sorry, looks like it failed to build. Could you fix this?
To make sure no other issues are present, maybe you can try building the project locally before pushing it to GitHub.
Add English and Polish translations. Add missing date formatter for Polish Add missing formatDate and use Prettier Cleanup Update lib/i18n.ts Co-authored-by: sage <laymonage@gmail.com> Fix setting language Update lib according to CR Fix other -> many Update client.ts Co-authored-by: sage <laymonage@gmail.com> Update components/Comment.tsx Co-authored-by: sage <laymonage@gmail.com>
|
Thank you very much @mmiszy! I will add proper support for i18n (include it in the configuration step, update the contributing guide, etc.) in a future PR. Welcome aboard! 🥳 |
Fixes #108
data-langattributeformatDateDistance