DCOS-43383: Translate /plugins component strings #3372

natmegs · 2018-10-24T04:19:33Z

Testing

There should be no visual changes as a result of these commits.

Trade-offs

Many unit tests had to be refactored, rewritten, or converted to integration tests.

brandonc

Wow! That was a tedious review. Thanks for organizing your commits that made it a lot easier. Only one or two minor items. Another question: Can we now remove react-intl and if not can you remove any more en-us catalog items based on some of the migrations you did?

plugins/services/src/js/components/DeclinedOffersTable.js

plugins/services/src/js/components/DeploymentsModal.js

plugins/services/src/js/components/DeploymentStatusIndicator.js

plugins/services/src/js/components/RecentOffersSummary.js

brandonc · 2018-10-25T07:31:58Z

plugins/services/src/js/components/modals/CreateServiceModalForm.js

@@ -513,7 +530,13 @@ class CreateServiceModalForm extends Component {
        <TabButton
          className={item.className}
          id={item.id}
-          label={item.label}
+          label={


I may be misreading rn, but when is label not a string?

I believe there is a situation where it was being passed a styled span thus breaking Trans

plugins/services/src/js/components/modals/ServiceActionDisabledModal.js

plugins/services/src/js/containers/service-detail/ServiceDetail.js

plugins/services/src/js/service-configuration/ServiceConfigBaseSectionDisplay.js

brandonc · 2018-10-25T08:10:23Z

plugins/catalog/src/js/repositories/components/AddRepositoryFormModal.js

        required: false,
        min: "0",
        max: `${numberOfRepositories}`,
        step: "1",
-        validationErrorText: `Must be a positive integer between 0 and ${numberOfRepositories} representing its priority. 0 is the highest and ${numberOfRepositories} denotes the lowest priority.`,
+        validationErrorText: i18n._(


I don't think this can be translated this way since you'll get a difference "id" phrase for every numberOfRepositories

I think I was wrong about this? The phrase looks extracted.

brandonc

I meant to "request changes" in my last review

nLight · 2018-10-26T14:50:27Z

rebased + added a small commit to include #3336 I didn't run lingui-extract-with-plugins as it has created a lot of new strings with the latest plugins so I decided to leave it to you :)

nLight · 2018-10-26T15:08:14Z

CONTRIBUTING.md

@@ -496,7 +496,7 @@ You will need a fully functional cluster to run your system tests.

 ## i18n

-DCOS UI uses [React-Intl](https://github.com/yahoo/react-intl) to enable i18n, please look at the documentation. Currently this project is only supporting `en-us` but planning to support more languages/locales in the future.
+DCOS UI uses [lingui](https://github.com/lingui/js-lingui) to enable i18n, please look at the documentation. Currently this project is only supporting `en-us` but planning to support more languages/locales in the future.


nLight · 2018-10-26T15:08:45Z

package.json

@@ -67,7 +67,6 @@
    "react-addons-pure-render-mixin": "15.4.1",
    "react-dom": "15.4.1",
    "react-gemini-scrollbar": "2.1.5",
-    "react-intl": "2.2.3",


nLight · 2018-10-26T15:09:35Z

src/js/index.js

@@ -101,9 +96,7 @@ RequestUtil.json = function(options = {}) {
              language={UserLanguageStore.get()}
              catalogs={{ en }}
            >
-              <IntlProvider locale={navigatorLanguage} messages={enUS}>


nLight · 2018-10-26T15:23:19Z

plugins/catalog/src/js/repositories/components/AddRepositoryFormModal.js

        required: false,
        min: "0",
        max: `${numberOfRepositories}`,
        step: "1",
-        validationErrorText: `Must be a positive integer between 0 and ${numberOfRepositories} representing its priority. 0 is the highest and ${numberOfRepositories} denotes the lowest priority.`,
+        validationErrorText:


Why can't we tag the whole thing?

I was under the misguided impression that i18n._(t... did not play nicely with strings including variables... but I just tried it now and it seems to work! Will change

nLight · 2018-10-26T15:26:07Z

plugins/services/src/js/components/DeploymentStatusIndicator.js

@@ -58,17 +59,17 @@ class DeploymentStatusIndicator extends mixin(StoreMixin) {
      return null;
    }

+    const deploymentText =
+      deploymentsCount === 1 ? i18nMark("deployment") : i18nMark("deployments");


I'd rather // TODO pluralize. Some languages 🇷🇺 have interesting rules around quantities :D

@brandonc conflicting feedback re: pluralization. Would you mind if I changed these back to L10NTODOs?

Well, tbh I have no idea about what kinds of problems can arise so let's mark it

nLight · 2018-10-26T15:27:47Z

plugins/services/src/js/components/DeploymentsModal.js

@@ -512,16 +533,15 @@ class DeploymentsModal extends mixin(StoreMixin) {
        </button>
      </div>
    );
+
+    // L10NTODO: Pluralize


nLight · 2018-10-26T15:29:53Z

plugins/services/src/js/components/modals/CreateServiceModalForm.js

-      findNestedPropertyInObject(appConfig, "containers.length") || 1
-    );
+    const serviceLabel =
+      (findNestedPropertyInObject(appConfig, "containers.length") || 1) === 1


Todo pluralize

nLight · 2018-10-26T15:37:24Z

plugins/services/src/js/components/DeploymentStatusIndicator.js

@@ -58,17 +59,17 @@ class DeploymentStatusIndicator extends mixin(StoreMixin) {
      return null;
    }

+    const deploymentText =


Suggested change

const deploymentText =

// L10NTODO: Pluralize

const deploymentText =

nLight · 2018-10-26T15:37:42Z

plugins/services/src/js/components/DeploymentsModal.js

-      return `This will stop the current deployment of ${listOfServiceNames} and
-              start a new deployment to ${UserActions.DELETE} the affected
-              ${service}.`;
+      if (serviceCount === 1) {


Suggested change

if (serviceCount === 1) {

// L10NTODO: Pluralize

if (serviceCount === 1) {

nLight · 2018-10-26T15:38:00Z

plugins/services/src/js/components/modals/CreateServiceModalForm.js

-      findNestedPropertyInObject(appConfig, "containers.length") || 1
-    );
+    const serviceLabel =
+      (findNestedPropertyInObject(appConfig, "containers.length") || 1) === 1


Suggested change

(findNestedPropertyInObject(appConfig, "containers.length") || 1) === 1

// L10NTODO: Pluralize

(findNestedPropertyInObject(appConfig, "containers.length") || 1) === 1

natmegs · 2018-10-26T19:40:03Z

@nLight Thanks for the feedback! I've addressed your comments but it's not obvious since they have not been marked as 'outdated'

brandonc

Only one remaining item! (See added comment to previous review)

brandonc · 2018-10-31T17:58:39Z

plugins/catalog/src/js/repositories/components/AddRepositoryFormModal.js

        required: false,
        min: "0",
        max: `${numberOfRepositories}`,
        step: "1",
-        validationErrorText: `Must be a positive integer between 0 and ${numberOfRepositories} representing its priority. 0 is the highest and ${numberOfRepositories} denotes the lowest priority.`,
+        validationErrorText: i18n._(


I think I was wrong about this? The phrase looks extracted.

DanielMSchmidt · 2018-11-05T04:51:26Z

tests/pages/services/DeploymentsModal-cy.js


        cy.wait(700).then(function() {
          expect(document.querySelectorAll(".modal").length).to.equal(0);
        });
      });
    });
  });
+
+  context("Rollback modal", function() {


DanielMSchmidt · 2018-11-05T04:57:11Z

plugins/services/src/js/components/modals/__tests__/EditServiceModal-test.js

@@ -72,7 +72,9 @@ describe("EditServiceModal", function() {

    const wrapper = shallow(
      <EditServiceModal params={{ id: "/my-new-service" }} />
-    );
+    )
+      .dive()


TIL about .dive, I normally switch to mount on this occasions.

DanielMSchmidt · 2018-11-05T05:09:14Z

@natmegs The CI errors look like something on the service page is wrong

…trings

…trings Mark and pretranslate component strings as neccessary, and replace unit tests with more appropriate integration tests.

Translate SearchLog component strings using combination of marking/macros and pretranslation with withI18next HOC. Modify applicable unit tests to use enzyme and to test behaviour rather than internal implementation details. Remove deprecated unit test.

…trings Translate column headings and modify ServiceConfigDisplayUtil render function to display translated marked strings. Modify headings in other components that use the util to be functions instead of Trans elements.

Translate text content, including marking and pretranslating bare strings, in all Nodes components.

Translate component text, including marking and pretranslating bare strings, for all Jobs components.

Translate text content, including marking and pretranslating bare strings, in all Repositories components.

Update translation catalog with all extracted messages. Closes DCOS-43383.

mesosphere-ci · 2018-11-05T22:28:41Z

🎉 This PR is included in version 2.35.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

natmegs added the work in progress label Oct 24, 2018

natmegs force-pushed the natalie/feat/DCOS-43383-strings-plugins branch from 19fdcee to ea00082 Compare October 24, 2018 16:41

natmegs removed the work in progress label Oct 24, 2018

brandonc reviewed Oct 25, 2018

View reviewed changes

brandonc suggested changes Oct 25, 2018

View reviewed changes

natmegs force-pushed the natalie/feat/DCOS-43383-strings-plugins branch 3 times, most recently from d352909 to 990b811 Compare October 26, 2018 02:27

nLight force-pushed the natalie/feat/DCOS-43383-strings-plugins branch from 990b811 to b015a4e Compare October 26, 2018 14:48

nLight reviewed Oct 26, 2018

View reviewed changes

natmegs force-pushed the natalie/feat/DCOS-43383-strings-plugins branch from b015a4e to 81f98d1 Compare October 26, 2018 19:36

natmegs force-pushed the natalie/feat/DCOS-43383-strings-plugins branch from 81f98d1 to 581b127 Compare October 31, 2018 17:33

brandonc reviewed Oct 31, 2018

View reviewed changes

natmegs force-pushed the natalie/feat/DCOS-43383-strings-plugins branch 2 times, most recently from 5e4a893 to 0b0e810 Compare October 31, 2018 20:47

brandonc previously approved these changes Oct 31, 2018

View reviewed changes

DanielMSchmidt previously approved these changes Nov 5, 2018

View reviewed changes

natmegs added 10 commits November 5, 2018 09:21

feat(ConfigurationMapBooleanValue): handle marked and pretranslated s…

f0b4dce

…trings

feat(DeclinedOffersTable): mark strings for translation

2c5d3b3

feat(DeploymentsModal): handle marking and pretranslating component s…

8912045

…trings Mark and pretranslate component strings as neccessary, and replace unit tests with more appropriate integration tests.

feat(DeploymentStatusIndicator): mark component strings for translation

c24a351

feat(FilterByService): translate component strings

e4fcf8c

feat(MarathonTaskDetailsList): translate component strings

fd4e39e

feat(PodDetail): translate component strings

1d7e843

feat(RecentOffersSummary): translate component strings

67dc850

feat(ServiceBreadcrumbs): translate component strings including plural

73603fd

natmegs and others added 22 commits November 5, 2018 09:21

feat(PodGeneralConfigSection): translate component text

925603b

feat(PodHealthChecksConfigSection): translate component text

9461dbc

feat(PodLabelsConfigSection): translate component text

91a888c

feat(PodNetworkConfigSection): translate component text

722c7fe

feat(PodPlacementConstraintsConfigSection): translate component text

c2fa0f9

feat(PodStorageConfigSection): translate component text

35b0a6a

feat(ServiceConfigBaseSectionDisplay): translate component text

39707e3

feat(ServiceGeneralConfigSection): translate component text

682487b

feat(ServiceHealthChecksConfigSection): translate component text

4cd86d8

feat(ServiceLabelsConfigSection): translate component text

9810012

feat(ServiceNetworkingConfigSection): translate component text

929562b

feat(ServicePlacementConstraintsConfigSection): translate component text

f1a2d0e

feat(ServiceStorageConfigSection): translate component text

ed8dcf1

feat(Nodes): translate text content

090bebb

Translate text content, including marking and pretranslating bare strings, in all Nodes components.

feat(Jobs): translate component text

536720c

Translate component text, including marking and pretranslating bare strings, for all Jobs components.

feat(Repositories): translate component text

8ac0046

Translate text content, including marking and pretranslating bare strings, in all Repositories components.

feat(AuthenticatedUserAccountDropdown): translate dropdown content

9fd2f02

chore(messages.json): update translation catalog

97168dd

chore(react-intl): remove any deprecated references to react-intl

95a0fda

chore: wrap Network Name with Trans

298f123

chore(messages.json): update translation message catalog

e2dcf8a

Update translation catalog with all extracted messages. Closes DCOS-43383.

natmegs dismissed stale reviews from DanielMSchmidt and brandonc via e2dcf8a November 5, 2018 17:23

natmegs force-pushed the natalie/feat/DCOS-43383-strings-plugins branch from 0b0e810 to e2dcf8a Compare November 5, 2018 17:23

brandonc approved these changes Nov 5, 2018

View reviewed changes

TattdCodeMonkey approved these changes Nov 5, 2018

View reviewed changes

brandonc merged commit 9ee59c2 into master Nov 5, 2018

brandonc deleted the natalie/feat/DCOS-43383-strings-plugins branch November 5, 2018 21:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DCOS-43383: Translate /plugins component strings #3372

DCOS-43383: Translate /plugins component strings #3372

natmegs commented Oct 24, 2018

brandonc left a comment

brandonc Oct 25, 2018

natmegs Oct 26, 2018

brandonc Oct 25, 2018

brandonc Oct 31, 2018

brandonc left a comment

nLight commented Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

natmegs Oct 26, 2018

nLight Oct 26, 2018

natmegs Oct 26, 2018

brandonc Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

nLight Oct 26, 2018

natmegs commented Oct 26, 2018

brandonc left a comment •

edited

Loading

brandonc Oct 31, 2018

DanielMSchmidt Nov 5, 2018

DanielMSchmidt Nov 5, 2018

DanielMSchmidt commented Nov 5, 2018

mesosphere-ci commented Nov 5, 2018

	const deploymentText =
	// L10NTODO: Pluralize
	const deploymentText =

	if (serviceCount === 1) {
	// L10NTODO: Pluralize
	if (serviceCount === 1) {

	(findNestedPropertyInObject(appConfig, "containers.length") \|\| 1) === 1
	// L10NTODO: Pluralize
	(findNestedPropertyInObject(appConfig, "containers.length") \|\| 1) === 1

DCOS-43383: Translate /plugins component strings #3372

DCOS-43383: Translate /plugins component strings #3372

Conversation

natmegs commented Oct 24, 2018

Testing

Trade-offs

brandonc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonc left a comment

Choose a reason for hiding this comment

nLight commented Oct 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natmegs commented Oct 26, 2018

brandonc left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielMSchmidt commented Nov 5, 2018

mesosphere-ci commented Nov 5, 2018

brandonc left a comment •

edited

Loading