From 80ac9584c879b49a97df599b9abd60f2a4dc29e6 Mon Sep 17 00:00:00 2001 From: Christos Date: Tue, 14 Jan 2025 11:31:10 +0200 Subject: [PATCH] i18n: Introduce `fixMe` method to `i18n-calypso` (#97690) --- packages/i18n-calypso/CHANGELOG.md | 4 +++ packages/i18n-calypso/README.md | 20 ++++++++++++ packages/i18n-calypso/package.json | 2 +- packages/i18n-calypso/src/i18n.js | 27 ++++++++++++++++ packages/i18n-calypso/src/index.js | 1 + packages/i18n-calypso/test/index.js | 49 +++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 1 deletion(-) diff --git a/packages/i18n-calypso/CHANGELOG.md b/packages/i18n-calypso/CHANGELOG.md index 8b734225dddfc..ae3d79ab2c0ef 100644 --- a/packages/i18n-calypso/CHANGELOG.md +++ b/packages/i18n-calypso/CHANGELOG.md @@ -1,3 +1,7 @@ +## 7.1.0 + +- Add new `fixMe` method for conditionally loading a copy given a respective translation exists + ## 7.0.0 - Update React peer dependency to v18. diff --git a/packages/i18n-calypso/README.md b/packages/i18n-calypso/README.md index c5a3e46604eb7..0ea397fc44a35 100644 --- a/packages/i18n-calypso/README.md +++ b/packages/i18n-calypso/README.md @@ -219,10 +219,30 @@ i18n.numberFormat( 2500.1, 2 ); // '2.500,10' i18n.numberFormat( 2500.33, { decimals: 3, thousandsSep: '*', decPoint: '@' } ); // '2*500@330' ``` +## fixMe Method + +Using the method `fixMe` you can apply a translation (`newCopy` parameter) only when a certain text has been translated or the locale is English (`en`, `en-gb`), otherwise apply the fallback translation (`oldCopy` parameter). This is a common pattern in the codebase for new translations that we need to ship straight away, and the function is meant to save a few inline conditions. + +Important: Due to the way our string extraction mechanism works, always pass `i18n.translate( '...' )` for `newCopy` parameter as otherwise the new string will not be picked up for translation. + +### Usage + +```js +const i18n = require( 'i18n-calypso' ); + +i18n.fixMe( { + text: 'new copy', + newCopy: i18n.translate( 'new copy' ), + oldCopy: i18n.translate( 'old copy' ), +} ); +``` + ## hasTranslation Method Using the method `hasTranslation` you can check whether a translation for a given string exists. As the `translate()` function will always return screen text that can be displayed (will supply the source text if no translation exists), it is unsuitable to determine whether text is translated. Other factors are optional [key hashing](#key-hashing) as well as purposeful translation to the source text. +Important: For most current usages in Calypso, where we conditionally render a new copy when a translation exists (otherwise a fallback), it's best to use `fixMe` method instead (above). It encapsulates a few of the conditions that would otherwise be necessary. + ### Usage ```js diff --git a/packages/i18n-calypso/package.json b/packages/i18n-calypso/package.json index ee13a3bb67772..89a75bdec12af 100644 --- a/packages/i18n-calypso/package.json +++ b/packages/i18n-calypso/package.json @@ -1,6 +1,6 @@ { "name": "i18n-calypso", - "version": "7.0.0", + "version": "7.1.0", "description": "I18n JavaScript library on top of Tannin originally used in Calypso.", "main": "dist/cjs/index.js", "module": "dist/esm/index.js", diff --git a/packages/i18n-calypso/src/i18n.js b/packages/i18n-calypso/src/i18n.js index 82e4015441995..1ce449aae506b 100644 --- a/packages/i18n-calypso/src/i18n.js +++ b/packages/i18n-calypso/src/i18n.js @@ -345,6 +345,33 @@ I18N.prototype.hasTranslation = function () { return !! getTranslation( this, normalizeTranslateArguments( arguments ) ); }; +/** + * Returns `newCopy` if given `text` is translated or locale is English, otherwise returns the `oldCopy`. + * ------------------ + * Important - Usage: + * ------------------ + * `newCopy` prop should be an actual `i18n.translate()` call from the consuming end. + * This is the only way currently to ensure that it is picked up by our string extraction mechanism + * and propagate into GlotPress for translation. + * ------------------ + * @param {Object} options + * @param {string} options.text - The text to check for translation. + * @param {string | Object} options.newCopy - The translation to return if the text is translated. + * @param {string | Object | undefined } options.oldCopy - The fallback to return if the text is not translated. + */ +I18N.prototype.fixMe = function ( { text, newCopy, oldCopy } ) { + if ( typeof text !== 'string' ) { + warn( 'fixMe() requires an object with a proper text property (string)' ); + return null; + } + + if ( [ 'en', 'en-gb' ].includes( this.getLocaleSlug() ) || this.hasTranslation( text ) ) { + return newCopy; + } + + return oldCopy; +}; + /** * Exposes single translation method. * See sibling README diff --git a/packages/i18n-calypso/src/index.js b/packages/i18n-calypso/src/index.js index 37a5b04355a4b..a9c953c7d7d36 100644 --- a/packages/i18n-calypso/src/index.js +++ b/packages/i18n-calypso/src/index.js @@ -27,3 +27,4 @@ export const stateObserver = i18n.stateObserver; export const on = i18n.on.bind( i18n ); export const off = i18n.off.bind( i18n ); export const emit = i18n.emit.bind( i18n ); +export const fixMe = i18n.fixMe.bind( i18n ); diff --git a/packages/i18n-calypso/test/index.js b/packages/i18n-calypso/test/index.js index bac8ec9451583..1adf9fdb81898 100644 --- a/packages/i18n-calypso/test/index.js +++ b/packages/i18n-calypso/test/index.js @@ -19,6 +19,7 @@ describe( 'I18n', function () { } ); afterEach( function () { + jest.clearAllMocks(); i18n.configure(); // ensure everything is reset } ); @@ -282,4 +283,52 @@ describe( 'I18n', function () { expect( translate( 'green', { context: 'color' } ) ).toBe( 'cursus' ); } ); } ); + + describe( 'fixMe', () => { + it( 'should return null if text is missing or wrong type', () => { + const result = i18n.fixMe( {} ); + expect( result ).toBe( null ); + } ); + + it( 'should return newCopy if locale is en', () => { + i18n.getLocaleSlug = jest.fn().mockReturnValue( 'en' ); + const result = i18n.fixMe( { + text: 'hello', + newCopy: 'hello', + oldCopy: 'hi', + } ); + expect( result ).toBe( 'hello' ); + } ); + + it( 'should return newCopy if locale is en-gb', () => { + i18n.getLocaleSlug = jest.fn().mockReturnValue( 'en-gb' ); + const result = i18n.fixMe( { + text: 'hello', + newCopy: 'hello', + oldCopy: 'hi', + } ); + expect( result ).toBe( 'hello' ); + } ); + + it( 'should return newCopy if text has a translation', () => { + i18n.hasTranslation = jest.fn().mockReturnValue( true ); + const result = i18n.fixMe( { + text: 'hello', + newCopy: 'bonjour', + oldCopy: 'hi', + } ); + expect( result ).toBe( 'bonjour' ); + } ); + + it( 'should return oldCopy if text does not have a translation and locale is not English', () => { + i18n.getLocaleSlug = jest.fn().mockReturnValue( 'fr' ); + i18n.hasTranslation = jest.fn().mockReturnValue( false ); + const result = i18n.fixMe( { + text: 'hello', + newCopy: 'bonjour', + oldCopy: 'hi', + } ); + expect( result ).toBe( 'hi' ); + } ); + } ); } );