Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add macro options to configure CharacterCount translations #2895

Merged
merged 15 commits into from
Oct 17, 2022
Merged

Add macro options to configure CharacterCount translations #2895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7a4d712
Update I18n to use '.' separated plural suffix
romaricpascal Oct 10, 2022
d7993a0
Test config key flattening is on multiple levels
romaricpascal Oct 10, 2022
e3a889e
Make CharacterCount use the dot separated plural forms
romaricpascal Oct 10, 2022
d88f1fa
Add helper macro for outputing i18n pluralisations
romaricpascal Oct 10, 2022
75d6527
Add options for passing translations to character count macro
romaricpascal Oct 10, 2022
5a01101
Fix typing of CharacterCount translations
romaricpascal Oct 11, 2022
cd41407
Fix typos
romaricpascal Oct 12, 2022
7fac761
Mark govukPluralisedI18nAttributes as private
romaricpascal Oct 13, 2022
c79ea2a
Split out logic to choose ruleset based on locale
36degrees Oct 14, 2022
c16fa49
Split out fallback logic for selecting plural rule
36degrees Oct 14, 2022
ee5882d
Simplify plural rule tests by using `it.each`
36degrees Oct 14, 2022
db8a51f
Organise unit tests by called function
36degrees Oct 14, 2022
1376462
Simplify getPluralSuffix logic
36degrees Oct 14, 2022
cd15d29
Move pluralRules and pluralRulesMap to class
36degrees Oct 14, 2022
bfccd7e
Merge pull request #2922 from alphagov/i18n-unit-test-coverage
36degrees Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions lib/jest-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -144,6 +161,7 @@ module.exports = {
getExamples,
htmlWithClassName,
nunjucksEnv,
callMacro,
render,
renderHtml,
renderSass,
Expand Down
13 changes: 10 additions & 3 deletions src/govuk/common.unit.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -48,7 +53,8 @@ describe('Common JS utilities', () => {
b: 'bat',
'c.a': 'cat',
'c.o': 'cow',
d: 'dog'
d: 'dog',
'e.l.e': 'elephant'
romaricpascal marked this conversation as resolved.
Show resolved Hide resolved
})
})

Expand All @@ -69,7 +75,8 @@ describe('Common JS utilities', () => {
b: 'bat',
'c.a': 'camel',
'c.o': 'cow',
d: 'dog'
d: 'dog',
'e.l.e': 'elephant'
})
})

Expand Down
73 changes: 38 additions & 35 deletions src/govuk/components/character-count/character-count.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {CharacterCountTranslations}
*/
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
*
Expand All @@ -25,27 +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 {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) {
Expand All @@ -54,20 +60,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)
Expand Down Expand Up @@ -371,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]
*/
8 changes: 4 additions & 4 deletions src/govuk/components/character-count/character-count.unit.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)

Expand All @@ -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: {
Expand Down
21 changes: 21 additions & 0 deletions src/govuk/components/character-count/character-count.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion src/govuk/components/character-count/template.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
{% from "../../macros/i18n.njk" import govukPluralisedI18nAttributes %}

{% from "../textarea/macro.njk" import govukTextarea %}
{% from "../hint/macro.njk" import govukHint %}

<div class="govuk-character-count" data-module="govuk-character-count"
{%- if params.maxlength %} data-maxlength="{{ params.maxlength }}"{% endif %}
{%- if params.threshold %} data-threshold="{{ params.threshold }}"{% endif %}
{%- if params.maxwords %} data-maxwords="{{ params.maxwords }}"{% endif %}>
{%- if params.maxwords %} data-maxwords="{{ params.maxwords }}"{% endif %}
{%- if params.charactersUnderLimitHtml %}{{govukPluralisedI18nAttributes('characters-under-limit', params.charactersUnderLimitHtml)}}{% endif %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super quick not-even-tried-it example, but how about a function that does all i18n attributes?

{% set i18n = params.i18n %}
{%- if i18n %}{{ govukI18nAttributes([
  ['characters-under-limit', i18n.charactersUnderLimitHtml],
  ['characters-at-limit', i18n.charactersAtLimitHtml],
  ['characters-over-limit', i18n.charactersOverLimitHtml],
  ['words-under-limit', i18n.wordsUnderLimitHtml],
  ['words-at-limit', i18n.wordsAtLimitHtml],
  ['words-over-limit', i18n.wordsOverLimitHtml]
])}}{% endif %}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is appealing, and we don't really need to introduce a separate i18n namespace for them, we can just pick the values from params directly as we do have to read them manually (unless someone knows of some black magic to go kebab-case to cameCase or the other way around with Nunjucks without adding a filter).

There's a little trickiness to it as translations can be strings or object, depending of if they're pluralised or not. Using the current helper, the template makes it clear which attribute expects which, so we'd need to devise an API that lets us do the same, maybe:

{{ govukI18nAttributes({
  'characters-under-limit': {plurals: {params.charactersUnderLimitHtml}},
  'characters-over-limit': {plurals: {params.charactersOverLimitHtml}},
  'characters-at-limit': {string: params.charactersAtLimitHtml},
  'words-under-limit': {plurals: {params.wordsUnderLimitHtml}},
  'words-over-limit': {plurals: {params.wordsOverLimitHtml}},
  'words-at-limit': {string: params.wordsAtLimitHtml}
 })}}

That said, I think there's ground for a codebase wise look at some abstraction around data attributes, like a govukDataAttributes helper on a similar model, as setting them is always the same thing:

  • check if the value is truthy (or not undefined in a few cases)
  • set data-{{name}}={{value}} (sometimes escaped)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an issue to track: #2917 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romaricpascal Yeah, look less at the "how" and more of a nice neat way to generate attributes

We already have this syntax elsewhere for .attributes:

{%- for attribute, value in message.attributes %} {{attribute}}="{{value}}"{% endfor %}

Then other "ad-hoc" data attributes in places:

{%- if params.disableAutoFocus !== undefined %} data-disable-auto-focus="{{ params.disableAutoFocus }}"{% endif %}

If each template had a map (or object) of all its available attributes, defaults, parameter overrides etc, you could write both regular and data attributes into the template in a nice unit-testable way

This certainly isn't a blocker, just an opportunity to makes the template look neater

{%- 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,
Expand Down
25 changes: 24 additions & 1 deletion src/govuk/components/character-count/template.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
})
Loading