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

fix: i18n: Apply Trans component to publish library dialogue #6564

Merged
merged 5 commits into from May 10, 2023

Conversation

Contextualist
Copy link
Contributor

This PR uses the Trans component introduced in #6534 to handle publish library dialogue translation entries. These entries need JSX to handle the embedded links. This PR supersedes #4268.

@vercel
Copy link

vercel bot commented May 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
excalidraw ✅ Ready (Inspect) Visit Preview May 10, 2023 5:31am
excalidraw-package-example ✅ Ready (Inspect) Visit Preview May 10, 2023 5:31am
1 Ignored Deployment
Name Status Preview Updated (UTC)
docs ⬜️ Ignored (Inspect) May 10, 2023 5:31am

@ad1992 ad1992 changed the title refactor: i18n: Apply Trans component to publish library dialogue fix: i18n: Apply Trans component to publish library dialogue May 8, 2023
@dwelle
Copy link
Member

dwelle commented May 8, 2023

Thanks @Contextualist! Can we also do this for the TopErrorBoundary.tsx?

We should also update the existing translations, consolidating the old strings into the new one, and sync back to Crowdin after we merge to master, so that we don't throw away all those existing translations away. Does that make sense?

@Contextualist
Copy link
Contributor Author

Can we also do this for the TopErrorBoundary.tsx?

Sure, done in 7bef66f

We should also update the existing translations, consolidating the old strings into the new one, and sync back to Crowdin after we merge to master, so that we don't throw away all those existing translations away. Does that make sense?

I tried to do an automated merge in bba9c00. Let me know if this is what you want.

Comment on lines 365 to 379
"noteDescription": {
"pre": "",
"link": "مستودع المكتبة العامة",
"post": "ليستخدمها الآخرون في رسوماتهم."
},
"noteGuidelines": {
"pre": "يجب الموافقة على المكتبة يدويًا أولاً. يرجى قراءة ",
"link": "الإرشادات",
"post": ""
},
"noteLicense": {
"pre": "",
"link": "رخصة إم أي تي ",
"post": "وهو ما يعني باختصار أنه يمكن لأي شخص استخدامها دون قيود."
},
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking we should not migrate RTL languages as it's likely significant that they didn't translate pre/post strings. Running it through a translator, it looks weird though, so I think it's best if people re-translate the whole string.

So let's revert the migrations for all RTL languages: ar-SA, fa-IR, he-IL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, reverted in 10e2284

@dwelle
Copy link
Member

dwelle commented May 10, 2023

We're good to go, thanks @Contextualist! ❤️

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

Successfully merging this pull request may close these issues.

None yet

3 participants