Skip to content

refactor: add missing translations in fa.json#5410

Merged
slorber merged 5 commits into
facebook:mainfrom
farshidinanloo:patch-1
Aug 30, 2021
Merged

refactor: add missing translations in fa.json#5410
slorber merged 5 commits into
facebook:mainfrom
farshidinanloo:patch-1

Conversation

@farshidinanloo
Copy link
Copy Markdown
Contributor

fix Persian translation

Motivation

there is repetitive word in translation:
1

the correct one is like this:
image

fix Persian translation
@facebook-github-bot
Copy link
Copy Markdown
Contributor

Hi @farshidinanloo!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 23, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 820f826

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/612a724222e06000087c97a2

😎 Browse the preview: https://deploy-preview-5410--docusaurus-2.netlify.app

@github-actions
Copy link
Copy Markdown

ghost commented Aug 23, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 97
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

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

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 23, 2021
@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link
Copy Markdown
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

@farshidinanloo do you have a desire to provide a missing translation for other messages in this file? Maybe you could do that in this PR?

@slorber
Copy link
Copy Markdown
Collaborator

slorber commented Aug 24, 2021

@massoudmaboudi can you help review this change?

@farshidinanloo
Copy link
Copy Markdown
Contributor Author

@lex111 @massoudmaboudi
I try to add other translations and there is an issue the date it shows is wrong it's about 3 years before the current date.

Copy link
Copy Markdown
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks but this is not exactly what we wanted 😅

Also if there is a problem with a date, please give much more details (screenshot, URL, date displayed when using English locale...) or something because it's hard to understand what you are talking about here. Have you noticed that it's written the date is simulated in dev?

@@ -1,18 +1,32 @@
{
"theme.AnnouncementBar.closeButtonAriaLabel": "بستن",
"theme.AnnouncementBar.closeButtonAriaLabel___DESCRIPTION": "برچسب ARIA دکمه بستن در نوار اعلانات",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please don't add/translate fields with ___DESCRIPTION, those are only in the base file to help you with additional context about the string to translate 🤪

Let me know how we can make this clearer

"theme.lastUpdated.lastUpdatedAtBy": "آخرین بروزرسانی در {atDate} توسط {byUser}",
"theme.lastUpdated.lastUpdatedAtBy": "آخرین بروزرسانی {atDate} {byUser}",
"theme.navbar.mobileSidebarSecondaryMenu.backButtonLabel": "→ بازگشت به منو اصلی",
"theme.navbar.mobileVersionsDropdown.label": "Versions",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The missing translations"we talk about are keys that are already in this file, such as this one (theme.navbar.mobileVersionsDropdown.label)

You are not supposed to add keys from base.json, just make sure all the keys already there are properly translated

"theme.lastUpdated.atDate": " در تاریخ {date}",
"theme.lastUpdated.byUser": " توسط {user}",
"theme.lastUpdated.lastUpdatedAtBy": "آخرین بروزرسانی در {atDate} توسط {byUser}",
"theme.lastUpdated.lastUpdatedAtBy": "آخرین بروزرسانی {atDate} {byUser}",
Copy link
Copy Markdown
Collaborator

@slorber slorber Aug 26, 2021

Choose a reason for hiding this comment

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

doesn't this lead to duplicate spaces?

Because theme.lastUpdated.atDate alrready contains a space (on purpose).

Other languages usually don't add spaces in theme.lastUpdated.lastUpdatedAtBy.

Please double-check for potentially duplicated spaces. I believe the correct form may be:

"theme.lastUpdated.lastUpdatedAtBy": "آخرین بروزرسانی{atDate}{byUser}",

The spaces are in {atDate}{byUser} because both of those variables are optional

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@slorber
sorry about adding wrong translations 😁
and about spaces you are right this is true form: {atDate}{byUser}
I role back the changes and fix this spaces
and the date in the snapshot is 1397/07/22 but in Solar Hijri calendar we are in year 1400, I try to add demo link
thank you

@slorber slorber changed the title Update fa.json refactor: add missing translations in fa.json Aug 26, 2021
@slorber slorber added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label Aug 26, 2021
@slorber
Copy link
Copy Markdown
Collaborator

slorber commented Aug 26, 2021

Thanks

There are others untranslated strings in this file that you could also translate. I see these 2:

  "theme.docs.tagDocListPageTitle": "{nDocsTagged} with \"{tagName}\"",
  "theme.docs.tagDocListPageTitle.nDocsTagged": "One doc tagged|{count} docs tagged",

and the date in the snapshot is 1397/07/22 but in Solar Hijri calendar we are in year 1400, I try to add demo link

Yes that would be useful because I have no idea about this calendar what's the problem here 🤪

The code we use to format blog post dates is:

new Intl.DateTimeFormat("fa", {
      day: 'numeric',
      month: 'long',
      year: 'numeric',
      timeZone: 'UTC',
    }).format(new Date())

image

Isn't it correct?

Do you have another alternative JS solution using the Intl API to print the correct date?

@farshidinanloo
Copy link
Copy Markdown
Contributor Author

@slorber
the date in your snapshot is correct.

but look at this demo I notice the date in local development and production is different when locale config is fa in local it show wrong Solar Hijri calendar as I said before and in production it show Gregorian date.

please look at this demo: https://docs-test-date.netlify.app/
source code: https://gitlab.com/farshidinanloo73/docusaurus-test

@slorber
Copy link
Copy Markdown
Collaborator

slorber commented Aug 30, 2021

Thanks 👍

Please take a look at this other PR, suggesting to use slightly different fa translations: #5434

@slorber slorber merged commit f1ae06f into facebook:main Aug 30, 2021
@slorber
Copy link
Copy Markdown
Collaborator

slorber commented Aug 30, 2021

About the calendars, thanks, I was able to figure out the problem and opened a dedicated issue: #5440

@massoudmaboudi
Copy link
Copy Markdown
Contributor

@slorber
Sorry for the late reply.
Yes, the changes are correct and you can merge them.

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: polish This PR adds a very minor behavior improvement that users will enjoy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants