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

feat(v2): add pt-PT translations for docusaurus-theme-classic #4536

Merged
merged 4 commits into from
Mar 30, 2021
Merged

feat(v2): add pt-PT translations for docusaurus-theme-classic #4536

merged 4 commits into from
Mar 30, 2021

Conversation

tiago-err
Copy link
Contributor

Motivation

Following this comment asking if it was possible to also have the translation for pt-PT translations, I wanted to help.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Since I only added a new translation JSON, there is no test plan, the existing tests should do fine.

Related PRs

None

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 30, 2021
@netlify
Copy link

netlify bot commented Mar 30, 2021

@netlify
Copy link

netlify bot commented Mar 30, 2021

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit bf36662

https://deploy-preview-4536--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Mar 30, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 62
🟢 Accessibility 96
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4536--docusaurus-2.netlify.app/

@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Mar 30, 2021
@slorber
Copy link
Collaborator

slorber commented Mar 30, 2021

thanks!

@slorber
Copy link
Collaborator

slorber commented Mar 30, 2021

Was wondering, if a user choose locale = "pt", should we apply PT or BR translations by default?

@netlify
Copy link

netlify bot commented Mar 30, 2021

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit 77e3217

https://deploy-preview-4536--docusaurus-2.netlify.app

@slorber slorber merged commit fb07bd8 into facebook:master Mar 30, 2021
@tiago-err
Copy link
Contributor Author

That is actually a good question! If you think with maths, it would probably be better to use pt-BR, since there are waaaay more brazilian people than portuguese people, however pt-PT is the "original" ahahah

If I had to choose only one, maybe pt-BR, since it more probable that someone would brazilian rather than portuguese.

@slorber
Copy link
Collaborator

slorber commented Mar 30, 2021

I see
But if you want a Docusaurus site in Brazilian, would you try using "pt" instead of using directly "pt-BR"?

Maybe we should have pt.json (for pt + pt-PT) and pt-BR.json?

@tiago-err
Copy link
Contributor Author

Yeah, that's probably the best approach, since I believe most brazilian people are more used to seeing "pt-BR" than simply "pt".

@slorber
Copy link
Collaborator

slorber commented Mar 30, 2021

Apparently, it's pt-BR :)

https://twitter.com/sebastienlorber/status/1376825837417340931

image

@tiago-err
Copy link
Contributor Author

Fair enough then, we learn things every day ahahah

@slorber
Copy link
Collaborator

slorber commented Apr 6, 2021

"pt" -> "pt-BR" implemented in #4581

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants