Skip to content

Commit

Permalink
Do not output empty conditionals
Browse files Browse the repository at this point in the history
If `item.conditional.html` is falsy, perhaps because it’s referencing a variable that is conditionally assigned, the macro currently outputs an empty `div.govuk-checkboxes__conditional` and associates it with the input using aria-controls.

By testing for `item.conditional.html` being truthy (which is the only property in the item.conditional object used within the macro) we can avoid this.
  • Loading branch information
36degrees committed Sep 27, 2019
1 parent f481811 commit c708087
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Align ‘Warning text’ icon with first line of the content fixing [#1352](http
- [Pull request #1585: Explicitly set font weight on warning-text component](https://github.com/alphagov/govuk-frontend/pull/1585)
- [Pull request #1587: Fix height and alignment issue within header in Chrome 76+](https://github.com/alphagov/govuk-frontend/pull/1587)
- [Pull request #1589: Remove role="button" from header button](https://github.com/alphagov/govuk-frontend/pull/1589)
- [Pull request #1595: Do not output conditionally revealed content for radios or checkboxes when it's empty](https://github.com/alphagov/govuk-frontend/pull/1595)

## 3.2.0 (Feature release)

Expand Down
6 changes: 3 additions & 3 deletions src/govuk/components/checkboxes/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

{% set isConditional = false %}
{% for item in params.items %}
{% if item.conditional %}
{% if item.conditional.html %}
{% set isConditional = true %}
{% endif %}
{% endfor %}
Expand Down Expand Up @@ -75,7 +75,7 @@
<input class="govuk-checkboxes__input" id="{{ id }}" name="{{ name }}" type="checkbox" value="{{ item.value }}"
{{-" checked" if item.checked }}
{{-" disabled" if item.disabled }}
{%- if item.conditional %} data-aria-controls="{{ conditionalId }}"{% endif -%}
{%- if item.conditional.html %} data-aria-controls="{{ conditionalId }}"{% endif -%}
{%- if itemDescribedBy %} aria-describedby="{{ itemDescribedBy }}"{% endif -%}
{%- for attribute, value in item.attributes %} {{ attribute }}="{{ value }}"{% endfor -%}>
{{ govukLabel({
Expand All @@ -95,7 +95,7 @@
}) | indent(6) | trim }}
{% endif %}
</div>
{% if item.conditional %}
{% if item.conditional.html %}
<div class="govuk-checkboxes__conditional{% if not item.checked %} govuk-checkboxes__conditional--hidden{% endif %}" id="{{ conditionalId }}">
{{ item.conditional.html | safe }}
</div>
Expand Down
36 changes: 36 additions & 0 deletions src/govuk/components/checkboxes/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,42 @@ describe('Checkboxes', () => {
expect($firstInput.attr('data-aria-controls')).toBe('conditional-example-conditional')
expect($firstConditional.attr('id')).toBe('conditional-example-conditional')
})

it('omits empty conditionals', () => {
const $ = render('checkboxes', {
name: 'example-conditional',
items: [
{
value: 'foo',
text: 'Foo',
conditional: {
html: false
}
}
]
})

const $component = $('.govuk-checkboxes')
expect($component.find('.govuk-checkboxes__conditional').length).toEqual(0)
})

it('does not associate checkboxes with empty conditionals', () => {
const $ = render('checkboxes', {
name: 'example-conditional',
items: [
{
value: 'foo',
text: 'Foo',
conditional: {
html: false
}
}
]
})

const $input = $('.govuk-checkboxes__input').first()
expect($input.attr('data-aria-controls')).toBeFalsy()
})
})

it('render additional label classes', () => {
Expand Down
6 changes: 3 additions & 3 deletions src/govuk/components/radios/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

{% set isConditional = false %}
{% for item in params.items %}
{% if item.conditional %}
{% if item.conditional.html %}
{% set isConditional = true %}
{% endif %}
{% endfor %}
Expand Down Expand Up @@ -69,7 +69,7 @@
<input class="govuk-radios__input" id="{{ id }}" name="{{ params.name }}" type="radio" value="{{ item.value }}"
{{-" checked" if item.checked }}
{{-" disabled" if item.disabled }}
{%- if item.conditional %} data-aria-controls="{{ conditionalId }}"{% endif -%}
{%- if item.conditional.html %} data-aria-controls="{{ conditionalId }}"{% endif -%}
{%- if hasHint %} aria-describedby="{{ itemHintId }}"{% endif -%}
{%- for attribute, value in item.attributes %} {{ attribute }}="{{ value }}"{% endfor -%}>
{{ govukLabel({
Expand All @@ -89,7 +89,7 @@
}) | indent(6) | trim }}
{% endif %}
</div>
{% if item.conditional %}
{% if item.conditional.html %}
<div class="govuk-radios__conditional{% if not item.checked %} govuk-radios__conditional--hidden{% endif %}" id="{{ conditionalId }}">
{{ item.conditional.html | safe }}
</div>
Expand Down
44 changes: 44 additions & 0 deletions src/govuk/components/radios/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,50 @@ describe('Radios', () => {
expect($firstInput.attr('data-aria-controls')).toBe('conditional-example-conditional')
expect($firstConditional.attr('id')).toBe('conditional-example-conditional')
})

it('omits empty conditionals', () => {
const $ = render('radios', {
name: 'example-conditional',
items: [
{
value: 'yes',
text: 'Yes',
conditional: {
html: false
}
},
{
value: 'no',
text: 'No'
}
]
})

const $component = $('.govuk-radios')
expect($component.find('.govuk-radios__conditional').length).toEqual(0)
})

it('does not associate radios with empty conditionals', () => {
const $ = render('radios', {
name: 'example-conditional',
items: [
{
value: 'yes',
text: 'Yes',
conditional: {
html: false
}
},
{
value: 'no',
text: 'No'
}
]
})

const $input = $('.govuk-radios__input').first()
expect($input.attr('data-aria-controls')).toBeFalsy()
})
})

it('render divider', () => {
Expand Down

0 comments on commit c708087

Please sign in to comment.