-
Notifications
You must be signed in to change notification settings - Fork 18
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
i18n: Make "VS Code" and "PHPStorm" translatable #48
i18n: Make "VS Code" and "PHPStorm" translatable #48
Conversation
Thanks for the PR! I've seen that PhpStorm is called the same in other languages like Japanese. https://www.jetbrains.com/ja-jp/phpstorm/ |
Thanks for the review! Indeed, in Japanese these terms remain the same. However, when looking at WordPress core, it seems that depending on the language, there may be no spaces before or after the term depending on the translation or context. Below is an example of how "YouTube URL" is translated into different formats and terms. Arabic
Dutch (Belgium)
|
I found those examples really useful, thank you for sharing @t-hamano! For further context, we explicitly prevented those strings from being translatable in b62b169 after it was noted that the button text was being incorrectly translated: Screenshot by @fluiddot As the buttons only display the app name, with no other text, maybe we don't need to make the button label translatable? Alternatively, perhaps a note for translators would prevent this issue? The changes to the error message look good to me, to allow for the differences in grammar you noted. |
Co-authored-by: Siobhan Bamber <siobhan@automattic.com>
Co-authored-by: Siobhan Bamber <siobhan@automattic.com>
I think it's a good idea 👍 |
@@ -152,7 +152,7 @@ function ShortcutsSection( { selectedSite }: Pick< ContentTabOverviewProps, 'sel | |||
if ( installedApps.vscode ) { | |||
// Use VS Code as a default even if none of the editors are installed | |||
buttonsArray.push( { | |||
label: 'VS Code', | |||
label: __( 'VS Code' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As outlined in #48 (comment), we explicitly changed this string and PhpStorm
to be not translatable. Based on their webpages, seems the IDE's names are not translated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all text needs to be translatable, whether it's a proper name or not.
For example, in the core, translation functions are applied to all service names like the ones below.
Mistakes can always be made in localization, including this button text. Personally, I feel that hard-coding only some proper nouns is insufficient to prevent this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add a note for translators for VS Code
and PhpStorm
phrases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see. In that case, I wonder if we could add a comment for translators to omit the IDE's name. Similar to what we added here. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, updated by 400e186.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is any language that would benefit from translating those phrases, it makes sense to make them translatable.
Adding a translator's note sounds like a good precaution, and if we spot the name was unnecessary translated, we can fix it in GlotPress.
@@ -152,7 +152,7 @@ function ShortcutsSection( { selectedSite }: Pick< ContentTabOverviewProps, 'sel | |||
if ( installedApps.vscode ) { | |||
// Use VS Code as a default even if none of the editors are installed | |||
buttonsArray.push( { | |||
label: 'VS Code', | |||
label: __( 'VS Code' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add a note for translators for VS Code
and PhpStorm
phrases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎊 ! Thanks @t-hamano for the update 🙇 !
Good point @wojtekn, I shared something similar in #48 (comment). I'd like to note that this was addressed in 400e186. |
Proposed Changes
This PR makes the texts "VS Code" and "PHPstorm" translatable. Since these terms are proper nouns, the text may be the same in any language, but I think all text present in the app should be translatable.
Testing Instructions
If it is the default language, the Overview tab content should continue to display as before.
Pre-merge Checklist