From 7a4d712e758568b3447f2f134b732cd8e05a296e Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 10 Oct 2022 12:18:47 +0100 Subject: [PATCH 01/14] Update I18n to use '.' separated plural suffix This'll let us use Objects to pass pluralised translations rather than one key per plural type --- src/govuk/i18n.mjs | 8 ++------ src/govuk/i18n.unit.test.mjs | 8 ++++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index 6e1a7ec49f..f337fc3a68 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -37,16 +37,12 @@ I18n.prototype.t = function (lookupKey, options) { // Get the plural suffix var pluralSuffix = this.getPluralSuffix(options.count) - // We need to transform this to have an initial uppercase letter, - // as our keys are stored in camelCase - pluralSuffix = pluralSuffix.charAt(0).toUpperCase() + pluralSuffix.slice(1) - // Overwrite our existing lookupKey - lookupKey = lookupKey + pluralSuffix + lookupKey = lookupKey + '.' + pluralSuffix // Throw an error if this new key doesn't exist if (!(lookupKey in this.translations)) { - throw new Error('i18n: Plural form "' + pluralSuffix + '" is required for "' + this.locale + '" locale') + throw new Error('i18n: Plural form ".' + pluralSuffix + '" is required for "' + this.locale + '" locale') } } diff --git a/src/govuk/i18n.unit.test.mjs b/src/govuk/i18n.unit.test.mjs index 73a741085d..6078037b15 100644 --- a/src/govuk/i18n.unit.test.mjs +++ b/src/govuk/i18n.unit.test.mjs @@ -153,17 +153,17 @@ describe('I18n', () => { describe('pluralisation', () => { it('throws an error if a required plural form is not provided ', () => { const i18n = new I18n({ - testOther: 'testing testing' + 'test.other': 'testing testing' }, { locale: 'en' }) - expect(() => { i18n.t('test', { count: 1 }) }).toThrowError('i18n: Plural form "One" is required for "en" locale') + expect(() => { i18n.t('test', { count: 1 }) }).toThrowError('i18n: Plural form ".one" is required for "en" locale') }) it('interpolates the count variable into the correct plural form', () => { const i18n = new I18n({ - testOne: '%{count} test', - testOther: '%{count} tests' + 'test.one': '%{count} test', + 'test.other': '%{count} tests' }, { locale: 'en' }) From d7993a0c6e1e1086ee36f7c5aa744951b9cd2712 Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 10 Oct 2022 12:27:30 +0100 Subject: [PATCH 02/14] Test config key flattening is on multiple levels --- src/govuk/common.unit.test.mjs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/govuk/common.unit.test.mjs b/src/govuk/common.unit.test.mjs index 5c0ce893e1..e01032fbf8 100644 --- a/src/govuk/common.unit.test.mjs +++ b/src/govuk/common.unit.test.mjs @@ -20,7 +20,12 @@ describe('Common JS utilities', () => { const config3 = { b: 'bat', c: { o: 'cow' }, - d: 'dog' + d: 'dog', + e: { + l: { + e: 'elephant' + } + } } it('flattens a single object', () => { @@ -48,7 +53,8 @@ describe('Common JS utilities', () => { b: 'bat', 'c.a': 'cat', 'c.o': 'cow', - d: 'dog' + d: 'dog', + 'e.l.e': 'elephant' }) }) @@ -69,7 +75,8 @@ describe('Common JS utilities', () => { b: 'bat', 'c.a': 'camel', 'c.o': 'cow', - d: 'dog' + d: 'dog', + 'e.l.e': 'elephant' }) }) From e3a889eeddbaa7d2e6f6bcc85ed4e3e02b207def Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 10 Oct 2022 12:47:19 +0100 Subject: [PATCH 03/14] Make CharacterCount use the dot separated plural forms --- .../character-count/character-count.mjs | 43 ++++++++++++------- .../character-count.unit.test.mjs | 8 ++-- src/govuk/i18n.mjs | 15 +++++++ 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/govuk/components/character-count/character-count.mjs b/src/govuk/components/character-count/character-count.mjs index 1fb5ac3b17..4218c5655c 100644 --- a/src/govuk/components/character-count/character-count.mjs +++ b/src/govuk/components/character-count/character-count.mjs @@ -5,6 +5,32 @@ import '../../vendor/polyfills/Element/prototype/classList.mjs' import { closestAttributeValue, extractConfigByNamespace, mergeConfigs, normaliseDataset } from '../../common.mjs' import { I18n } from '../../i18n.mjs' +/** + * @type {import('../../i18n.mjs').PluralisedTranslations} + */ +var TRANSLATIONS_DEFAULT = { + // Characters + charactersUnderLimit: { + one: 'You have %{count} character remaining', + other: 'You have %{count} characters remaining' + }, + charactersAtLimit: 'You have 0 characters remaining', + charactersOverLimit: { + one: 'You have %{count} character too many', + other: 'You have %{count} characters too many' + }, + // Words + wordsUnderLimit: { + one: 'You have %{count} word remaining', + other: 'You have %{count} words remaining' + }, + wordsAtLimit: 'You have 0 words remaining', + wordsOverLimit: { + one: 'You have %{count} word too many', + other: 'You have %{count} words too many' + } +} + /** * JavaScript enhancements for the CharacterCount component * @@ -25,7 +51,7 @@ import { I18n } from '../../i18n.mjs' * @param {Number} [config.threshold=0] - The percentage value of the limit at * which point the count message is displayed. If this attribute is set, the * count message will be hidden by default. - * @param {Object} [config.i18n] + * @param {Object} [config.i18n = DEFAULT_TRANSLATIONS] * @param {String} [config.i18n.charactersUnderLimitOne="You have %{count} character remaining"] * Message notifying users they're 1 character under the limit * @param {String} [config.i18n.charactersUnderLimitOther="You have %{count} characters remaining"] @@ -54,20 +80,7 @@ function CharacterCount ($module, config) { var defaultConfig = { threshold: 0, - i18n: { - // Characters - charactersUnderLimitOne: 'You have %{count} character remaining', - charactersUnderLimitOther: 'You have %{count} characters remaining', - charactersAtLimit: 'You have 0 characters remaining', - charactersOverLimitOne: 'You have %{count} character too many', - charactersOverLimitOther: 'You have %{count} characters too many', - // Words - wordsUnderLimitOne: 'You have %{count} word remaining', - wordsUnderLimitOther: 'You have %{count} words remaining', - wordsAtLimit: 'You have 0 words remaining', - wordsOverLimitOne: 'You have %{count} word too many', - wordsOverLimitOther: 'You have %{count} words too many' - } + i18n: TRANSLATIONS_DEFAULT } // Read config set using dataset ('data-' values) diff --git a/src/govuk/components/character-count/character-count.unit.test.mjs b/src/govuk/components/character-count/character-count.unit.test.mjs index 82a788d2c5..aa45b824d0 100644 --- a/src/govuk/components/character-count/character-count.unit.test.mjs +++ b/src/govuk/components/character-count/character-count.unit.test.mjs @@ -42,8 +42,8 @@ describe('CharacterCount', () => { describe('JavaScript configuration', () => { it('overrides the default translation keys', () => { const component = new CharacterCount(document.createElement('div'), { - i18n: { charactersUnderLimitOne: 'Custom text. Count: %{count}' }, - 'i18n.charactersOverLimitOther': 'Different custom text. Count: %{count}' + i18n: { charactersUnderLimit: { one: 'Custom text. Count: %{count}' } }, + 'i18n.charactersOverLimit.other': 'Different custom text. Count: %{count}' }) expect(component.formatCountMessage(1, 'characters')).toEqual('Custom text. Count: 1') @@ -89,7 +89,7 @@ describe('CharacterCount', () => { describe('Data attribute configuration', () => { it('overrides the default translation keys', () => { const $div = document.createElement('div') - $div.setAttribute('data-i18n.characters-under-limit-one', 'Custom text. Count: %{count}') + $div.setAttribute('data-i18n.characters-under-limit.one', 'Custom text. Count: %{count}') const component = new CharacterCount($div) @@ -101,7 +101,7 @@ describe('CharacterCount', () => { describe('precedence over JavaScript configuration', () => { it('overrides translation keys', () => { const $div = document.createElement('div') - $div.setAttribute('data-i18n.characters-under-limit-one', 'Custom text. Count: %{count}') + $div.setAttribute('data-i18n.characters-under-limit.one', 'Custom text. Count: %{count}') const component = new CharacterCount($div, { i18n: { diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index f337fc3a68..0a6425139e 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -1,3 +1,18 @@ +/** + * Associates translated messages to plural type they correspond to. + * + * Allows to group pluralised messages under a single key when passing + * translations to a component's constructor + * + * @typedef {object} PluralisedTranslation + * @property {string} other + * @property {string} [zero] + * @property {string} [one] + * @property {string} [two] + * @property {string} [few] + * @property {string} [many] + */ + /** * i18n support initialisation function * From d88f1fa8585dd1db3dca26e7f8d9a994df86fc1b Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 10 Oct 2022 14:04:48 +0100 Subject: [PATCH 04/14] Add helper macro for outputing i18n pluralisations --- lib/jest-helpers.js | 24 +++++++++++++++++++++--- src/govuk/macros/i18n.njk | 13 +++++++++++++ src/govuk/macros/i18n.unit.test.mjs | 25 +++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 src/govuk/macros/i18n.njk create mode 100644 src/govuk/macros/i18n.unit.test.mjs diff --git a/lib/jest-helpers.js b/lib/jest-helpers.js index 33d155ea9d..e2bb3968a6 100644 --- a/lib/jest-helpers.js +++ b/lib/jest-helpers.js @@ -28,13 +28,30 @@ const nunjucksEnv = nunjucks.configure([configPaths.src, configPaths.components] */ function renderHtml (componentName, options, callBlock = false) { if (typeof options === 'undefined') { - throw new Error('Parameters passed to `render` should be an object but are undefined') + throw new Error( + 'Parameters passed to `render` should be an object but are undefined' + ) } const macroName = componentNameToMacroName(componentName) - const macroParams = JSON.stringify(options, null, 2) + const macroPath = `${componentName}/macro.njk` - let macroString = `{%- from "${componentName}/macro.njk" import ${macroName} -%}` + return callMacro(macroName, macroPath, [options], callBlock) +} + +/** + * Returns the string result from calling a macro + * + * @param {string} macroName - The name of the macro + * @param {string} macroPath - The path to the file containing the macro *from the root of the project* + * @param {Array} params - The parameters that will be passed to the macro. They'll be `JSON.stringify`ed and joined with a `,` + * @param {string} [callBlock] - Content for an optional callBlock, if necessary for the macro to receive one + * @returns string - The result of calling the macro + */ +function callMacro (macroName, macroPath, params = [], callBlock = false) { + const macroParams = params.map(param => JSON.stringify(param, null, 2)).join(',') + + let macroString = `{%- from "${macroPath}" import ${macroName} -%}` // If we're nesting child components or text, pass the children to the macro // using the 'caller' Nunjucks feature @@ -144,6 +161,7 @@ module.exports = { getExamples, htmlWithClassName, nunjucksEnv, + callMacro, render, renderHtml, renderSass, diff --git a/src/govuk/macros/i18n.njk b/src/govuk/macros/i18n.njk new file mode 100644 index 0000000000..93fe5c0335 --- /dev/null +++ b/src/govuk/macros/i18n.njk @@ -0,0 +1,13 @@ +{# + # Renders the data attributes for the different plural forms + # of the given translation key. + # + # Helps reduce the boilerplate in component templates as they're quite verbose + # @param {string} translationKey - The kebab-cased name of the translation key + # @param {objet} pluralForms + # An object associating translation messages to the plural form they correspond to + # http://unicode.org/reports/tr35/tr35-numbers.html#Language_Plural_Rules + #} +{% macro govukPluralisedI18nAttributes(translationKey, pluralForms) %} + {% for pluralType, message in pluralForms %} data-i18n.{{translationKey}}.{{pluralType}}="{{message | escape}}"{% endfor %} +{% endmacro %} diff --git a/src/govuk/macros/i18n.unit.test.mjs b/src/govuk/macros/i18n.unit.test.mjs new file mode 100644 index 0000000000..2916fcffde --- /dev/null +++ b/src/govuk/macros/i18n.unit.test.mjs @@ -0,0 +1,25 @@ +import { callMacro } from '../../../lib/jest-helpers' + +describe('i18n.njk', () => { + describe('govukPluralisedI18nAttributes', () => { + function callMacroUnderTest (...args) { + return callMacro('govukPluralisedI18nAttributes', 'macros/i18n.njk', ...args) + } + + it('renders a single plural type', () => { + const attributes = callMacroUnderTest(['translation-key', { other: 'You have %{count} characters remaining.' }]) + // Note the starting space so we ensure it doesn't stick to possible other previous attributes + expect(attributes).toEqual(' data-i18n.translation-key.other="You have %{count} characters remaining."') + }) + + it('renders multiple plural types', () => { + const attributes = callMacroUnderTest(['translation-key', { other: 'You have %{count} characters remaining.', one: 'One character remaining' }]) + expect(attributes).toEqual(' data-i18n.translation-key.other="You have %{count} characters remaining." data-i18n.translation-key.one="One character remaining"') + }) + + it('outputs nothing if there are no translations', () => { + const attributes = callMacroUnderTest(['translation-key', {}]) + expect(attributes).toEqual('') + }) + }) +}) From 75d65276f4828126828236c91f2c6da26f854cc2 Mon Sep 17 00:00:00 2001 From: Romaric Date: Mon, 10 Oct 2022 14:57:15 +0100 Subject: [PATCH 05/14] Add options for passing translations to character count macro --- .../character-count/character-count.yaml | 21 ++++++++++++++++ .../components/character-count/template.njk | 10 +++++++- .../character-count/template.test.js | 25 ++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/govuk/components/character-count/character-count.yaml b/src/govuk/components/character-count/character-count.yaml index 0c72df8590..eab0202b6c 100644 --- a/src/govuk/components/character-count/character-count.yaml +++ b/src/govuk/components/character-count/character-count.yaml @@ -167,6 +167,27 @@ examples: label: text: Full address + - name: with translations + data: + id: with-translations + name: with-translations + maxlength: 10 + charactersUnderLimitHtml: + other: '%{count} characters to go' + one: 'One character to go' + charactersAtLimitHtml: 'Zero characters left' + charactersOverLimitHtml: + other: '%{count} characters too many' + one: 'One character too many' + wordsUnderLimitHtml: + other: '%{count} words to go' + one: 'One word to go' + wordsAtLimitHtml: 'Zero words left' + wordsOverLimitHtml: + other: '%{count} words too many' + one: 'One word too many' + + # Hidden examples are not shown in the review app, but are used for tests and HTML fixtures - name: classes hidden: true diff --git a/src/govuk/components/character-count/template.njk b/src/govuk/components/character-count/template.njk index d812a777ee..5247d4416e 100644 --- a/src/govuk/components/character-count/template.njk +++ b/src/govuk/components/character-count/template.njk @@ -1,10 +1,18 @@ +{% from "../../macros/i18n.njk" import govukPluralisedI18nAttributes %} + {% from "../textarea/macro.njk" import govukTextarea %} {% from "../hint/macro.njk" import govukHint %}
+{%- if params.maxwords %} data-maxwords="{{ params.maxwords }}"{% endif %} +{%- if params.charactersUnderLimitHtml %}{{govukPluralisedI18nAttributes('characters-under-limit', params.charactersUnderLimitHtml)}}{% endif %} +{%- if params.charactersAtLimitHtml %} data-i18n.characters-at-limit="{{ params.charactersAtLimitHtml | escape}}"{% endif %} +{%- if params.charactersOverLimitHtml %}{{govukPluralisedI18nAttributes('characters-over-limit', params.charactersOverLimitHtml)}}{% endif %} +{%- if params.wordsUnderLimitHtml %}{{govukPluralisedI18nAttributes('words-under-limit', params.wordsUnderLimitHtml)}}{% endif %} +{%- if params.wordsAtLimitHtml %} data-i18n.words-at-limit="{{ params.wordsAtLimitHtml | escape}}"{% endif %} +{%- if params.wordsOverLimitHtml %}{{govukPluralisedI18nAttributes('words-over-limit', params.wordsOverLimitHtml)}}{% endif %}> {{ govukTextarea({ id: params.id, name: params.name, diff --git a/src/govuk/components/character-count/template.test.js b/src/govuk/components/character-count/template.test.js index c2fe3e2bdb..de551d20d4 100644 --- a/src/govuk/components/character-count/template.test.js +++ b/src/govuk/components/character-count/template.test.js @@ -221,11 +221,34 @@ describe('Character count', () => { }) describe('with threshold', () => { - it('hides the count to start with', async () => { + it('hides the count to start with', () => { const $ = render('character-count', examples['with threshold']) const $component = $('.govuk-character-count') expect($component.attr('data-threshold')).toEqual('75') }) }) + + describe('translations', () => { + it('renders with translation data attributes', () => { + const $ = render('character-count', examples['with translations']) + + const $component = $('[data-module]') + + Object.entries({ + 'data-i18n.characters-under-limit.one': 'One character to go', + 'data-i18n.characters-under-limit.other': '%{count} characters to go', + 'data-i18n.characters-at-limit': 'Zero characters left', + 'data-i18n.characters-over-limit.one': 'One character too many', + 'data-i18n.characters-over-limit.other': '%{count} characters too many', + 'data-i18n.words-under-limit.one': 'One word to go', + 'data-i18n.words-under-limit.other': '%{count} words to go', + 'data-i18n.words-at-limit': 'Zero words left', + 'data-i18n.words-over-limit.one': 'One word too many', + 'data-i18n.words-over-limit.other': '%{count} words too many' + }).forEach(([attributeName, expectedValue]) => { + expect($component.attr(attributeName)).toEqual(expectedValue) + }) + }) + }) }) From 5a01101d41e9773137f618d02cb2a3aeab1a58bf Mon Sep 17 00:00:00 2001 From: Romaric Date: Tue, 11 Oct 2022 16:08:46 +0100 Subject: [PATCH 06/14] Fix typing of CharacterCount translations --- .../character-count/character-count.mjs | 34 +++++++------------ src/govuk/i18n.mjs | 30 ++++++++-------- 2 files changed, 27 insertions(+), 37 deletions(-) diff --git a/src/govuk/components/character-count/character-count.mjs b/src/govuk/components/character-count/character-count.mjs index 4218c5655c..95239faccf 100644 --- a/src/govuk/components/character-count/character-count.mjs +++ b/src/govuk/components/character-count/character-count.mjs @@ -6,7 +6,7 @@ import { closestAttributeValue, extractConfigByNamespace, mergeConfigs, normalis import { I18n } from '../../i18n.mjs' /** - * @type {import('../../i18n.mjs').PluralisedTranslations} + * @type {CharacterCountTranslations} */ var TRANSLATIONS_DEFAULT = { // Characters @@ -51,27 +51,7 @@ var TRANSLATIONS_DEFAULT = { * @param {Number} [config.threshold=0] - The percentage value of the limit at * which point the count message is displayed. If this attribute is set, the * count message will be hidden by default. - * @param {Object} [config.i18n = DEFAULT_TRANSLATIONS] - * @param {String} [config.i18n.charactersUnderLimitOne="You have %{count} character remaining"] - * Message notifying users they're 1 character under the limit - * @param {String} [config.i18n.charactersUnderLimitOther="You have %{count} characters remaining"] - * Message notifying users they're any number of characters under the limit - * @param {String} [config.i18n.charactersAtLimit="You have 0 characters remaining"] - * Message notifying users they've reached the limit number of characters - * @param {String} [config.i18n.charactersOverLimitOne="You have %{count} character too many"] - * Message notifying users they're 1 character over the limit - * @param {String} [config.i18n.charactersOverLimitOther="You have %{count} characters too many"] - * Message notifying users they're any number of characters over the limit - * @param {String} [config.i18n.wordsUnderLimitOne="You have %{count} word remaining"] - * Message notifying users they're 1 word under the limit - * @param {String} [config.i18n.wordsUnderLimitOther="You have %{count} words remaining"] - * Message notifying users they're any number of words under the limit - * @param {String} [config.i18n.wordsAtLimit="You have 0 words remaining"] - * Message notifying users they've reached the limit number of words - * @param {String} [config.i18n.wordsOverLimitOne="You have %{count} word too many"] - * Message notifying users they're 1 word over the limit - * @param {String} [config.i18n.wordsOverLimitOther="You have %{count} words too many"] - * Message notifying users they're any number of words over the limit + * @param {CharacterCountTranslations} [config.i18n = DEFAULT_TRANSLATIONS] */ function CharacterCount ($module, config) { if (!$module) { @@ -384,3 +364,13 @@ CharacterCount.prototype.isOverThreshold = function () { } export default CharacterCount + +/** + * @typedef {object} CharacterCountTranslations + * @property {import('../../i18n.mjs').PluralisedTranslation} [charactersUnderLimit] + * @property {string} [charactersAtLimit] + * @property {import('../../i18n.mjs').PluralisedTranslation} [charactersOverLimit] + * @property {import('../../i18n.mjs').PluralisedTranslation} [wordsUnderLimit] + * @property {string} [wordsAtLimit] + * @property {import('../../i18n.mjs').PluralisedTranslation} [wordsOverLimit] + */ diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index 0a6425139e..d56156323d 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -1,18 +1,3 @@ -/** - * Associates translated messages to plural type they correspond to. - * - * Allows to group pluralised messages under a single key when passing - * translations to a component's constructor - * - * @typedef {object} PluralisedTranslation - * @property {string} other - * @property {string} [zero] - * @property {string} [one] - * @property {string} [two] - * @property {string} [few] - * @property {string} [many] - */ - /** * i18n support initialisation function * @@ -312,3 +297,18 @@ I18n.prototype.pluralRules = { return 'other' } } + +/** + * Associates translated messages to plural type they correspond to. + * + * Allows to group pluralised messages under a single key when passing + * translations to a component's constructor + * + * @typedef {object} PluralisedTranslation + * @property {string} other + * @property {string} [zero] + * @property {string} [one] + * @property {string} [two] + * @property {string} [few] + * @property {string} [many] + */ From cd414074f202266fe3f28fb1f5eee0e06b4386be Mon Sep 17 00:00:00 2001 From: Romaric Pascal Date: Wed, 12 Oct 2022 10:41:47 +0100 Subject: [PATCH 07/14] Fix typos Co-authored-by: Oliver Byford --- src/govuk/macros/i18n.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/govuk/macros/i18n.njk b/src/govuk/macros/i18n.njk index 93fe5c0335..c3fe548b18 100644 --- a/src/govuk/macros/i18n.njk +++ b/src/govuk/macros/i18n.njk @@ -4,7 +4,7 @@ # # Helps reduce the boilerplate in component templates as they're quite verbose # @param {string} translationKey - The kebab-cased name of the translation key - # @param {objet} pluralForms + # @param {object} pluralForms # An object associating translation messages to the plural form they correspond to # http://unicode.org/reports/tr35/tr35-numbers.html#Language_Plural_Rules #} From 7fac761795d528ccec71f940af69b1c33342bbb1 Mon Sep 17 00:00:00 2001 From: Romaric Date: Thu, 13 Oct 2022 16:28:31 +0100 Subject: [PATCH 08/14] Mark govukPluralisedI18nAttributes as private --- src/govuk/macros/i18n.njk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/govuk/macros/i18n.njk b/src/govuk/macros/i18n.njk index c3fe548b18..d4b1c4f800 100644 --- a/src/govuk/macros/i18n.njk +++ b/src/govuk/macros/i18n.njk @@ -3,6 +3,8 @@ # of the given translation key. # # Helps reduce the boilerplate in component templates as they're quite verbose + # + # @private # @param {string} translationKey - The kebab-cased name of the translation key # @param {object} pluralForms # An object associating translation messages to the plural form they correspond to From c79ea2a23814c39ff202a4c70ea2c05256c25b62 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Oct 2022 15:20:20 +0100 Subject: [PATCH 09/14] Split out logic to choose ruleset based on locale This part of the execution flow is not currently tested as `Intl.PluralRules` exists in the test environment, so we never follow the 'else' part of the control flow. Splitting it out allows us to unit test it and ensure that it works as expected. Add error handling for unsupported locales. --- src/govuk/i18n.mjs | 46 ++++++++++++++++++++++-------------- src/govuk/i18n.unit.test.mjs | 20 ++++++++++++++++ 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index d56156323d..173baec385 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -138,17 +138,10 @@ I18n.prototype.hasIntlNumberFormatSupport = function () { /** * Get the appropriate suffix for the plural form. * - * The locale may include a regional indicator (such as en-GB), but we don't - * usually care about this part, as pluralisation rules are usually the same - * regardless of region. There are exceptions, however, (e.g. Portuguese) so - * this searches by both the full and shortened locale codes, just to be sure. - * * @param {number} count - Number used to determine which pluralisation to use. * @returns {string} - The suffix associated with the correct pluralisation for this locale. */ I18n.prototype.getPluralSuffix = function (count) { - var locale = this.locale - var localeShort = locale.split('-')[0] var keySuffix = 'other' // Validate that the number is actually a number. @@ -169,22 +162,39 @@ I18n.prototype.getPluralSuffix = function (count) { // make sure our number is one of those. count = Math.abs(Math.floor(count)) - // Look through the plural rules map to find which `pluralRule` is - // appropriate for our current `locale`. - for (var pluralRule in this.pluralRulesMap) { - if (Object.prototype.hasOwnProperty.call(this.pluralRulesMap, pluralRule)) { - var languages = this.pluralRulesMap[pluralRule] - if (languages.indexOf(locale) > -1 || languages.indexOf(localeShort) > -1) { - keySuffix = this.pluralRules[pluralRule](count) - break - } - } - } + return this.pluralRules[this.getPluralRulesForLocale()](count) } return keySuffix } +/** + * Work out which pluralisation rules to use for the current locale + * + * The locale may include a regional indicator (such as en-GB), but we don't + * usually care about this part, as pluralisation rules are usually the same + * regardless of region. There are exceptions, however, (e.g. Portuguese) so + * this searches by both the full and shortened locale codes, just to be sure. + * + * @returns {string} - The name of the pluralisation rule to use (a key for one + * of the functions in this.pluralRules) + */ +I18n.prototype.getPluralRulesForLocale = function () { + var locale = this.locale + var localeShort = locale.split('-')[0] + + // Look through the plural rules map to find which `pluralRule` is + // appropriate for our current `locale`. + for (var pluralRule in this.pluralRulesMap) { + if (Object.prototype.hasOwnProperty.call(this.pluralRulesMap, pluralRule)) { + var languages = this.pluralRulesMap[pluralRule] + if (languages.indexOf(locale) > -1 || languages.indexOf(localeShort) > -1) { + return pluralRule + } + } + } +} + /** * Map of plural rules to languages where those rules apply. * diff --git a/src/govuk/i18n.unit.test.mjs b/src/govuk/i18n.unit.test.mjs index 6078037b15..485d14ab15 100644 --- a/src/govuk/i18n.unit.test.mjs +++ b/src/govuk/i18n.unit.test.mjs @@ -150,6 +150,26 @@ describe('I18n', () => { }) }) + describe('.getPluralRulesForLocale', () => { + it('returns the correct rules for a locale in the map', () => { + const locale = 'ar' + const i18n = new I18n({}, { locale }) + expect(i18n.getPluralRulesForLocale()).toBe('arabic') + }) + + it('returns the correct rules for a locale in the map with regional indicator', () => { + const locale = 'pt-PT' + const i18n = new I18n({}, { locale }) + expect(i18n.getPluralRulesForLocale()).toBe('spanish') + }) + + it('returns the correct rules for a locale allowing for no regional indicator', () => { + const locale = 'cy-GB' + const i18n = new I18n({}, { locale }) + expect(i18n.getPluralRulesForLocale()).toBe('welsh') + }) + }) + describe('pluralisation', () => { it('throws an error if a required plural form is not provided ', () => { const i18n = new I18n({ From c16fa497c4571fd0cfc874dba47ffdd7a31ec315 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Oct 2022 15:25:28 +0100 Subject: [PATCH 10/14] Split out fallback logic for selecting plural rule This allows us to test the entire fallback logic together, passing a locale and asserting that we have the same behaviour as Intl.PluralRules, rather than testing the individual functions in i18n.PluralRules. --- src/govuk/i18n.mjs | 29 +++++-- src/govuk/i18n.unit.test.mjs | 157 ++++++++++++++++++----------------- 2 files changed, 107 insertions(+), 79 deletions(-) diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index 173baec385..7a03e35245 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -158,16 +158,35 @@ I18n.prototype.getPluralSuffix = function (count) { var pluralRules = new Intl.PluralRules(this.locale) keySuffix = pluralRules.select(count) } else { - // Currently our custom code can only handle positive integers, so let's - // make sure our number is one of those. - count = Math.abs(Math.floor(count)) - - return this.pluralRules[this.getPluralRulesForLocale()](count) + return this.selectPluralRuleFromFallback(count) } return keySuffix } +/** + * Get the plural rule using our fallback implementation + * + * This is split out into a separate function to make it easier to test the + * fallback behaviour in an environment where Intl.PluralRules exists. + * + * @param {Number} count - Number used to determine which pluralisation to use. + * @returns {string} - The suffix associated with the correct pluralisation for this locale. + */ +I18n.prototype.selectPluralRuleFromFallback = function (count) { + // Currently our custom code can only handle positive integers, so let's + // make sure our number is one of those. + count = Math.abs(Math.floor(count)) + + var ruleset = this.getPluralRulesForLocale() + + if (ruleset) { + return this.pluralRules[ruleset](count) + } + + return 'other' +} + /** * Work out which pluralisation rules to use for the current locale * diff --git a/src/govuk/i18n.unit.test.mjs b/src/govuk/i18n.unit.test.mjs index 485d14ab15..c9616d0783 100644 --- a/src/govuk/i18n.unit.test.mjs +++ b/src/govuk/i18n.unit.test.mjs @@ -191,116 +191,125 @@ describe('I18n', () => { expect(i18n.t('test', { count: 1 })).toBe('1 test') expect(i18n.t('test', { count: 5 })).toBe('5 tests') }) + }) - describe('fallback plural rules', () => { - const testNumbers = [0, 1, 2, 5, 25, 100] + describe('.selectPluralRuleFromFallback', () => { + const testNumbers = [0, 1, 2, 5, 25, 100] - it('returns the correct plural form for a given count (Arabic rules)', () => { - const locale = 'ar' - const localeNumbers = [105, 125] + it('returns the correct plural form for a given count (Arabic rules)', () => { + const locale = 'ar' + const localeNumbers = [105, 125] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.arabic(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (Chinese rules)', () => { - const locale = 'zh' - const localeNumbers = [] + it('returns the correct plural form for a given count (Chinese rules)', () => { + const locale = 'zh' + const localeNumbers = [] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.chinese(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (French rules)', () => { - const locale = 'fr' - const localeNumbers = [] + it('returns the correct plural form for a given count (French rules)', () => { + const locale = 'fr' + const localeNumbers = [] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.french(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (German rules)', () => { - const locale = 'de' - const localeNumbers = [] + it('returns the correct plural form for a given count (German rules)', () => { + const locale = 'de' + const localeNumbers = [] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.german(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (Irish rules)', () => { - const locale = 'ga' - const localeNumbers = [9] + it('returns the correct plural form for a given count (Irish rules)', () => { + const locale = 'ga' + const localeNumbers = [9] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.irish(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (Russian rules)', () => { - const locale = 'ru' - const localeNumbers = [3, 13, 101] + it('returns the correct plural form for a given count (Russian rules)', () => { + const locale = 'ru' + const localeNumbers = [3, 13, 101] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.russian(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (Scottish rules)', () => { - const locale = 'gd' - const localeNumbers = [15] + it('returns the correct plural form for a given count (Scottish rules)', () => { + const locale = 'gd' + const localeNumbers = [15] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.scottish(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (Spanish rules)', () => { - const locale = 'es' - const localeNumbers = [1000000, 2000000] + it('returns the correct plural form for a given count (Spanish rules)', () => { + const locale = 'es' + const localeNumbers = [1000000, 2000000] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.spanish(num)).toBe(intl.select(num)) - }) + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) + }) - it('returns the correct plural form for a given count (Welsh rules)', () => { - const locale = 'cy' - const localeNumbers = [3, 6] + it('returns the correct plural form for a given count (Welsh rules)', () => { + const locale = 'cy' + const localeNumbers = [3, 6] - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const i18n = new I18n({}, { locale }) + const intl = new Intl.PluralRules(locale) + + testNumbers.concat(localeNumbers).forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) + }) + }) + + it('returns "other" for unsupported locales', () => { + const locale = 'la' + const i18n = new I18n({}, { locale }) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.pluralRules.welsh(num)).toBe(intl.select(num)) - }) + testNumbers.forEach(num => { + expect(i18n.selectPluralRuleFromFallback(num)).toBe('other') }) }) }) From ee5882dd0e322317577f1a0fe0d8c333bd0f3672 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Oct 2022 15:26:57 +0100 Subject: [PATCH 11/14] Simplify plural rule tests by using `it.each` --- src/govuk/i18n.unit.test.mjs | 123 ++++++----------------------------- 1 file changed, 20 insertions(+), 103 deletions(-) diff --git a/src/govuk/i18n.unit.test.mjs b/src/govuk/i18n.unit.test.mjs index c9616d0783..89a078eb6c 100644 --- a/src/govuk/i18n.unit.test.mjs +++ b/src/govuk/i18n.unit.test.mjs @@ -194,112 +194,27 @@ describe('I18n', () => { }) describe('.selectPluralRuleFromFallback', () => { - const testNumbers = [0, 1, 2, 5, 25, 100] - - it('returns the correct plural form for a given count (Arabic rules)', () => { - const locale = 'ar' - const localeNumbers = [105, 125] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (Chinese rules)', () => { - const locale = 'zh' - const localeNumbers = [] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (French rules)', () => { - const locale = 'fr' - const localeNumbers = [] - + // The locales we want to test, with numbers for any 'special cases' in + // those locales we want to ensure are handled correctly + const locales = [ + ['ar', [105, 125]], + ['zh'], + ['fr'], + ['de'], + ['ga', [9]], + ['ru', [3, 13, 101]], + ['gd', [15]], + ['es', [1000000, 2000000]], + ['cy', [3, 6]] + ] + + it.each(locales)('matches `Intl.PluralRules.select()` for %s locale', (locale, localeNumbers = []) => { const i18n = new I18n({}, { locale }) const intl = new Intl.PluralRules(locale) - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (German rules)', () => { - const locale = 'de' - const localeNumbers = [] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (Irish rules)', () => { - const locale = 'ga' - const localeNumbers = [9] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) + const numbersToTest = [0, 1, 2, 5, 25, 100, ...localeNumbers] - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (Russian rules)', () => { - const locale = 'ru' - const localeNumbers = [3, 13, 101] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (Scottish rules)', () => { - const locale = 'gd' - const localeNumbers = [15] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (Spanish rules)', () => { - const locale = 'es' - const localeNumbers = [1000000, 2000000] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { - expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) - }) - }) - - it('returns the correct plural form for a given count (Welsh rules)', () => { - const locale = 'cy' - const localeNumbers = [3, 6] - - const i18n = new I18n({}, { locale }) - const intl = new Intl.PluralRules(locale) - - testNumbers.concat(localeNumbers).forEach(num => { + numbersToTest.forEach(num => { expect(i18n.selectPluralRuleFromFallback(num)).toBe(intl.select(num)) }) }) @@ -308,7 +223,9 @@ describe('I18n', () => { const locale = 'la' const i18n = new I18n({}, { locale }) - testNumbers.forEach(num => { + const numbersToTest = [0, 1, 2, 5, 25, 100] + + numbersToTest.forEach(num => { expect(i18n.selectPluralRuleFromFallback(num)).toBe('other') }) }) From db8a51fc188875bd72a9461b4c5c715d9cf3d55b Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Oct 2022 15:29:14 +0100 Subject: [PATCH 12/14] Organise unit tests by called function --- src/govuk/i18n.unit.test.mjs | 210 +++++++++++++++++------------------ 1 file changed, 105 insertions(+), 105 deletions(-) diff --git a/src/govuk/i18n.unit.test.mjs b/src/govuk/i18n.unit.test.mjs index 89a078eb6c..0789da7ede 100644 --- a/src/govuk/i18n.unit.test.mjs +++ b/src/govuk/i18n.unit.test.mjs @@ -5,7 +5,7 @@ import { I18n } from './i18n.mjs' describe('I18n', () => { - describe('retrieving translations', () => { + describe('.t', () => { let config = {} beforeEach(() => { @@ -37,116 +37,139 @@ describe('I18n', () => { const i18n = new I18n(config) expect(() => i18n.t()).toThrow('i18n: lookup key missing') }) - }) - describe('string interpolation', () => { - const config = { - nameString: 'My name is %{name}' - } + describe('string interpolation', () => { + const config = { + nameString: 'My name is %{name}' + } - it('throws an error if the options data is not present', () => { - const i18n = new I18n(config) - expect(() => { i18n.t('nameString') }).toThrowError('i18n: cannot replace placeholders in string if no option data provided') - }) + it('throws an error if the options data is not present', () => { + const i18n = new I18n(config) + expect(() => { i18n.t('nameString') }).toThrowError('i18n: cannot replace placeholders in string if no option data provided') + }) - it('throws an error if the options object is empty', () => { - const i18n = new I18n(config) - expect(() => { i18n.t('nameString', {}) }).toThrowError('i18n: no data found to replace %{name} placeholder in string') - }) + it('throws an error if the options object is empty', () => { + const i18n = new I18n(config) + expect(() => { i18n.t('nameString', {}) }).toThrowError('i18n: no data found to replace %{name} placeholder in string') + }) - it('throws an error if the options object does not have a matching key', () => { - const i18n = new I18n(config) - expect(() => { i18n.t('nameString', { unrelatedThing: 'hello' }) }).toThrowError('i18n: no data found to replace %{name} placeholder in string') - }) + it('throws an error if the options object does not have a matching key', () => { + const i18n = new I18n(config) + expect(() => { i18n.t('nameString', { unrelatedThing: 'hello' }) }).toThrowError('i18n: no data found to replace %{name} placeholder in string') + }) - it('only matches %{} as a placeholder', () => { - const i18n = new I18n({ - price: '%{name}, this } item %{ costs $5.00' + it('only matches %{} as a placeholder', () => { + const i18n = new I18n({ + price: '%{name}, this } item %{ costs $5.00' + }) + expect(i18n.t('price', { name: 'John' })).toBe('John, this } item %{ costs $5.00') }) - expect(i18n.t('price', { name: 'John' })).toBe('John, this } item %{ costs $5.00') - }) - it('can lookup a placeholder value with non-alphanumeric key', () => { - const i18n = new I18n({ - age: 'My age is %{current-age}' + it('can lookup a placeholder value with non-alphanumeric key', () => { + const i18n = new I18n({ + age: 'My age is %{current-age}' + }) + expect(i18n.t('age', { 'current-age': 55 })).toBe('My age is 55') }) - expect(i18n.t('age', { 'current-age': 55 })).toBe('My age is 55') - }) - it('can lookup a placeholder value with reserved name as key', () => { - const i18n = new I18n({ - age: 'My age is %{valueOf}' + it('can lookup a placeholder value with reserved name as key', () => { + const i18n = new I18n({ + age: 'My age is %{valueOf}' + }) + expect(i18n.t('age', { valueOf: 55 })).toBe('My age is 55') }) - expect(i18n.t('age', { valueOf: 55 })).toBe('My age is 55') - }) - it('throws an expected error if placeholder key with reserved name is not present in options', () => { - const i18n = new I18n({ - age: 'My age is %{valueOf}' + it('throws an expected error if placeholder key with reserved name is not present in options', () => { + const i18n = new I18n({ + age: 'My age is %{valueOf}' + }) + expect(() => { i18n.t('age', {}) }).toThrowError('i18n: no data found to replace %{valueOf} placeholder in string') }) - expect(() => { i18n.t('age', {}) }).toThrowError('i18n: no data found to replace %{valueOf} placeholder in string') - }) - it('replaces the placeholder with the provided data', () => { - const i18n = new I18n(config) - expect(i18n.t('nameString', { name: 'John' })).toBe('My name is John') - }) + it('replaces the placeholder with the provided data', () => { + const i18n = new I18n(config) + expect(i18n.t('nameString', { name: 'John' })).toBe('My name is John') + }) - it('can replace a placeholder with a falsey value', () => { - const i18n = new I18n({ - nameString: 'My name is %{name}', - stock: 'Stock level: %{quantity}' + it('can replace a placeholder with a falsey value', () => { + const i18n = new I18n({ + nameString: 'My name is %{name}', + stock: 'Stock level: %{quantity}' + }) + expect(i18n.t('nameString', { name: '' })).toBe('My name is ') + expect(i18n.t('stock', { quantity: 0 })).toBe('Stock level: 0') }) - expect(i18n.t('nameString', { name: '' })).toBe('My name is ') - expect(i18n.t('stock', { quantity: 0 })).toBe('Stock level: 0') - }) - it('can pass false as a placeholder replacement to hide the value', () => { - const i18n = new I18n({ - personalDetails: 'John Smith %{age}' + it('can pass false as a placeholder replacement to hide the value', () => { + const i18n = new I18n({ + personalDetails: 'John Smith %{age}' + }) + expect(i18n.t('personalDetails', { age: false })).toBe('John Smith ') }) - expect(i18n.t('personalDetails', { age: false })).toBe('John Smith ') - }) - it('selects the correct data to replace in the string', () => { - const i18n = new I18n(config) - expect(i18n.t('nameString', { number: 50, name: 'Claire', otherName: 'Zoe' })).toBe('My name is Claire') - }) + it('selects the correct data to replace in the string', () => { + const i18n = new I18n(config) + expect(i18n.t('nameString', { number: 50, name: 'Claire', otherName: 'Zoe' })).toBe('My name is Claire') + }) - it('replaces multiple placeholders, if present', () => { - const i18n = new I18n({ - nameString: 'Their name is %{name}. %{name} is %{age} years old' + it('replaces multiple placeholders, if present', () => { + const i18n = new I18n({ + nameString: 'Their name is %{name}. %{name} is %{age} years old' + }) + expect(i18n.t('nameString', { number: 50, name: 'Andrew', otherName: 'Vic', age: 22 })).toBe('Their name is Andrew. Andrew is 22 years old') }) - expect(i18n.t('nameString', { number: 50, name: 'Andrew', otherName: 'Vic', age: 22 })).toBe('Their name is Andrew. Andrew is 22 years old') - }) - it('nested placeholder only resolves with a matching key', () => { - const i18n = new I18n({ - nameString: 'Their name is %{name%{age}}' + it('nested placeholder only resolves with a matching key', () => { + const i18n = new I18n({ + nameString: 'Their name is %{name%{age}}' + }) + expect(i18n.t('nameString', { name: 'Andrew', age: 55, 'name%{age}': 'Testing' })).toBe('Their name is Testing') }) - expect(i18n.t('nameString', { name: 'Andrew', age: 55, 'name%{age}': 'Testing' })).toBe('Their name is Testing') - }) - it('handles placeholder-style text within options values', () => { - const i18n = new I18n(config) - expect(i18n.t('nameString', { name: '%{name}' })).toBe('My name is %{name}') - }) + it('handles placeholder-style text within options values', () => { + const i18n = new I18n(config) + expect(i18n.t('nameString', { name: '%{name}' })).toBe('My name is %{name}') + }) + + it('formats numbers that are passed as placeholders', () => { + const translations = { ageString: 'I am %{age} years old' } + const i18nEn = new I18n(translations, { locale: 'en' }) + const i18nDe = new I18n(translations, { locale: 'de' }) - it('formats numbers that are passed as placeholders', () => { - const translations = { ageString: 'I am %{age} years old' } - const i18nEn = new I18n(translations, { locale: 'en' }) - const i18nDe = new I18n(translations, { locale: 'de' }) + expect(i18nEn.t('ageString', { age: 2000 })).toBe('I am 2,000 years old') + expect(i18nDe.t('ageString', { age: 2000 })).toBe('I am 2.000 years old') + }) - expect(i18nEn.t('ageString', { age: 2000 })).toBe('I am 2,000 years old') - expect(i18nDe.t('ageString', { age: 2000 })).toBe('I am 2.000 years old') + it('does not format number-like strings that are passed as placeholders', () => { + const i18n = new I18n({ + yearString: 'Happy new year %{year}' + }) + + expect(i18n.t('yearString', { year: '2023' })).toBe('Happy new year 2023') + }) }) - it('does not format number-like strings that are passed as placeholders', () => { - const i18n = new I18n({ - yearString: 'Happy new year %{year}' + describe('pluralisation', () => { + it('throws an error if a required plural form is not provided ', () => { + const i18n = new I18n({ + 'test.other': 'testing testing' + }, { + locale: 'en' + }) + expect(() => { i18n.t('test', { count: 1 }) }).toThrowError('i18n: Plural form ".one" is required for "en" locale') }) - expect(i18n.t('yearString', { year: '2023' })).toBe('Happy new year 2023') + it('interpolates the count variable into the correct plural form', () => { + const i18n = new I18n({ + 'test.one': '%{count} test', + 'test.other': '%{count} tests' + }, { + locale: 'en' + }) + + expect(i18n.t('test', { count: 1 })).toBe('1 test') + expect(i18n.t('test', { count: 5 })).toBe('5 tests') + }) }) }) @@ -170,29 +193,6 @@ describe('I18n', () => { }) }) - describe('pluralisation', () => { - it('throws an error if a required plural form is not provided ', () => { - const i18n = new I18n({ - 'test.other': 'testing testing' - }, { - locale: 'en' - }) - expect(() => { i18n.t('test', { count: 1 }) }).toThrowError('i18n: Plural form ".one" is required for "en" locale') - }) - - it('interpolates the count variable into the correct plural form', () => { - const i18n = new I18n({ - 'test.one': '%{count} test', - 'test.other': '%{count} tests' - }, { - locale: 'en' - }) - - expect(i18n.t('test', { count: 1 })).toBe('1 test') - expect(i18n.t('test', { count: 5 })).toBe('5 tests') - }) - }) - describe('.selectPluralRuleFromFallback', () => { // The locales we want to test, with numbers for any 'special cases' in // those locales we want to ensure are handled correctly From 1376462361c7b9f88651176fbc68eeaa5bb697bb Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Oct 2022 15:31:31 +0100 Subject: [PATCH 13/14] Simplify getPluralSuffix logic --- src/govuk/i18n.mjs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index 7a03e35245..7f1f48ec7f 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -142,26 +142,21 @@ I18n.prototype.hasIntlNumberFormatSupport = function () { * @returns {string} - The suffix associated with the correct pluralisation for this locale. */ I18n.prototype.getPluralSuffix = function (count) { - var keySuffix = 'other' - // Validate that the number is actually a number. // // Number(count) will turn anything that can't be converted to a Number type // into 'NaN'. isFinite filters out NaN, as it isn't a finite number. count = Number(count) - if (!isFinite(count)) { return keySuffix } + if (!isFinite(count)) { return 'other' } // Check to verify that all the requirements for Intl.PluralRules are met. // If so, we can use that instead of our custom implementation. Otherwise, // use the hardcoded fallback. if (this.hasIntlPluralRulesSupport()) { - var pluralRules = new Intl.PluralRules(this.locale) - keySuffix = pluralRules.select(count) + return new Intl.PluralRules(this.locale).select(count) } else { return this.selectPluralRuleFromFallback(count) } - - return keySuffix } /** From cd15d29b2c4f05d8056fd476962ac4c7bc5044a4 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Oct 2022 15:37:36 +0100 Subject: [PATCH 14/14] Move pluralRules and pluralRulesMap to class --- src/govuk/i18n.mjs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/govuk/i18n.mjs b/src/govuk/i18n.mjs index 7f1f48ec7f..976850f949 100644 --- a/src/govuk/i18n.mjs +++ b/src/govuk/i18n.mjs @@ -176,7 +176,7 @@ I18n.prototype.selectPluralRuleFromFallback = function (count) { var ruleset = this.getPluralRulesForLocale() if (ruleset) { - return this.pluralRules[ruleset](count) + return I18n.pluralRules[ruleset](count) } return 'other' @@ -199,9 +199,9 @@ I18n.prototype.getPluralRulesForLocale = function () { // Look through the plural rules map to find which `pluralRule` is // appropriate for our current `locale`. - for (var pluralRule in this.pluralRulesMap) { - if (Object.prototype.hasOwnProperty.call(this.pluralRulesMap, pluralRule)) { - var languages = this.pluralRulesMap[pluralRule] + for (var pluralRule in I18n.pluralRulesMap) { + if (Object.prototype.hasOwnProperty.call(I18n.pluralRulesMap, pluralRule)) { + var languages = I18n.pluralRulesMap[pluralRule] if (languages.indexOf(locale) > -1 || languages.indexOf(localeShort) > -1) { return pluralRule } @@ -239,7 +239,7 @@ I18n.prototype.getPluralRulesForLocale = function () { * Spanish: European Portuguese (pt-PT), Italian (it), Spanish (es) * Welsh: Welsh (cy) */ -I18n.prototype.pluralRulesMap = { +I18n.pluralRulesMap = { arabic: ['ar'], chinese: ['my', 'zh', 'id', 'ja', 'jv', 'ko', 'ms', 'th', 'vi'], french: ['hy', 'bn', 'fr', 'gu', 'hi', 'fa', 'pa', 'zu'], @@ -266,7 +266,7 @@ I18n.prototype.pluralRulesMap = { * @param {number} n - The `count` number being passed through. This must be a positive integer. Negative numbers and decimals aren't accounted for. * @returns {string} - The string that needs to be suffixed to the key (without separator). */ -I18n.prototype.pluralRules = { +I18n.pluralRules = { arabic: function (n) { if (n === 0) { return 'zero' } if (n === 1) { return 'one' }