Fix missing translations #1227

aaemnnosttv · 2020-03-06T08:28:17Z

Summary

Addresses issue #1163

Relevant technical choices

Most fixes were found using the new ESLint rules introduced in First pass at I18N-specific ESLint rules WordPress/gutenberg#20555
- Ensures that all missing "translators:" comments are present
- Ensures that all translation functions use the proper text-domain
Made a few small changes to strings that were done incorrectly (will highlight below)
One of these required VRT references to be updated

Once the above PR is merged, and we're using the newer ESLint plugin, we can add the following to our eslintrc.json

diff --git a/.eslintrc.json b/.eslintrc.json
index 7da2f721..78900670 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -8,6 +8,13 @@
     "es6": true
   },
   "rules": {
+    "@wordpress/valid-text-domain": [
+      "error",
+      {
+        "allowDefault": false,
+        "allowedTextDomains": ["google-site-kit"]
+      }
+    ],
     "no-restricted-globals": [
       "error",
       {

Checklist

My code is tested and passes existing unit tests.
My code has an appropriate set of unit tests which all pass.
My code is backward-compatible with WordPress 4.7 and PHP 5.6.
My code follows the WordPress coding standards.
My code has proper inline documentation.
I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

aaemnnosttv · 2020-03-06T08:32:38Z

assets/js/components/optin.js

-					{ __( 'Help us improve the Site Kit plugin by allowing tracking of anonymous usage stats. All data are treated in accordance with ', 'google-site-kit' ) }
-					<a href="https://policies.google.com/privacy" target="_blank" rel="noopener noreferrer">{ __( 'Google Privacy Policy', 'google-site-kit' ) }</a>.


This is incorrect because strings can't be properly translated when concatenated. This also renders oddly in some cases, e.g. in Español where the translation is missing the space at the end so the text and the link are running together.

aaemnnosttv · 2020-03-06T08:33:27Z

assets/js/modules/analytics/setup.js

@@ -1102,7 +1102,8 @@ class AnalyticsSetup extends Component {
 				}

 				{ !! existingTag &&
-					<p>{ sprintf( __( 'An existing analytics tag was found on your site with the id %s. If later on you decide to replace this tag, Site Kit can place the new tag for you. Make sure you remove the old tag first.', 'google-site-kit' ), existingTag ) }</p>
+					/* translators: %s: Analytics tag ID */
+					<p>{ sprintf( __( 'An existing analytics tag was found on your site with the ID %s. If later on you decide to replace this tag, Site Kit can place the new tag for you. Make sure you remove the old tag first.', 'google-site-kit' ), existingTag ) }</p>


Capitalized "ID"

I doubt that this translator comment is going to be picked up. It should be inside the brackets, preceding either sprintf or ideally the __() call.

<p>{ sprintf( /* translators: %s: Analytics tag ID */ __( 'An existing analytics tag was found on your site with the ID %s. If later on you decide to replace this tag, Site Kit can place the new tag for you. Make sure you remove the old tag first.', 'google-site-kit' ), existingTag ) }</p>

aaemnnosttv · 2020-03-06T08:36:42Z

assets/js/modules/search-console/dashboard/dashboard-widget-sitestats.js

+		let title = __( 'Search Traffic Summary', 'google-site-kit' );
+
+		if ( pageTitle && pageTitle.length ) {
+			/* translators: %s: page title */
+			title = sprintf( __( 'Search Traffic Summary for %s', 'google-site-kit' ), decodeHtmlEntity( pageTitle ) );
+		}


The ESLint rules weren't properly detecting the translators string in a multiline ternary so I split it up this way, which is probably cleaner anyway 😄

Makes sense 👍

With the ternary it wouldn't be clear which __() call it should be associated with.

aaemnnosttv · 2020-03-06T08:53:14Z

assets/js/components/setup-module.js

 									sprintf( __( 'Set up %s', 'google-site-kit' ), name ) :
-									sprintf( __( 'Setup Analytics to gain access to %s', 'google-site-kit' ), name )
+									/* translators: %s: module name */
+									sprintf( __( 'Set up Analytics to gain access to %s', 'google-site-kit' ), name )


Changed from "Setup Analytics" to "Set up Analytics" for consistency, but also because it is the correct form to use here. This caused VRT to fail, hence updates for those as well.

felixarntz

Awesome work! +1 on changing the concatenated strings - I'm sure there are more of these around, let's keep improving these problematic occurrences as we spot them.

googlebot · 2020-03-06T19:17:21Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-03-06T19:17:32Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

aaemnnosttv added 5 commits March 6, 2020 10:17

Move translations in defaultProps to defaults.

6fa7a85

Add missing text domain in setup proxy.

7f92afb

Fix improper translation string in OptIn.

33c6f66

Add missing translators comments.

8696d4c

Refactor options.chart.title, add translators comment.

8fe1c26

googlebot added the cla: yes label Mar 6, 2020

aaemnnosttv commented Mar 6, 2020

View reviewed changes

aaemnnosttv added 2 commits March 6, 2020 10:44

OptIn: extract privacy policy URL to placeholder.

21c84f1

Approve visual changes.

41de4ee

aaemnnosttv commented Mar 6, 2020

View reviewed changes

aaemnnosttv marked this pull request as ready for review March 6, 2020 09:02

aaemnnosttv requested a review from felixarntz March 6, 2020 09:02

felixarntz approved these changes Mar 6, 2020

View reviewed changes

Merge branch 'develop' into fix/1163-missing-translations

f40b189

googlebot added cla: no and removed cla: yes labels Mar 6, 2020

felixarntz added cla: yes and removed cla: no labels Mar 6, 2020

felixarntz merged commit 226c1df into develop Mar 6, 2020

felixarntz deleted the fix/1163-missing-translations branch March 6, 2020 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing translations #1227

Fix missing translations #1227

aaemnnosttv commented Mar 6, 2020 •

edited

aaemnnosttv Mar 6, 2020 •

edited

aaemnnosttv Mar 6, 2020

swissspidy Mar 11, 2020

aaemnnosttv Mar 6, 2020

swissspidy Mar 11, 2020

aaemnnosttv Mar 6, 2020

felixarntz left a comment

googlebot commented Mar 6, 2020

googlebot commented Mar 6, 2020

		{ __( 'Help us improve the Site Kit plugin by allowing tracking of anonymous usage stats. All data are treated in accordance with ', 'google-site-kit' ) }
		<a href="https://policies.google.com/privacy" target="_blank" rel="noopener noreferrer">{ __( 'Google Privacy Policy', 'google-site-kit' ) }</a>.

Fix missing translations #1227

Fix missing translations #1227

Conversation

aaemnnosttv commented Mar 6, 2020 • edited

Summary

Relevant technical choices

Checklist

aaemnnosttv Mar 6, 2020 • edited

Choose a reason for hiding this comment

aaemnnosttv Mar 6, 2020

Choose a reason for hiding this comment

swissspidy Mar 11, 2020

Choose a reason for hiding this comment

aaemnnosttv Mar 6, 2020

Choose a reason for hiding this comment

swissspidy Mar 11, 2020

Choose a reason for hiding this comment

aaemnnosttv Mar 6, 2020

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

googlebot commented Mar 6, 2020

googlebot commented Mar 6, 2020

aaemnnosttv commented Mar 6, 2020 •

edited

aaemnnosttv Mar 6, 2020 •

edited