From bc41cc574442b69a783d4e6cb62baa8f6f1ec499 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 9 Dec 2021 18:19:56 +0100 Subject: [PATCH 1/6] add lang attribute to fallback translations Signed-off-by: Kerry Archibald --- __mocks__/browser-request.js | 12 +- src/languageHandler.tsx | 70 +++++++--- test/i18n-test/languageHandler-test.js | 179 +++++++++++++++++-------- 3 files changed, 186 insertions(+), 75 deletions(-) diff --git a/__mocks__/browser-request.js b/__mocks__/browser-request.js index 4c59e8a43ae..7e26849f28a 100644 --- a/__mocks__/browser-request.js +++ b/__mocks__/browser-request.js @@ -1,10 +1,14 @@ const en = require("../src/i18n/strings/en_EN"); const de = require("../src/i18n/strings/de_DE"); +const lv = { + "Save": "Saglabāt", +}; // Mock the browser-request for the languageHandler tests to return -// Fake languages.json containing references to en_EN and de_DE +// Fake languages.json containing references to en_EN, de_DE and lv // en_EN.json // de_DE.json +// lv.json - mock version with few translations, used to test fallback translation module.exports = jest.fn((opts, cb) => { const url = opts.url || opts.uri; if (url && url.endsWith("languages.json")) { @@ -17,11 +21,17 @@ module.exports = jest.fn((opts, cb) => { "fileName": "de_DE.json", "label": "German", }, + "lv": { + "fileName": "lv.json", + "label": "Latvian" + } })); } else if (url && url.endsWith("en_EN.json")) { cb(undefined, {status: 200}, JSON.stringify(en)); } else if (url && url.endsWith("de_DE.json")) { cb(undefined, {status: 200}, JSON.stringify(de)); + } else if (url && url.endsWith("lv.json")) { + cb(undefined, {status: 200}, JSON.stringify(lv)); } else { cb(true, {status: 404}, ""); } diff --git a/src/languageHandler.tsx b/src/languageHandler.tsx index d5d20e5181b..9e47e563879 100644 --- a/src/languageHandler.tsx +++ b/src/languageHandler.tsx @@ -38,8 +38,9 @@ const ANNOTATE_STRINGS = false; // We use english strings as keys, some of which contain full stops counterpart.setSeparator('|'); -// Fall back to English -counterpart.setFallbackLocale('en'); + +// see `translateWithFallback` for an explanation of fallback handling +const FALLBACK_LOCALE = 'en'; interface ITranslatableError extends Error { translatedMessage: string; @@ -53,7 +54,7 @@ interface ITranslatableError extends Error { */ export function newTranslatableError(message: string) { const error = new Error(message) as ITranslatableError; - error.translatedMessage = _t(message); + error.translatedMessage = _tPlain(message); return error; } @@ -72,6 +73,24 @@ export function _td(s: string): string { // eslint-disable-line @typescript-esli return s; } +/** + * to improve screen reader experience translations that are not in the main page language + * eg a translation that fell back to english from another language + * should be wrapped with an appropriate `lang='en'` attribute + * counterpart's `translate` doesn't expose a way to determine if the resulting translation + * is in the target locale or a fallback locale + * for this reason, we do not set a fallback via `counterpart.setFallbackLocale` + * and fallback 'manually' so we can mark fallback strings appropriately + * */ +const translateWithFallback = (text: string, options?: object): { translated?: string, isFallback?: boolean } => { + const translated = counterpart.translate(text, options); + if (!/^missing translation:/.test(translated)) { + return { translated }; + } + const fallbackTranslated = counterpart.translate(text, { ...options, fallbackLocale: FALLBACK_LOCALE }); + return { translated: fallbackTranslated, isFallback: true }; +}; + // Wrapper for counterpart's translation function so that it handles nulls and undefineds properly // Takes the same arguments as counterpart.translate() function safeCounterpartTranslate(text: string, options?: object) { @@ -82,10 +101,8 @@ function safeCounterpartTranslate(text: string, options?: object) { // valid ES6 template strings to i18n strings it's extremely easy to pass undefined/null // if there are no existing null guards. To avoid this making the app completely inoperable, // we'll check all the values for undefined/null and stringify them here. - let count; if (options && typeof options === 'object') { - count = options['count']; Object.keys(options).forEach((k) => { if (options[k] === undefined) { logger.warn("safeCounterpartTranslate called with undefined interpolation name: " + k); @@ -97,13 +114,7 @@ function safeCounterpartTranslate(text: string, options?: object) { } }); } - let translated = counterpart.translate(text, options); - if (translated === undefined && count !== undefined) { - // counterpart does not do fallback if no pluralisation exists - // in the preferred language, so do it here - translated = counterpart.translate(text, Object.assign({}, options, { locale: 'en' })); - } - return translated; + return translateWithFallback(text, options); } type SubstitutionValue = number | string | React.ReactNode | ((sub: string) => React.ReactNode); @@ -117,6 +128,26 @@ export type Tags = Record; export type TranslatedString = string | React.ReactNode; +/** + * Translate text without tag substitions or fallback span wrapper + * Useful for translations that will not be displayed in the DOM, eg: error messages + * @param {string} text The untranslated text, e.g "click here now to %(foo)s". + * @param {object} variables Variable substitutions, e.g { foo: 'bar' } + */ +export const _tPlain = (text: string, variables?: IVariables): string => { + // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components + // However, still pass the variables to counterpart so that it can choose the correct plural if count is given + // It is enough to pass the count variable, but in the future counterpart might make use of other information too + const args = Object.assign({ interpolate: false }, variables); + + // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution) + const { translated } = safeCounterpartTranslate(text, args); + + // For development/testing purposes it is useful to also output the original string + // Don't do that for release versions + return ANNOTATE_STRINGS ? `@@${text}##${translated}@@` : translated; +}; + /* * Translates text and optionally also replaces XML-ish elements in the text with e.g. React components * @param {string} text The untranslated text, e.g "click here now to %(foo)s". @@ -135,7 +166,7 @@ export type TranslatedString = string | React.ReactNode; */ // eslint-next-line @typescript-eslint/naming-convention // eslint-nexline @typescript-eslint/naming-convention -export function _t(text: string, variables?: IVariables): string; +export function _t(text: string, variables?: IVariables): string | React.ReactNode; export function _t(text: string, variables: IVariables, tags: Tags): React.ReactNode; export function _t(text: string, variables?: IVariables, tags?: Tags): TranslatedString { // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components @@ -144,21 +175,24 @@ export function _t(text: string, variables?: IVariables, tags?: Tags): Translate const args = Object.assign({ interpolate: false }, variables); // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution) - const translated = safeCounterpartTranslate(text, args); + const { translated, isFallback } = safeCounterpartTranslate(text, args); const substituted = substitute(translated, variables, tags); + // wrap en fallback translation with lang attribute for screen readers + const result = isFallback ? { substituted } : substituted; + // For development/testing purposes it is useful to also output the original string // Don't do that for release versions if (ANNOTATE_STRINGS) { - if (typeof substituted === 'string') { - return `@@${text}##${substituted}@@`; + if (typeof result === 'string') { + return `@@${text}##${result}@@`; } else { return { substituted }; } - } else { - return substituted; } + + return result; } /** diff --git a/test/i18n-test/languageHandler-test.js b/test/i18n-test/languageHandler-test.js index 70b2fd169dc..2aa2a685de9 100644 --- a/test/i18n-test/languageHandler-test.js +++ b/test/i18n-test/languageHandler-test.js @@ -6,74 +6,141 @@ const expect = require('expect'); const testUtils = require('../test-utils'); describe('languageHandler', function() { - beforeEach(function(done) { - testUtils.stubClient(); + const selfClosingTagSub = 'Accept to continue:'; + const textInTagSub = 'Upgrade to your own domain'; + const plurals = 'and %(count)s others...'; + const variableSub = 'You are now ignoring %(userId)s'; - languageHandler.setLanguage('en').then(done); - languageHandler.setMissingEntryGenerator(key => key.split("|", 2)[1]); - }); + describe('when translations exist in language', () => { + beforeEach(function(done) { + testUtils.stubClient(); - it('translates a string to german', function(done) { - languageHandler.setLanguage('de').then(function() { - const translated = languageHandler._t('Rooms'); - expect(translated).toBe('Räume'); - }).then(done); - }); + languageHandler.setLanguage('en').then(done); + languageHandler.setMissingEntryGenerator(key => key.split("|", 2)[1]); + }); - it('handles plurals', function() { - const text = 'and %(count)s others...'; - expect(languageHandler._t(text, { count: 1 })).toBe('and one other...'); - expect(languageHandler._t(text, { count: 2 })).toBe('and 2 others...'); - }); + it('translates a string to german', function(done) { + languageHandler.setLanguage('de').then(function() { + const translated = languageHandler._t('Rooms'); + expect(translated).toBe('Räume'); + }).then(done); + }); - it('handles simple variable subsitutions', function() { - const text = 'You are now ignoring %(userId)s'; - expect(languageHandler._t(text, { userId: 'foo' })).toBe('You are now ignoring foo'); - }); + it('handles plurals', function() { + const text = plurals; + expect(languageHandler._t(text, { count: 1 })).toBe('and one other...'); + expect(languageHandler._t(text, { count: 2 })).toBe('and 2 others...'); + }); - it('handles simple tag substitution', function() { - const text = 'Press to start a chat with someone'; - expect(languageHandler._t(text, {}, { 'StartChatButton': () => 'foo' })) - .toBe('Press foo to start a chat with someone'); - }); + it('handles simple variable subsitutions', function() { + const text = 'You are now ignoring %(userId)s'; + expect(languageHandler._t(text, { userId: 'foo' })).toBe('You are now ignoring foo'); + }); - it('handles text in tags', function() { - const text = 'Click here to join the discussion!'; - expect(languageHandler._t(text, {}, { 'a': (sub) => `x${sub}x` })) - .toBe('xClick herex to join the discussion!'); - }); + it('handles simple tag substitution', function() { + const text = selfClosingTagSub; + expect(languageHandler._t(text, {}, { 'policyLink': () => 'foo' })) + .toBe('Accept foo to continue:'); + }); - it('variable substitution with React component', function() { - const text = 'You are now ignoring %(userId)s'; - expect(languageHandler._t(text, { userId: () => foo })) - .toEqual((You are now ignoring foo)); - }); + it('handles text in tags', function() { + const text = textInTagSub; + expect(languageHandler._t(text, {}, { 'a': (sub) => `x${sub}x` })) + .toBe('xUpgradex to your own domain'); + }); - it('variable substitution with plain React component', function() { - const text = 'You are now ignoring %(userId)s'; - expect(languageHandler._t(text, { userId: foo })) - .toEqual((You are now ignoring foo)); - }); + it('variable substitution with React component', function() { + const text = variableSub; + expect(languageHandler._t(text, { userId: () => foo })) + .toEqual((You are now ignoring foo)); + }); - it('tag substitution with React component', function() { - const text = 'Press to start a chat with someone'; - expect(languageHandler._t(text, {}, { 'StartChatButton': () => foo })) - .toEqual(Press foo to start a chat with someone); - }); + it('variable substitution with plain React component', function() { + const text = variableSub; + expect(languageHandler._t(text, { userId: foo })) + .toEqual((You are now ignoring foo)); + }); - it('replacements in the wrong order', function() { - const text = '%(var1)s %(var2)s'; - expect(languageHandler._t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2'); - }); + it('tag substitution with React component', function() { + const text = selfClosingTagSub; + expect(languageHandler._t(text, {}, { 'policyLink': () => foo })) + .toEqual(Accept foo to continue:); + }); - it('multiple replacements of the same variable', function() { - const text = '%(var1)s %(var1)s'; - expect(languageHandler.substitute(text, { var1: 'val1' })).toBe('val1 val1'); + it('replacements in the wrong order', function() { + const text = '%(var1)s %(var2)s'; + expect(languageHandler._t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2'); + }); + + it('multiple replacements of the same variable', function() { + const text = '%(var1)s %(var1)s'; + expect(languageHandler.substitute(text, { var1: 'val1' })).toBe('val1 val1'); + }); + + it('multiple replacements of the same tag', function() { + const text = 'Click here to join the discussion! or here'; + expect(languageHandler.substitute(text, {}, { 'a': (sub) => `x${sub}x` })) + .toBe('xClick herex to join the discussion! xor herex'); + }); }); - it('multiple replacements of the same tag', function() { - const text = 'Click here to join the discussion! or here'; - expect(languageHandler.substitute(text, {}, { 'a': (sub) => `x${sub}x` })) - .toBe('xClick herex to join the discussion! xor herex'); + describe('when a translation string does not exist in active language', () => { + beforeEach(async () => { + testUtils.stubClient(); + await languageHandler.setLanguage('lv'); + // counterpart doesnt expose any way to restore default config + // missingEntryGenerator is mocked in the root setup file + // reset to default here + const counterpartDefaultMissingEntryGen = function(key) { return 'missing translation: ' + key; }; + languageHandler.setMissingEntryGenerator(counterpartDefaultMissingEntryGen); + }); + + it('marks basic fallback translations lang attribute', () => { + const translated = languageHandler._t('Rooms'); + expect(translated).toEqual(Rooms); + }); + + it('handles plurals', function() { + const text = plurals; + expect(languageHandler._t(text, { count: 1 })).toEqual(and one other...); + expect(languageHandler._t(text, { count: 2 })).toEqual(and 2 others...); + }); + + it('handles simple variable subsitutions', function() { + const text = 'You are now ignoring %(userId)s'; + expect(languageHandler._t(text, { userId: 'foo' })).toEqual( + You are now ignoring foo, + ); + }); + + it('handles simple tag substitution', function() { + const text = selfClosingTagSub; + expect(languageHandler._t(text, {}, { 'policyLink': () => 'foo' })) + .toEqual(Accept foo to continue:); + }); + + it('handles text in tags', function() { + const text = textInTagSub; + expect(languageHandler._t(text, {}, { 'a': (sub) => `x${sub}x` })) + .toEqual(xUpgradex to your own domain); + }); + + it('variable substitution with React component', function() { + const text = variableSub; + expect(languageHandler._t(text, { userId: () => foo })) + .toEqual((You are now ignoring foo)); + }); + + it('variable substitution with plain React component', function() { + const text = variableSub; + expect(languageHandler._t(text, { userId: foo })) + .toEqual((You are now ignoring foo)); + }); + + it('tag substitution with React component', function() { + const text = selfClosingTagSub; + expect(languageHandler._t(text, {}, { 'policyLink': () => foo })) + .toEqual(Accept foo to continue:); + }); }); }); From 314080908c65d93dd449417fb4603aa86f3fb81f Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 9 Dec 2021 18:22:35 +0100 Subject: [PATCH 2/6] readability improvement Signed-off-by: Kerry Archibald --- src/languageHandler.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/languageHandler.tsx b/src/languageHandler.tsx index 9e47e563879..86567ae9971 100644 --- a/src/languageHandler.tsx +++ b/src/languageHandler.tsx @@ -84,11 +84,11 @@ export function _td(s: string): string { // eslint-disable-line @typescript-esli * */ const translateWithFallback = (text: string, options?: object): { translated?: string, isFallback?: boolean } => { const translated = counterpart.translate(text, options); - if (!/^missing translation:/.test(translated)) { - return { translated }; + if (/^missing translation:/.test(translated)) { + const fallbackTranslated = counterpart.translate(text, { ...options, fallbackLocale: FALLBACK_LOCALE }); + return { translated: fallbackTranslated, isFallback: true }; } - const fallbackTranslated = counterpart.translate(text, { ...options, fallbackLocale: FALLBACK_LOCALE }); - return { translated: fallbackTranslated, isFallback: true }; + return { translated }; }; // Wrapper for counterpart's translation function so that it handles nulls and undefineds properly From 7e6fbb937ef72bbd7dc4cd005ee818e87ae90e67 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 10 Dec 2021 15:27:07 +0100 Subject: [PATCH 3/6] split _t and _tDom Signed-off-by: Kerry --- src/languageHandler.tsx | 83 +++++++------- test/i18n-test/languageHandler-test.js | 146 ------------------------ test/i18n-test/languageHandler-test.tsx | 87 ++++++++++++++ 3 files changed, 130 insertions(+), 186 deletions(-) delete mode 100644 test/i18n-test/languageHandler-test.js create mode 100644 test/i18n-test/languageHandler-test.tsx diff --git a/src/languageHandler.tsx b/src/languageHandler.tsx index 86567ae9971..61c4aaf9f25 100644 --- a/src/languageHandler.tsx +++ b/src/languageHandler.tsx @@ -54,7 +54,7 @@ interface ITranslatableError extends Error { */ export function newTranslatableError(message: string) { const error = new Error(message) as ITranslatableError; - error.translatedMessage = _tPlain(message); + error.translatedMessage = _t(message); return error; } @@ -93,7 +93,12 @@ const translateWithFallback = (text: string, options?: object): { translated?: s // Wrapper for counterpart's translation function so that it handles nulls and undefineds properly // Takes the same arguments as counterpart.translate() -function safeCounterpartTranslate(text: string, options?: object) { +function safeCounterpartTranslate(text: string, variables?: object) { + // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components + // However, still pass the variables to counterpart so that it can choose the correct plural if count is given + // It is enough to pass the count variable, but in the future counterpart might make use of other information too + const options = { ...variables, interpolate: false }; + // Horrible hack to avoid https://github.com/vector-im/element-web/issues/4191 // The interpolation library that counterpart uses does not support undefined/null // values and instead will throw an error. This is a problem since everywhere else @@ -101,7 +106,6 @@ function safeCounterpartTranslate(text: string, options?: object) { // valid ES6 template strings to i18n strings it's extremely easy to pass undefined/null // if there are no existing null guards. To avoid this making the app completely inoperable, // we'll check all the values for undefined/null and stringify them here. - if (options && typeof options === 'object') { Object.keys(options).forEach((k) => { if (options[k] === undefined) { @@ -128,25 +132,19 @@ export type Tags = Record; export type TranslatedString = string | React.ReactNode; -/** - * Translate text without tag substitions or fallback span wrapper - * Useful for translations that will not be displayed in the DOM, eg: error messages - * @param {string} text The untranslated text, e.g "click here now to %(foo)s". - * @param {object} variables Variable substitutions, e.g { foo: 'bar' } - */ -export const _tPlain = (text: string, variables?: IVariables): string => { - // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components - // However, still pass the variables to counterpart so that it can choose the correct plural if count is given - // It is enough to pass the count variable, but in the future counterpart might make use of other information too - const args = Object.assign({ interpolate: false }, variables); - - // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution) - const { translated } = safeCounterpartTranslate(text, args); +// For development/testing purposes it is useful to also output the original string +// Don't do that for release versions +const annotateStrings = (result: TranslatedString, translationKey: string): TranslatedString => { + if (!ANNOTATE_STRINGS) { + return result; + } - // For development/testing purposes it is useful to also output the original string - // Don't do that for release versions - return ANNOTATE_STRINGS ? `@@${text}##${translated}@@` : translated; -}; + if (typeof result === 'string') { + return `@@${translationKey}##${result}@@`; + } else { + return {result}; + } +} /* * Translates text and optionally also replaces XML-ish elements in the text with e.g. React components @@ -165,34 +163,39 @@ export const _tPlain = (text: string, variables?: IVariables): string => { * @return a React component if any non-strings were used in substitutions, otherwise a string */ // eslint-next-line @typescript-eslint/naming-convention -// eslint-nexline @typescript-eslint/naming-convention -export function _t(text: string, variables?: IVariables): string | React.ReactNode; +export function _t(text: string, variables?: IVariables): string; export function _t(text: string, variables: IVariables, tags: Tags): React.ReactNode; export function _t(text: string, variables?: IVariables, tags?: Tags): TranslatedString { - // Don't do substitutions in counterpart. We handle it ourselves so we can replace with React components - // However, still pass the variables to counterpart so that it can choose the correct plural if count is given - // It is enough to pass the count variable, but in the future counterpart might make use of other information too - const args = Object.assign({ interpolate: false }, variables); - // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution) - const { translated, isFallback } = safeCounterpartTranslate(text, args); + const { translated } = safeCounterpartTranslate(text, variables); const substituted = substitute(translated, variables, tags); + return annotateStrings(substituted, text); +} + +/* + * Wraps normal _t function and adds atttribution for translations that used a fallback locale + * Wraps translations that fell back from active locale to fallback locale with a `>` + * @param {string} text The untranslated text, e.g "click here now to %(foo)s". + * @param {object} variables Variable substitutions, e.g { foo: 'bar' } + * @param {object} tags Tag substitutions e.g. { 'a': (sub) => {sub} } + * + * @return a React component if any non-strings were used in substitutions + * or translation used a fallback locale, otherwise a string + */ +// eslint-next-line @typescript-eslint/naming-convention +export function _tDom(text: string, variables?: IVariables): TranslatedString; +export function _tDom(text: string, variables: IVariables, tags: Tags): React.ReactNode; +export function _tDom(text: string, variables?: IVariables, tags?: Tags): TranslatedString { + // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution) + const { translated, isFallback } = safeCounterpartTranslate(text, variables); + const substituted = substitute(translated, variables, tags); + // wrap en fallback translation with lang attribute for screen readers const result = isFallback ? { substituted } : substituted; - // For development/testing purposes it is useful to also output the original string - // Don't do that for release versions - if (ANNOTATE_STRINGS) { - if (typeof result === 'string') { - return `@@${text}##${result}@@`; - } else { - return { substituted }; - } - } - - return result; + return annotateStrings(result, text); } /** diff --git a/test/i18n-test/languageHandler-test.js b/test/i18n-test/languageHandler-test.js deleted file mode 100644 index 2aa2a685de9..00000000000 --- a/test/i18n-test/languageHandler-test.js +++ /dev/null @@ -1,146 +0,0 @@ -import * as languageHandler from '../../src/languageHandler'; - -const React = require('react'); -const expect = require('expect'); - -const testUtils = require('../test-utils'); - -describe('languageHandler', function() { - const selfClosingTagSub = 'Accept to continue:'; - const textInTagSub = 'Upgrade to your own domain'; - const plurals = 'and %(count)s others...'; - const variableSub = 'You are now ignoring %(userId)s'; - - describe('when translations exist in language', () => { - beforeEach(function(done) { - testUtils.stubClient(); - - languageHandler.setLanguage('en').then(done); - languageHandler.setMissingEntryGenerator(key => key.split("|", 2)[1]); - }); - - it('translates a string to german', function(done) { - languageHandler.setLanguage('de').then(function() { - const translated = languageHandler._t('Rooms'); - expect(translated).toBe('Räume'); - }).then(done); - }); - - it('handles plurals', function() { - const text = plurals; - expect(languageHandler._t(text, { count: 1 })).toBe('and one other...'); - expect(languageHandler._t(text, { count: 2 })).toBe('and 2 others...'); - }); - - it('handles simple variable subsitutions', function() { - const text = 'You are now ignoring %(userId)s'; - expect(languageHandler._t(text, { userId: 'foo' })).toBe('You are now ignoring foo'); - }); - - it('handles simple tag substitution', function() { - const text = selfClosingTagSub; - expect(languageHandler._t(text, {}, { 'policyLink': () => 'foo' })) - .toBe('Accept foo to continue:'); - }); - - it('handles text in tags', function() { - const text = textInTagSub; - expect(languageHandler._t(text, {}, { 'a': (sub) => `x${sub}x` })) - .toBe('xUpgradex to your own domain'); - }); - - it('variable substitution with React component', function() { - const text = variableSub; - expect(languageHandler._t(text, { userId: () => foo })) - .toEqual((You are now ignoring foo)); - }); - - it('variable substitution with plain React component', function() { - const text = variableSub; - expect(languageHandler._t(text, { userId: foo })) - .toEqual((You are now ignoring foo)); - }); - - it('tag substitution with React component', function() { - const text = selfClosingTagSub; - expect(languageHandler._t(text, {}, { 'policyLink': () => foo })) - .toEqual(Accept foo to continue:); - }); - - it('replacements in the wrong order', function() { - const text = '%(var1)s %(var2)s'; - expect(languageHandler._t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2'); - }); - - it('multiple replacements of the same variable', function() { - const text = '%(var1)s %(var1)s'; - expect(languageHandler.substitute(text, { var1: 'val1' })).toBe('val1 val1'); - }); - - it('multiple replacements of the same tag', function() { - const text = 'Click here to join the discussion! or here'; - expect(languageHandler.substitute(text, {}, { 'a': (sub) => `x${sub}x` })) - .toBe('xClick herex to join the discussion! xor herex'); - }); - }); - - describe('when a translation string does not exist in active language', () => { - beforeEach(async () => { - testUtils.stubClient(); - await languageHandler.setLanguage('lv'); - // counterpart doesnt expose any way to restore default config - // missingEntryGenerator is mocked in the root setup file - // reset to default here - const counterpartDefaultMissingEntryGen = function(key) { return 'missing translation: ' + key; }; - languageHandler.setMissingEntryGenerator(counterpartDefaultMissingEntryGen); - }); - - it('marks basic fallback translations lang attribute', () => { - const translated = languageHandler._t('Rooms'); - expect(translated).toEqual(Rooms); - }); - - it('handles plurals', function() { - const text = plurals; - expect(languageHandler._t(text, { count: 1 })).toEqual(and one other...); - expect(languageHandler._t(text, { count: 2 })).toEqual(and 2 others...); - }); - - it('handles simple variable subsitutions', function() { - const text = 'You are now ignoring %(userId)s'; - expect(languageHandler._t(text, { userId: 'foo' })).toEqual( - You are now ignoring foo, - ); - }); - - it('handles simple tag substitution', function() { - const text = selfClosingTagSub; - expect(languageHandler._t(text, {}, { 'policyLink': () => 'foo' })) - .toEqual(Accept foo to continue:); - }); - - it('handles text in tags', function() { - const text = textInTagSub; - expect(languageHandler._t(text, {}, { 'a': (sub) => `x${sub}x` })) - .toEqual(xUpgradex to your own domain); - }); - - it('variable substitution with React component', function() { - const text = variableSub; - expect(languageHandler._t(text, { userId: () => foo })) - .toEqual((You are now ignoring foo)); - }); - - it('variable substitution with plain React component', function() { - const text = variableSub; - expect(languageHandler._t(text, { userId: foo })) - .toEqual((You are now ignoring foo)); - }); - - it('tag substitution with React component', function() { - const text = selfClosingTagSub; - expect(languageHandler._t(text, {}, { 'policyLink': () => foo })) - .toEqual(Accept foo to continue:); - }); - }); -}); diff --git a/test/i18n-test/languageHandler-test.tsx b/test/i18n-test/languageHandler-test.tsx new file mode 100644 index 00000000000..b34235c7dbc --- /dev/null +++ b/test/i18n-test/languageHandler-test.tsx @@ -0,0 +1,87 @@ +import { _t, _tDom, TranslatedString, setLanguage, setMissingEntryGenerator, substitute } from '../../src/languageHandler'; + +const React = require('react'); +const expect = require('expect'); + +const testUtils = require('../test-utils'); + +describe('languageHandler', function () { + const basicString = 'Rooms'; + const selfClosingTagSub = 'Accept to continue:'; + const textInTagSub = 'Upgrade to your own domain'; + const plurals = 'and %(count)s others...'; + const variableSub = 'You are now ignoring %(userId)s'; + + type TestCase = [string, string, Record, Record, TranslatedString] + const testCasesEn: TestCase[] = [ + ['translates a basic string', basicString, {}, undefined, 'Rooms'], + ['handles plurals when count is 1', plurals, { count: 1 }, undefined, 'and one other...'], + ['handles plurals when count is not 1', plurals, { count: 2 }, undefined, 'and 2 others...'], + ['handles simple variable substitution', variableSub, { userId: 'foo' }, undefined, 'You are now ignoring foo'], + ['handles simple tag substitution', selfClosingTagSub, {}, { 'policyLink': () => 'foo' }, 'Accept foo to continue:'], + ['handles text in tags', textInTagSub, {}, { 'a': (sub) => `x${sub}x` }, 'xUpgradex to your own domain'], + ['handles variable substitution with React function component', variableSub, { userId: () => foo }, undefined, You are now ignoring foo], + ['handles variable substitution with react node', variableSub, { userId: foo }, undefined, You are now ignoring foo], + ['handles tag substitution with React function component', selfClosingTagSub, {}, { 'policyLink': () => foo }, Accept foo to continue:], + ]; + + describe('when translations exist in language', () => { + beforeEach(function (done) { + testUtils.stubClient(); + + setLanguage('en').then(done); + setMissingEntryGenerator(key => key.split("|", 2)[1]); + }); + + it('translates a string to german', function (done) { + setLanguage('de').then(function () { + const translated = _t(basicString); + expect(translated).toBe('Räume'); + }).then(done); + }); + + it.each(testCasesEn)("%s", async (_d, translationString, variables, tags, result) => { + expect(_t(translationString, variables, tags)).toEqual(result); + }); + + it('replacements in the wrong order', function () { + const text = '%(var1)s %(var2)s'; + expect(_t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2'); + }); + + it('multiple replacements of the same variable', function () { + const text = '%(var1)s %(var1)s'; + expect(substitute(text, { var1: 'val1' })).toBe('val1 val1'); + }); + + it('multiple replacements of the same tag', function () { + const text = 'Click here to join the discussion! or here'; + expect(substitute(text, {}, { 'a': (sub) => `x${sub}x` })) + .toBe('xClick herex to join the discussion! xor herex'); + }); + }); + + describe('when a translation string does not exist in active language', () => { + beforeEach(async () => { + testUtils.stubClient(); + await setLanguage('lv'); + // counterpart doesnt expose any way to restore default config + // missingEntryGenerator is mocked in the root setup file + // reset to default here + const counterpartDefaultMissingEntryGen = function (key) { return 'missing translation: ' + key; }; + setMissingEntryGenerator(counterpartDefaultMissingEntryGen); + }); + + describe('_t', () => { + it.each(testCasesEn)("%s and translates with fallback locale", async (_d, translationString, variables, tags, result) => { + expect(_t(translationString, variables, tags)).toEqual(result); + }); + }) + + describe('_tDom()', () => { + it.each(testCasesEn)("%s and translates with fallback locale, attributes fallback locale", async (_d, translationString, variables, tags, result) => { + expect(_tDom(translationString, variables, tags)).toEqual({result}); + }); + }) + }); +}); From 323a3cd820a6e4774eb9fb4ee8a878f0f67a8770 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 10 Dec 2021 15:33:05 +0100 Subject: [PATCH 4/6] use tDom in HomePage Signed-off-by: Kerry Archibald --- src/components/structures/HomePage.tsx | 20 +++++++++---------- .../views/elements/MiniAvatarUploader.tsx | 5 +++-- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/components/structures/HomePage.tsx b/src/components/structures/HomePage.tsx index 9c8013f866e..137ca799ccc 100644 --- a/src/components/structures/HomePage.tsx +++ b/src/components/structures/HomePage.tsx @@ -19,7 +19,7 @@ import { useContext, useState } from "react"; import AutoHideScrollbar from './AutoHideScrollbar'; import { getHomePageUrl } from "../../utils/pages"; -import { _t } from "../../languageHandler"; +import { _tDom } from "../../languageHandler"; import SdkConfig from "../../SdkConfig"; import * as sdk from "../../index"; import dis from "../../dispatcher/dispatcher"; @@ -72,8 +72,8 @@ const UserWelcomeTop = () => { return
cli.setAvatarUrl(url)} > { /> -

{ _t("Welcome %(name)s", { name: ownProfile.displayName }) }

-

{ _t("Now, let's help you get started") }

+

{_tDom("Welcome %(name)s", { name: ownProfile.displayName })}

+

{_tDom("Now, let's help you get started")}

; }; @@ -113,8 +113,8 @@ const HomePage: React.FC = ({ justRegistered = false }) => { introSection = {config.brand} -

{ _t("Welcome to %(appName)s", { appName: config.brand }) }

-

{ _t("Own your conversations.") }

+

{_tDom("Welcome to %(appName)s", { appName: config.brand })}

+

{_tDom("Own your conversations.")}

; } @@ -123,13 +123,13 @@ const HomePage: React.FC = ({ justRegistered = false }) => { { introSection }
- { _t("Send a Direct Message") } + {_tDom("Send a Direct Message")} - { _t("Explore Public Rooms") } + {_tDom("Explore Public Rooms")} - { _t("Create a Group Chat") } + {_tDom("Create a Group Chat")}
diff --git a/src/components/views/elements/MiniAvatarUploader.tsx b/src/components/views/elements/MiniAvatarUploader.tsx index 22ff4bf4b32..837433e0137 100644 --- a/src/components/views/elements/MiniAvatarUploader.tsx +++ b/src/components/views/elements/MiniAvatarUploader.tsx @@ -24,14 +24,15 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext"; import { useTimeout } from "../../../hooks/useTimeout"; import Analytics from "../../../Analytics"; import CountlyAnalytics from '../../../CountlyAnalytics'; +import { TranslatedString } from '../../../languageHandler'; import RoomContext from "../../../contexts/RoomContext"; export const AVATAR_SIZE = 52; interface IProps { hasAvatar: boolean; - noAvatarLabel?: string; - hasAvatarLabel?: string; + noAvatarLabel?: TranslatedString; + hasAvatarLabel?: TranslatedString; setAvatarUrl(url: string): Promise; } From cbed25c1418ba96223fa9aadb2889dda92a5d5a2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 10 Dec 2021 15:49:09 +0100 Subject: [PATCH 5/6] lint Signed-off-by: Kerry Archibald --- src/components/structures/HomePage.tsx | 14 +-- src/languageHandler.tsx | 4 +- test/i18n-test/languageHandler-test.tsx | 119 ++++++++++++++++++------ 3 files changed, 97 insertions(+), 40 deletions(-) diff --git a/src/components/structures/HomePage.tsx b/src/components/structures/HomePage.tsx index 137ca799ccc..2a931d760b2 100644 --- a/src/components/structures/HomePage.tsx +++ b/src/components/structures/HomePage.tsx @@ -86,8 +86,8 @@ const UserWelcomeTop = () => { /> -

{_tDom("Welcome %(name)s", { name: ownProfile.displayName })}

-

{_tDom("Now, let's help you get started")}

+

{ _tDom("Welcome %(name)s", { name: ownProfile.displayName }) }

+

{ _tDom("Now, let's help you get started") }

; }; @@ -113,8 +113,8 @@ const HomePage: React.FC = ({ justRegistered = false }) => { introSection = {config.brand} -

{_tDom("Welcome to %(appName)s", { appName: config.brand })}

-

{_tDom("Own your conversations.")}

+

{ _tDom("Welcome to %(appName)s", { appName: config.brand }) }

+

{ _tDom("Own your conversations.") }

; } @@ -123,13 +123,13 @@ const HomePage: React.FC = ({ justRegistered = false }) => { { introSection }
- {_tDom("Send a Direct Message")} + { _tDom("Send a Direct Message") } - {_tDom("Explore Public Rooms")} + { _tDom("Explore Public Rooms") } - {_tDom("Create a Group Chat")} + { _tDom("Create a Group Chat") }
diff --git a/src/languageHandler.tsx b/src/languageHandler.tsx index 61c4aaf9f25..36c1fd6c0af 100644 --- a/src/languageHandler.tsx +++ b/src/languageHandler.tsx @@ -142,9 +142,9 @@ const annotateStrings = (result: TranslatedString, translationKey: string): Tran if (typeof result === 'string') { return `@@${translationKey}##${result}@@`; } else { - return {result}; + return { result }; } -} +}; /* * Translates text and optionally also replaces XML-ish elements in the text with e.g. React components diff --git a/test/i18n-test/languageHandler-test.tsx b/test/i18n-test/languageHandler-test.tsx index b34235c7dbc..c07b5543c04 100644 --- a/test/i18n-test/languageHandler-test.tsx +++ b/test/i18n-test/languageHandler-test.tsx @@ -1,40 +1,90 @@ -import { _t, _tDom, TranslatedString, setLanguage, setMissingEntryGenerator, substitute } from '../../src/languageHandler'; +import React from 'react'; -const React = require('react'); -const expect = require('expect'); +import { + _t, + _tDom, + TranslatedString, + setLanguage, + setMissingEntryGenerator, + substitute, +} from '../../src/languageHandler'; +import { stubClient } from '../test-utils'; -const testUtils = require('../test-utils'); - -describe('languageHandler', function () { +describe('languageHandler', function() { const basicString = 'Rooms'; const selfClosingTagSub = 'Accept to continue:'; const textInTagSub = 'Upgrade to your own domain'; const plurals = 'and %(count)s others...'; const variableSub = 'You are now ignoring %(userId)s'; - type TestCase = [string, string, Record, Record, TranslatedString] + type TestCase = [string, string, Record, Record, TranslatedString]; const testCasesEn: TestCase[] = [ ['translates a basic string', basicString, {}, undefined, 'Rooms'], - ['handles plurals when count is 1', plurals, { count: 1 }, undefined, 'and one other...'], - ['handles plurals when count is not 1', plurals, { count: 2 }, undefined, 'and 2 others...'], - ['handles simple variable substitution', variableSub, { userId: 'foo' }, undefined, 'You are now ignoring foo'], - ['handles simple tag substitution', selfClosingTagSub, {}, { 'policyLink': () => 'foo' }, 'Accept foo to continue:'], + [ + 'handles plurals when count is 1', + plurals, + { count: 1 }, + undefined, + 'and one other...', + ], + [ + 'handles plurals when count is not 1', + plurals, + { count: 2 }, + undefined, + 'and 2 others...', + ], + [ + 'handles simple variable substitution', + variableSub, + { userId: 'foo' }, + undefined, + 'You are now ignoring foo', + ], + [ + 'handles simple tag substitution', + selfClosingTagSub, + {}, + { 'policyLink': () => 'foo' }, + 'Accept foo to continue:', + ], ['handles text in tags', textInTagSub, {}, { 'a': (sub) => `x${sub}x` }, 'xUpgradex to your own domain'], - ['handles variable substitution with React function component', variableSub, { userId: () => foo }, undefined, You are now ignoring foo], - ['handles variable substitution with react node', variableSub, { userId: foo }, undefined, You are now ignoring foo], - ['handles tag substitution with React function component', selfClosingTagSub, {}, { 'policyLink': () => foo }, Accept foo to continue:], + [ + 'handles variable substitution with React function component', + variableSub, + { userId: () => foo }, + undefined, + // eslint-disable-next-line react/jsx-key + You are now ignoring foo, + ], + [ + 'handles variable substitution with react node', + variableSub, + { userId: foo }, + undefined, + // eslint-disable-next-line react/jsx-key + You are now ignoring foo, + ], + [ + 'handles tag substitution with React function component', + selfClosingTagSub, + {}, + { 'policyLink': () => foo }, + // eslint-disable-next-line react/jsx-key + Accept foo to continue:, + ], ]; describe('when translations exist in language', () => { - beforeEach(function (done) { - testUtils.stubClient(); + beforeEach(function(done) { + stubClient(); setLanguage('en').then(done); setMissingEntryGenerator(key => key.split("|", 2)[1]); }); - it('translates a string to german', function (done) { - setLanguage('de').then(function () { + it('translates a string to german', function(done) { + setLanguage('de').then(function() { const translated = _t(basicString); expect(translated).toBe('Räume'); }).then(done); @@ -44,17 +94,17 @@ describe('languageHandler', function () { expect(_t(translationString, variables, tags)).toEqual(result); }); - it('replacements in the wrong order', function () { + it('replacements in the wrong order', function() { const text = '%(var1)s %(var2)s'; expect(_t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2'); }); - it('multiple replacements of the same variable', function () { + it('multiple replacements of the same variable', function() { const text = '%(var1)s %(var1)s'; expect(substitute(text, { var1: 'val1' })).toBe('val1 val1'); }); - it('multiple replacements of the same tag', function () { + it('multiple replacements of the same tag', function() { const text = 'Click here to join the discussion! or here'; expect(substitute(text, {}, { 'a': (sub) => `x${sub}x` })) .toBe('xClick herex to join the discussion! xor herex'); @@ -63,25 +113,32 @@ describe('languageHandler', function () { describe('when a translation string does not exist in active language', () => { beforeEach(async () => { - testUtils.stubClient(); + stubClient(); await setLanguage('lv'); // counterpart doesnt expose any way to restore default config // missingEntryGenerator is mocked in the root setup file // reset to default here - const counterpartDefaultMissingEntryGen = function (key) { return 'missing translation: ' + key; }; + const counterpartDefaultMissingEntryGen = + function(key) { return 'missing translation: ' + key; }; setMissingEntryGenerator(counterpartDefaultMissingEntryGen); }); describe('_t', () => { - it.each(testCasesEn)("%s and translates with fallback locale", async (_d, translationString, variables, tags, result) => { - expect(_t(translationString, variables, tags)).toEqual(result); - }); - }) + it.each(testCasesEn)( + "%s and translates with fallback locale", + async (_d, translationString, variables, tags, result) => { + expect(_t(translationString, variables, tags)).toEqual(result); + }, + ); + }); describe('_tDom()', () => { - it.each(testCasesEn)("%s and translates with fallback locale, attributes fallback locale", async (_d, translationString, variables, tags, result) => { - expect(_tDom(translationString, variables, tags)).toEqual({result}); - }); - }) + it.each(testCasesEn)( + "%s and translates with fallback locale, attributes fallback locale", + async (_d, translationString, variables, tags, result) => { + expect(_tDom(translationString, variables, tags)).toEqual({ result }); + }, + ); + }); }); }); From 010d274b234ce2e8810672bac4c6e5fca1a79f72 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 5 Jan 2022 10:42:02 +0100 Subject: [PATCH 6/6] bump matrix-web-i18n Signed-off-by: Kerry Archibald --- src/i18n/strings/en_EN.json | 9 --------- yarn.lock | 1 + 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 72dd21e9b26..61db847fdc6 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2947,15 +2947,6 @@ "Community %(groupId)s not found": "Community %(groupId)s not found", "This homeserver does not support communities": "This homeserver does not support communities", "Failed to load %(groupId)s": "Failed to load %(groupId)s", - "Great, that'll help people know it's you": "Great, that'll help people know it's you", - "Add a photo so people know it's you.": "Add a photo so people know it's you.", - "Welcome %(name)s": "Welcome %(name)s", - "Now, let's help you get started": "Now, let's help you get started", - "Welcome to %(appName)s": "Welcome to %(appName)s", - "Own your conversations.": "Own your conversations.", - "Send a Direct Message": "Send a Direct Message", - "Explore Public Rooms": "Explore Public Rooms", - "Create a Group Chat": "Create a Group Chat", "Upgrade to %(hostSignupBrand)s": "Upgrade to %(hostSignupBrand)s", "Open dial pad": "Open dial pad", "Public community": "Public community", diff --git a/yarn.lock b/yarn.lock index 959cf70c79c..306aabd9f31 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6040,6 +6040,7 @@ mathml-tag-names@^2.1.3: browser-request "^0.3.3" bs58 "^4.0.1" content-type "^1.0.4" + eslint-plugin-import "^2.25.2" loglevel "^1.7.1" p-retry "^4.5.0" qs "^6.9.6"