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

Add translation about stub in layout #3969

Merged
merged 6 commits into from
Aug 10, 2023
Merged

Conversation

imbant
Copy link
Contributor

@imbant imbant commented Jul 31, 2023

What kind of changes does this PR include?

  • Translated content
  • Changes to the docs site code

Description

Add translation about stub. Please see this issue about why I create this PR

@netlify
Copy link

netlify bot commented Jul 31, 2023

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f8b04f6
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/64d521e753e9c7000856c54e
😎 Deploy Preview https://deploy-preview-3969--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@imbant imbant changed the title Add translation about stub Add translation about stub in layout Jul 31, 2023
@sarah11918 sarah11918 added the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Jul 31, 2023
@github-actions github-actions bot removed the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Aug 2, 2023
@imbant imbant marked this pull request as ready for review August 2, 2023 09:18
@sarah11918 sarah11918 added i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! site improvement Some thing that improves the website functionality - ask @delucis for help! labels Aug 5, 2023
@Alynva
Copy link
Contributor

Alynva commented Aug 6, 2023

Looks like the getTranslation throws an error when the key is not found in that language:

https://github.com/withastro/docs/blob/main/src/i18n/util.ts#L59-L70

So we might need to add all translations before mergin!

/cc @sarah11918 @Yan-Thomas @delucis

My bad, didn't saw the || translations[fallbackLang][key] x.x
LGTM!

@imbant
Copy link
Contributor Author

imbant commented Aug 7, 2023

Looks like the getTranslation throws an error when the key is not found in that language:

https://github.com/withastro/docs/blob/main/src/i18n/util.ts#L59-L70

So we might need to add all translations before mergin!

/cc @sarah11918 @Yan-Thomas @delucis

My bad, didn't saw the || translations[fallbackLang][key] x.x LGTM!

Yep, I tested that when there is no translation for that language, it will fall back to English :)

@github-actions github-actions bot added i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! and removed i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! labels Aug 7, 2023
@github-actions github-actions bot removed the i18n Anything to do with internationalization & translation efforts - ask @YanThomas for help! label Aug 7, 2023
@imbant
Copy link
Contributor Author

imbant commented Aug 7, 2023

Sorry, I just broke my main branch by mistake. So I force-pushed it to fix it. Everything is the same as when Alynva approved these changes

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Nice effort, team! I will wait for Yan or Chris for final approval on this one, but really appreciate this thoughtful contribution, @imbant ! 🥳

@sarah11918
Copy link
Member

Hi @imbant, we had a team meeting about this one (and, my apologies, I forgot we'd already a long time ago had a discussion months ago!) and our translators at the time decided that it's better NOT to translate this content. In fact, it was my mistake to mark the stubs as i18n:true in the first place!

Most of our stub pages are really quite small, with little text to translate, so there's not lot of extra benefit from translating them. And then, if we receive an English contribution to make a more full guide, there would be a big difference between the English language page and a not-yet-updated translated page. We think it's better to have those pages fall-back to English rather than have a time where we display a stub for non-English pages when there is actually good content on the English page.

Keeping the stubs untranslated means that they will default to English, and we will only mark them as i18n:true when there is a full guide. That way, this problem never happens.

Thank you for starting these translations, even though we can't use them. 😅 We look forward to more contributions from you! 🙌

@sarah11918 sarah11918 closed this Aug 8, 2023
@sarah11918
Copy link
Member

Oops, apparently I missed a meeting!

I think we're going to fix the issue a different way, and so we CAN use this! So I'm going to stay out of this and let @Yan-Thomas finish this off. Reopening at this point, since we think we can use it!

@sarah11918 sarah11918 reopened this Aug 8, 2023
@imbant
Copy link
Contributor Author

imbant commented Aug 9, 2023

OK. Thanks for the detailed explanation. Just let me know when there is something that I need to change😀

Copy link
Member

@yanthomasdev yanthomasdev left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @imbant 🙌

@yanthomasdev yanthomasdev merged commit bdea118 into withastro:main Aug 10, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site improvement Some thing that improves the website functionality - ask @delucis for help!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants