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.

Sign up for GitHub

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

Jump to bottom

i18n: Make "VS Code" and "PHPStorm" translatable #48

Merged
merged 5 commits into from
Apr 29, 2024
Merged

i18n: Make "VS Code" and "PHPStorm" translatable #48

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7e18ae3
i18n: Makes "VS Code" and "PHPStorm" translatable
t-hamano Apr 26, 2024
10a66e2
Update src/components/content-tab-overview.tsx
t-hamano Apr 29, 2024
491d486
Update src/components/content-tab-overview.tsx
t-hamano Apr 29, 2024
4f37357
Fix lint
t-hamano Apr 29, 2024
400e186
Add translators comment
t-hamano Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/components/content-tab-overview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as Sentry from '@sentry/electron/renderer';
import { __, sprintf } from '@wordpress/i18n';
import { __ } from '@wordpress/i18n';
import {
archive,
code,
Expand Down Expand Up @@ -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' ),
Copy link
Contributor

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.

Copy link
Contributor Author

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.

https://github.com/WordPress/wordpress-develop/blob/66b5d25be2b5e69833bacfe8f64de660ffc2bfa9/src/wp-includes/theme.php#L2394-L2433

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.

Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor Author

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.

className: 'text-nowrap',
icon: code,
onClick: async () => {
Expand All @@ -161,17 +161,17 @@ function ShortcutsSection( { selectedSite }: Pick< ContentTabOverviewProps, 'sel
} catch ( error ) {
Sentry.captureException( error );
alert(
sprintf(
__( "Could not open the site code in %s. Please check if it's installed correctly." ),
'VS Code'
// translators: "VS Code" is the brand name for an IDE and does not need to be translated
__(
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
"Could not open the site code in VS Code. Please check if it's installed correctly."
)
);
}
},
} );
} else if ( installedApps.phpstorm ) {
buttonsArray.push( {
label: 'PhpStorm',
label: __( 'PhpStorm' ),
className: 'text-nowrap',
icon: code,
onClick: async () => {
Expand All @@ -180,9 +180,9 @@ function ShortcutsSection( { selectedSite }: Pick< ContentTabOverviewProps, 'sel
} catch ( error ) {
Sentry.captureException( error );
alert(
sprintf(
__( "Could not open the site code in %s. Please check if it's installed correctly." ),
'PhpStorm'
// translators: "PhpStorm" is the brand name for an IDE and does not need to be translated
__(
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
"Could not open the site code in PhpStorm. Please check if it's installed correctly."
)
);
}
Expand Down
Loading