-
Notifications
You must be signed in to change notification settings - Fork 329
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
Update checkboxes and radio buttons to include item hint classes on i… #1648
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,7 +88,7 @@ | |
{% if hasHint %} | ||
{{ govukHint({ | ||
id: itemHintId, | ||
classes: 'govuk-checkboxes__hint', | ||
classes: 'govuk-checkboxes__hint' + (' ' + item.hint.classes if item.hint.classes), | ||
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. Non-blocking I wonder if we could avoid the extra whitespace by doing something like
Hopefully this'd output the space only when there are classes. 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. The current approach doesnt add whitespace after all so works well. |
||
attributes: item.hint.attributes, | ||
html: item.hint.html, | ||
text: item.hint.text | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -560,7 +560,12 @@ describe('Checkboxes', () => { | |
|
||
describe('when they include an error message', () => { | ||
it('renders the error message', () => { | ||
const $ = render('checkboxes', examples['with all fieldset attributes']) | ||
const $ = render('checkboxes', { | ||
name: 'example', | ||
errorMessage: { | ||
text: 'Please select an option' | ||
} | ||
}) | ||
|
||
expect(htmlWithClassName($, '.govuk-error-message')).toMatchSnapshot() | ||
}) | ||
|
@@ -667,13 +672,50 @@ describe('Checkboxes', () => { | |
|
||
describe('when they include a hint', () => { | ||
it('renders the hint', () => { | ||
const $ = render('checkboxes', examples['with all fieldset attributes']) | ||
const $ = render('checkboxes', { | ||
name: 'example', | ||
hint: { | ||
text: 'If you have dual nationality, select all options that are relevant to you.' | ||
}, | ||
items: [ | ||
{ | ||
value: 'british', | ||
text: 'British', | ||
hint: { | ||
text: 'Hint for british option here' | ||
} | ||
}, | ||
{ | ||
value: 'irish', | ||
text: 'Irish' | ||
}, | ||
{ | ||
hint: { | ||
text: 'Hint for other option here', | ||
classes: 'app-checkboxes__hint-other', | ||
attributes: { | ||
'data-test-attribute': true | ||
} | ||
} | ||
} | ||
] | ||
}) | ||
|
||
expect(htmlWithClassName($, '.govuk-hint')).toMatchSnapshot() | ||
}) | ||
|
||
it('associates the fieldset as "described by" the hint', () => { | ||
const $ = render('checkboxes', examples['with all fieldset attributes']) | ||
const $ = render('checkboxes', { | ||
name: 'example', | ||
fieldset: { | ||
legend: { | ||
text: 'What is your nationality?' | ||
} | ||
}, | ||
hint: { | ||
text: 'If you have dual nationality, select all options that are relevant to you.' | ||
} | ||
}) | ||
|
||
const $fieldset = $('.govuk-fieldset') | ||
const $hint = $('.govuk-hint') | ||
|
@@ -687,11 +729,19 @@ describe('Checkboxes', () => { | |
|
||
it('associates the fieldset as "described by" the hint and parent fieldset', () => { | ||
const describedById = 'some-id' | ||
const params = examples['with all fieldset attributes'] | ||
|
||
params.fieldset.describedBy = describedById | ||
|
||
const $ = render('checkboxes', params) | ||
const $ = render('checkboxes', { | ||
name: 'example', | ||
fieldset: { | ||
describedBy: describedById, | ||
legend: { | ||
text: 'What is your nationality?' | ||
} | ||
}, | ||
hint: { | ||
text: 'If you have dual nationality, select all options that are relevant to you.' | ||
} | ||
}) | ||
const $fieldset = $('.govuk-fieldset') | ||
const $hint = $('.govuk-hint') | ||
|
||
|
@@ -700,13 +750,25 @@ describe('Checkboxes', () => { | |
) | ||
|
||
expect($fieldset.attr('aria-describedby')).toMatch(hintId) | ||
delete params.fieldset.describedBy | ||
}) | ||
}) | ||
|
||
describe('when they include both a hint and an error message', () => { | ||
it('associates the fieldset as described by both the hint and the error message', () => { | ||
const $ = render('checkboxes', examples['with all fieldset attributes']) | ||
const $ = render('checkboxes', { | ||
name: 'example', | ||
errorMessage: { | ||
text: 'Please select an option' | ||
}, | ||
fieldset: { | ||
legend: { | ||
text: 'What is your nationality?' | ||
} | ||
}, | ||
hint: { | ||
text: 'If you have dual nationality, select all options that are relevant to you.' | ||
} | ||
}) | ||
|
||
const $fieldset = $('.govuk-fieldset') | ||
const errorMessageId = $('.govuk-error-message').attr('id') | ||
|
@@ -722,11 +784,22 @@ describe('Checkboxes', () => { | |
|
||
it('associates the fieldset as described by the hint, error message and parent fieldset', () => { | ||
const describedById = 'some-id' | ||
const params = examples['with all fieldset attributes'] | ||
|
||
params.fieldset.describedBy = describedById | ||
|
||
const $ = render('checkboxes', params) | ||
const $ = render('checkboxes', { | ||
name: 'example', | ||
errorMessage: { | ||
text: 'Please select an option' | ||
}, | ||
fieldset: { | ||
describedBy: describedById, | ||
legend: { | ||
text: 'What is your nationality?' | ||
} | ||
}, | ||
hint: { | ||
text: 'If you have dual nationality, select all options that are relevant to you.' | ||
} | ||
}) | ||
|
||
const $fieldset = $('.govuk-fieldset') | ||
const errorMessageId = $('.govuk-error-message').attr('id') | ||
|
@@ -738,14 +811,18 @@ describe('Checkboxes', () => { | |
|
||
expect($fieldset.attr('aria-describedby')) | ||
.toMatch(combinedIds) | ||
|
||
delete params.fieldset.describedBy | ||
}) | ||
}) | ||
|
||
describe('nested dependant components', () => { | ||
it('have correct nesting order', () => { | ||
const $ = render('checkboxes', examples['with all fieldset attributes']) | ||
const $ = render('checkboxes', { | ||
fieldset: { | ||
legend: { | ||
text: 'What is your nationality?' | ||
} | ||
} | ||
}) | ||
|
||
const $component = $('.govuk-form-group > .govuk-fieldset > .govuk-checkboxes') | ||
expect($component.length).toBeTruthy() | ||
|
@@ -777,7 +854,25 @@ describe('Checkboxes', () => { | |
}) | ||
|
||
it('passes through fieldset params without breaking', () => { | ||
const $ = render('checkboxes', examples['with all fieldset attributes']) | ||
const $ = render('checkboxes', { | ||
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. "with all fieldset attributes" specified It would be nice to have a test for 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. As far as I know idPrefix is only needed for items, which is why the snapshot has not changed, so I think that'd be something to do later. |
||
name: 'example', | ||
errorMessage: { | ||
text: 'Please select an option' | ||
}, | ||
fieldset: { | ||
classes: 'app-fieldset--custom-modifier', | ||
attributes: { | ||
'data-attribute': 'value', | ||
'data-second-attribute': 'second-value' | ||
}, | ||
legend: { | ||
text: 'What is your nationality?' | ||
} | ||
}, | ||
hint: { | ||
text: 'If you have dual nationality, select all options that are relevant to you.' | ||
} | ||
}) | ||
|
||
expect(htmlWithClassName($, '.govuk-fieldset')).toMatchSnapshot() | ||
}) | ||
|
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.
Nit: capitalise "british".