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.

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

Fix Error Summary component outputting list HTML when no errorList is provided #4971

Merged
merged 1 commit into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ It was previously only possible to use tabular numbers by using the `govuk-font-

This change was introduced in [pull request #4973: Add override class for tabular numbers](https://github.com/alphagov/govuk-frontend/pull/4973).

### Deprecations
### Deprecated features

#### Importing layers using `all` files

Expand Down Expand Up @@ -70,6 +70,7 @@ We've made fixes to GOV.UK Frontend in the following pull requests:
- [#4942: Remove duplicate `errorMessage` argument for the password input component](https://github.com/alphagov/govuk-frontend/pull/4942) - thanks to [Tim South](https://github.com/tim-s-ccs) for contributing this change
- [#4961: Fix tree-shaking when importing `govuk-frontend`](https://github.com/alphagov/govuk-frontend/pull/4961)
- [#4963: Fix input value not being set if the value was '0'](https://github.com/alphagov/govuk-frontend/pull/4963) – thanks to [@dwp-dmitri-algazin](https://github.com/dwp-dmitri-algazin) for reporting this issue
- [#4971: Fix Error Summary component outputting list HTML when no `errorList` is provided](https://github.com/alphagov/govuk-frontend/pull/4971)

## 5.3.1 (Fix release)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,21 @@

.govuk-error-summary__body {
p {
margin-top: 0;
@include govuk-responsive-margin(4, "bottom");
margin-bottom: 0;
}

> * + * {
@include govuk-responsive-margin(4, "top");
}
}

// Cross-component class - adjusts styling of list component
.govuk-error-summary__list {
margin-top: 0;
margin-bottom: 0;
}

// Remove the bottom margin from the last list item
.govuk-error-summary__list li:last-child {
margin-bottom: 0;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ params:
description: Not strictly a parameter but [Nunjucks code convention](https://mozilla.github.io/nunjucks/templating.html#call). Using a `call` block enables you to call a macro with all the text inside the tag. This is helpful if you want to pass a lot of content into a macro. To use it, you will need to wrap the entire error summary component in a `call` block.
- name: errorList
type: array
required: true
description: The list of errors to include in the error summary.
required: false
description: A list of errors to include in the error summary.
params:
- name: href
type: string
Expand Down Expand Up @@ -77,6 +77,10 @@ examples:
- text: Invalid username or password
- text: Agree to the terms of service to log in
href: '#example-error-1'
- name: with description only
options:
titleText: There is a problem
descriptionText: The file couldn't be loaded. Try again later.
- name: with everything
options:
titleText: There is a problem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,22 @@
{{ caller() if caller else (params.descriptionHtml | safe | trim | indent(8) if params.descriptionHtml else params.descriptionText) }}
</p>
{% endif %}
<ul class="govuk-list govuk-error-summary__list">
{% for item in params.errorList %}
<li>
{% if item.href %}
<a href="{{ item.href }}"
{{- govukAttributes(item.attributes) }}>
{{- item.html | safe | trim | indent(12) if item.html else item.text -}}
</a>
{% else %}
{{ item.html | safe | trim | indent(10) if item.html else item.text }}
{% endif %}
</li>
{% endfor %}
</ul>
{% if params.errorList | length %}
<ul class="govuk-list govuk-error-summary__list">
{% for item in params.errorList %}
<li>
{% if item.href %}
<a href="{{ item.href }}"
{{- govukAttributes(item.attributes) }}>
{{- item.html | safe | trim | indent(12) if item.html else item.text -}}
</a>
{% else %}
{{ item.html | safe | trim | indent(10) if item.html else item.text }}
{% endif %}
</li>
{% endfor %}
</ul>
{% endif %}
</div>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ describe('Error-summary', () => {
expect(summaryTitle).toBe('There is a problem')
})

it('renders error list element', () => {
const $ = render('error-summary', examples.default)
const $errorList = $('.govuk-error-summary__list')

expect($errorList).toHaveLength(1)
})

it('number of error items matches the number of items specified', () => {
const $ = render('error-summary', examples.default)
const errorList = $('.govuk-error-summary .govuk-error-summary__list li')
Expand Down Expand Up @@ -112,6 +119,13 @@ describe('Error-summary', () => {
expect($component.attr('second-attribute')).toBe('bar')
})

it("doesn't render the error list element if no errors are passed", () => {
const $ = render('error-summary', examples['with description only'])
const $errorList = $('.govuk-error-summary__list')

expect($errorList).toHaveLength(0)
})

it('renders anchor tag with attributes', () => {
const $ = render('error-summary', examples['error list with attributes'])

Expand Down