i18n: Fix translations on the Use my domain screen #57064

sdnunca · 2021-10-15T13:29:42Z

Changes proposed in this Pull Request

Adds dynamic getters to the translated properties of optionInfo objects so that __ is called every time they are accessed, to make sure the text is refreshed when translations are loaded.

Testing instructions

Change your user language to a non-English language.
Go to https://wordpress.com/domains/add/use-my-domain/{site-URL}
Enter a test domain name, such as example.com.
Make sure the mapping options are translated:

Related to #325-gh-Automattic/i18n-issues

github-actions · 2021-10-15T13:30:01Z

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-17767
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-17767&env=jetpack

matticbot · 2021-10-15T13:30:35Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~71 bytes added 📈 [gzipped])

name     parsed_size           gzip_size
domains       +642 B  (+0.0%)      +71 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~40 bytes added 📈 [gzipped])

name                             parsed_size           gzip_size
async-load-signup-steps-domains       +306 B  (+0.1%)      +40 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

yuliyan

The Use a domain I own title on the initial screen and the Suggested setup title, when you select the Connection your domain option, still appear untranslated.

yuliyan · 2021-10-15T13:58:39Z

client/components/domains/use-my-domain/utilities/option-info.js

-		topText: __( 'Manage your domain directly on WordPress.com' ),
-		learnMoreLink: INCOMING_DOMAIN_TRANSFER,
-		benefits: [
+	transferSupported,


I suppose, these were extracted as separate constants for better readability?

delputnam · 2021-10-15T16:26:10Z

@sdnunca Thanks for the PR. Is this the recommended way to ensure that the correct text is rendered after the translated strings are loaded?

It feels redundant to have to list all of the strings twice. Is there a better way we (@Automattic/nomado) could have handled this to avoid this issue?

rafaelgallani · 2021-10-15T16:50:27Z

@sdnunca Thanks for the PR!
I was working on it as a part of #56366, so I had already addressed the points commented by @yuliyan.
I'll cherry-pick and change them a bit to make them similar to your commits and push them to your branch 😄

sdnunca · 2021-10-20T00:34:58Z

@delputnam The redundant strings felt weird to me as well. I've updated the PR to use getters directly when creating the object.

@rafaelgalani Thank you so much for your work on this. Apologies for the inconvenience, I was not aware you're already looking into this.

@yuliyan Do you see any potential issues with using getters this way? I've double checked to make sure the strings are still extracted correctly and it seems fine.

yuliyan · 2021-10-20T12:40:01Z

@yuliyan Do you see any potential issues with using getters this way? I've double checked to make sure the strings are still extracted correctly and it seems fine.

@sdnunca, IIRC, initially I got some build errors with the getter syntax and had to switch to Object.defineProperty with the duplicated strings, but that doesn't seem to be applicable anymore. I don't think there'll be any problems, so it looks like a nice improvement!

rafaelgallani · 2021-10-27T19:17:25Z

Rebasing on trunk

rafaelgallani · 2021-10-27T19:20:40Z

@yuliyan Could you please take one last look at it again? I'll merge this PR since some other developments we're doing depend on it.
I just rebased it with the latest trunk changes. I tested it again and it looks good to me both code and translation-wise. 😄

yuliyan

@rafaelgalani, I'm getting reference errors when I go to domains/add/use-my-domain/, likely due to used undefined __ dependency in use-my-domain/index.js.

yuliyan · 2021-10-28T10:06:43Z

client/components/domains/use-my-domain/index.jsx

@@ -304,7 +305,7 @@ function UseMyDomain( {
 				/* translators: %s - the name of the domain the user will add to their site */
 				return sprintf( __( 'Use a domain I own: %s' ), domainName );
 		}
-	}, [ domainName, mode, inputMode ] );
+	}, [ domainName, mode, inputMode, __ ] );


I think you forgot to call useI18n hook and as a result __ is not defined in the scope.

That's weird - even weirder that this is not even coming from this PR, anyway, just added it, thanks 😄

yuliyan · 2021-10-28T10:15:35Z

client/components/domains/connect-domain-step/constants.ts

+/**
+ * Define properties with translatable strings getters.
+ */
+Object.defineProperties( stepsHeading, {


Object.defineProperties here could probably be avoided if you use the getter syntax, similar to how it's been done in use-my-domain/utilities/option-info.js

yuliyan · 2021-10-28T10:23:02Z

client/components/domains/connect-domain-step/page-definitions.js

@@ -23,15 +36,15 @@ export const connectADomainStepsDefinition = {
 	[ stepSlug.SUGGESTED_LOGIN ]: {
 		mode: modeType.SUGGESTED,
 		step: stepType.LOG_IN_TO_PROVIDER,
-		name: __( 'Log in to provider' ),
+		name: labels[ stepSlug.SUGGESTED_LOGIN ](),


I don't think this would work - labels[ stepSlug.SUGGESTED_LOGIN ] will only be called once during the initial property definition, whereas it's expected __ to be always executed when property is accessed.
I missed the Object.defineProperties loop at the bottom. However, I think this can also be simplified using the getter syntax.

rafaelgallani · 2021-10-28T16:07:47Z

@yuliyan Done! Thanks for your review. Can you please take one last look? 😄
I re-tested it and it all looks good - everything correctly translated.

yuliyan

Tested again and it seems to be working fine. I couldn't spot any issues, so it LGTM 🚀

rafaelgallani · 2021-10-29T15:20:48Z

@yuliyan Thank you! Merging 🚀

sdnunca requested a review from a team as a code owner October 15, 2021 13:29

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 15, 2021

yuliyan reviewed Oct 15, 2021

View reviewed changes

sdnunca and others added 5 commits October 27, 2021 16:11

i18n: Fix translations on the Use my domain screen

03120d4

i18n: Fix "Use a domain I own" heading translations

ad0f405

i18n: Fix "Use a domain I own" auth code description translation

329e710

i18n: Fix "Use a domain I own" page definitions' translations

8e80e55

Use gettter syntax instead of Object.defineProperties

bf1c5f8

rafaelgallani force-pushed the fix/translations-on-use-my-domain-page branch from 6fa2478 to bf1c5f8 Compare October 27, 2021 19:17

rafaelgallani mentioned this pull request Oct 27, 2021

Adding new useMyDomain flow to onboarding #56366

Merged

yuliyan self-requested a review October 28, 2021 10:04

yuliyan requested changes Oct 28, 2021

View reviewed changes

rafaelgallani added 2 commits October 28, 2021 12:52

Add __to the component's scope

5db593d

Simplify declarations using getter syntax

8a8400d

yuliyan self-requested a review October 29, 2021 12:45

yuliyan approved these changes Oct 29, 2021

View reviewed changes

rafaelgallani merged commit f8c7fe5 into trunk Oct 29, 2021

rafaelgallani deleted the fix/translations-on-use-my-domain-page branch October 29, 2021 15:21

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

i18n: Fix translations on the Use my domain screen #57064

i18n: Fix translations on the Use my domain screen #57064

sdnunca commented Oct 15, 2021

github-actions bot commented Oct 15, 2021 •

edited by matticbot

Loading

matticbot commented Oct 15, 2021 •

edited

Loading

yuliyan left a comment

yuliyan Oct 15, 2021

delputnam commented Oct 15, 2021

rafaelgallani commented Oct 15, 2021

sdnunca commented Oct 20, 2021

yuliyan commented Oct 20, 2021

rafaelgallani commented Oct 27, 2021

rafaelgallani commented Oct 27, 2021 •

edited

Loading

yuliyan left a comment •

edited

Loading

yuliyan Oct 28, 2021

rafaelgallani Oct 28, 2021 •

edited

Loading

yuliyan Oct 28, 2021

yuliyan Oct 28, 2021 •

edited

Loading

rafaelgallani commented Oct 28, 2021

yuliyan left a comment

rafaelgallani commented Oct 29, 2021

i18n: Fix translations on the Use my domain screen #57064

i18n: Fix translations on the Use my domain screen #57064

Conversation

sdnunca commented Oct 15, 2021

Changes proposed in this Pull Request

Testing instructions

github-actions bot commented Oct 15, 2021 • edited by matticbot Loading

matticbot commented Oct 15, 2021 • edited Loading

yuliyan left a comment

Choose a reason for hiding this comment

yuliyan Oct 15, 2021

Choose a reason for hiding this comment

delputnam commented Oct 15, 2021

rafaelgallani commented Oct 15, 2021

sdnunca commented Oct 20, 2021

yuliyan commented Oct 20, 2021

rafaelgallani commented Oct 27, 2021

rafaelgallani commented Oct 27, 2021 • edited Loading

yuliyan left a comment • edited Loading

Choose a reason for hiding this comment

yuliyan Oct 28, 2021

Choose a reason for hiding this comment

rafaelgallani Oct 28, 2021 • edited Loading

Choose a reason for hiding this comment

yuliyan Oct 28, 2021

Choose a reason for hiding this comment

yuliyan Oct 28, 2021 • edited Loading

Choose a reason for hiding this comment

rafaelgallani commented Oct 28, 2021

yuliyan left a comment

Choose a reason for hiding this comment

rafaelgallani commented Oct 29, 2021

github-actions bot commented Oct 15, 2021 •

edited by matticbot

Loading

matticbot commented Oct 15, 2021 •

edited

Loading

rafaelgallani commented Oct 27, 2021 •

edited

Loading

yuliyan left a comment •

edited

Loading

rafaelgallani Oct 28, 2021 •

edited

Loading

yuliyan Oct 28, 2021 •

edited

Loading