Skip to content

Commit

Permalink
Add failing test for invalid HTML in summary list
Browse files Browse the repository at this point in the history
When a summary list has actions in only some of the rows, we include an empty span with the `govuk-summary-list__actions` class in the rows that do not have actions:

```
dl.govuk-summary-list
  div.govuk-summary-list__row
     dt.govuk-summary-list__key
     dd.govuk-summary-list__value
     dd.govuk-summary-list__actions <-- List of actions
  div.govuk-summary-list__row
     dt.govuk-summary-list__key
     dd.govuk-summary-list__value
     span.govuk-summary-list__actions <-- Empty
```

This is required because (from the tablet breakpoint upwards) the summary list is displayed as a table, and the bottom border on each 'row' is applied to the individual 'cells' within that row (the key, value, and actions elements). Omitting the actions element from a row therefore means that that part of the row does not have a bottom border.

However, this is invalid HTML as the content model for the `<dl>` element [1] only allows for 'either: Zero or more groups each consisting of one or more dt elements followed by one or more dd elements, optionally intermixed with script-supporting elements. Or: One or more div elements, optionally intermixed with script-supporting elements.'

As such, it's not valid for a <span> to be a direct child of a `<dl>`. This has been flagged by automated testing tools such as Axe (as seen in #1806) – although we're not aware of any issues that it would cause for users in practise.

Add a failing test that covers this by running Axe on the example that has rows both with and without actions.

The test fails with:

> Summary list › rows › actions › passes accessibility tests when some rows do not have actions
>
> expect(received).toHaveNoViolations(expected)
>
> Expected the HTML found at $('dl') to have no violations:
>
> <dl class="govuk-summary-list">
>
> Received:
>
> "<dl> elements must only directly contain properly-ordered <dt> and <dd> groups, <script>, <template> or <div> elements (definition-list)"
>
> Fix all of the following:
>    List element has direct children that are not allowed inside <dt> or <dd> elements
>
> You can find more information on this issue here:
> https://dequeuniversity.com/rules/axe/4.2/definition-list?application=axeAPI

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
  • Loading branch information
36degrees committed Oct 14, 2021
1 parent d70db37 commit 792e27b
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/govuk/components/summary-list/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ describe('Summary list', () => {
expect($action[1].tagName).toEqual('span')
expect($($action[1]).text()).toEqual('')
})

it('passes accessibility tests when only some rows have actions', async () => {
const $ = render('summary-list', examples['with some actions'])

const results = await axe($.html())
expect(results).toHaveNoViolations()
})
})
})
})

0 comments on commit 792e27b

Please sign in to comment.