-
Notifications
You must be signed in to change notification settings - Fork 339
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.
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
Allow CharacterCount component to receive i18n config via JS #2887
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
ed82784
Refactor character count message generation
romaricpascal fcd3ec1
Add unit test for count message formatting
romaricpascal 9e8e2d1
Use I18n for formatting count message
romaricpascal 0400810
Add test for passing different translations
romaricpascal 71b7a3b
Add tests for formatting of numbers in CharacterCount
romaricpascal 88fcbc3
Add separate translation key when limit is met
romaricpascal 66d8a1b
Default character count locale to closest 'lang' attribute
romaricpascal 74d3b94
Update JSDoc
romaricpascal 5fe3fc5
Rewrite test checking the closestAttributeValue reads up the DOM tree
romaricpascal 152c562
Remove configuration of locale via JavaScript
romaricpascal 10d6bc2
Tidy up character count translation tests
romaricpascal a2a4934
Revert translation string for when limit is met
romaricpascal 4dcdeae
Remove dom-helpers
romaricpascal a5ed606
Fix out of sync values in JSDoc
romaricpascal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
119 changes: 119 additions & 0 deletions
119
src/govuk/components/character-count/character-count.unit.test.mjs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
/** | ||
* @jest-environment jsdom | ||
*/ | ||
|
||
import CharacterCount from './character-count.mjs' | ||
|
||
describe('CharacterCount', () => { | ||
describe('formatCountMessage', () => { | ||
describe('default configuration', () => { | ||
let component | ||
beforeAll(() => { | ||
// The component won't initialise if we don't pass it an element | ||
component = new CharacterCount(document.createElement('div')) | ||
}) | ||
|
||
const cases = [ | ||
{ number: 1, type: 'characters', expected: 'You have 1 character remaining' }, | ||
{ number: 10, type: 'characters', expected: 'You have 10 characters remaining' }, | ||
{ number: -1, type: 'characters', expected: 'You have 1 character too many' }, | ||
{ number: -10, type: 'characters', expected: 'You have 10 characters too many' }, | ||
{ number: 0, type: 'characters', expected: 'You have 0 characters remaining' }, | ||
{ number: 1, type: 'words', expected: 'You have 1 word remaining' }, | ||
{ number: 10, type: 'words', expected: 'You have 10 words remaining' }, | ||
{ number: -1, type: 'words', expected: 'You have 1 word too many' }, | ||
{ number: -10, type: 'words', expected: 'You have 10 words too many' }, | ||
{ number: 0, type: 'words', expected: 'You have 0 words remaining' } | ||
] | ||
it.each(cases)( | ||
'picks the relevant translation for $number $type', | ||
function test ({ number, type, expected }) { | ||
expect(component.formatCountMessage(number, type)).toEqual(expected) | ||
} | ||
) | ||
|
||
it('formats the number inserted in the message', () => { | ||
expect(component.formatCountMessage(10000, 'words')).toEqual('You have 10,000 words remaining') | ||
expect(component.formatCountMessage(-10000, 'words')).toEqual('You have 10,000 words too many') | ||
}) | ||
}) | ||
|
||
describe('i18n', () => { | ||
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}' | ||
}) | ||
|
||
expect(component.formatCountMessage(1, 'characters')).toEqual('Custom text. Count: 1') | ||
expect(component.formatCountMessage(-10, 'characters')).toEqual('Different custom text. Count: 10') | ||
// Other keys remain untouched | ||
expect(component.formatCountMessage(10, 'characters')).toEqual('You have 10 characters remaining') | ||
}) | ||
|
||
it('uses specific keys for when limit is reached', () => { | ||
const component = new CharacterCount(document.createElement('div'), { | ||
i18n: { charactersAtLimit: 'Custom text.' }, | ||
'i18n.wordsAtLimit': 'Different custom text.' | ||
}) | ||
|
||
expect(component.formatCountMessage(0, 'characters')).toEqual('Custom text.') | ||
expect(component.formatCountMessage(0, 'words')).toEqual('Different custom text.') | ||
}) | ||
}) | ||
|
||
describe('lang attribute configuration', () => { | ||
it('overrides the locale when set on the element', () => { | ||
const $div = document.createElement('div') | ||
$div.setAttribute('lang', 'de') | ||
|
||
const component = new CharacterCount($div) | ||
|
||
expect(component.formatCountMessage(10000, 'words')).toEqual('You have 10.000 words remaining') | ||
}) | ||
|
||
it('overrides the locale when set on an ancestor', () => { | ||
const $parent = document.createElement('div') | ||
$parent.setAttribute('lang', 'de') | ||
|
||
const $div = document.createElement('div') | ||
$parent.appendChild($div) | ||
|
||
const component = new CharacterCount($div) | ||
|
||
expect(component.formatCountMessage(10000, 'words')).toEqual('You have 10.000 words remaining') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Super minor one, but any thoughts on logically grouping what the test is doing? const $parent = document.createElement('div')
const $div = document.createElement('div')
$parent.setAttribute('lang', 'de')
$parent.appendChild($div)
const component = new CharacterCount($div)
const countMessage = component.formatCountMessage(10000, 'words')
expect(countMessage).toEqual('You have 10.000 words remaining')
Helps with readability slightly, not a blocker though |
||
}) | ||
}) | ||
|
||
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}') | ||
|
||
const component = new CharacterCount($div) | ||
|
||
expect(component.formatCountMessage(1, 'characters')).toEqual('Custom text. Count: 1') | ||
// Other keys remain untouched | ||
expect(component.formatCountMessage(10, 'characters')).toEqual('You have 10 characters remaining') | ||
}) | ||
|
||
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}') | ||
|
||
const component = new CharacterCount($div, { | ||
i18n: { | ||
charactersUnderLimitOne: 'Different custom text. Count: %{count}' | ||
} | ||
}) | ||
expect(component.formatCountMessage(1, 'characters')).toEqual('Custom text. Count: 1') | ||
// Other keys remain untouched | ||
expect(component.formatCountMessage(10, 'characters')).toEqual('You have 10 characters remaining') | ||
}) | ||
}) | ||
}) | ||
}) | ||
}) | ||
}) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the polyfill definitely go here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to merge with this line, we're going to address it before the next release under #2907