From d70db372014a5a06e3c5ee18c22f44eff3744c69 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Wed, 8 Sep 2021 12:59:53 +0100 Subject: [PATCH 1/6] Do not disable axe rules for definition lists We wrap each name-value group in the summary list's `
` with a grouping `
` element, which is valid according to the current WHATWG HTML spec [1]. At the time we wrote these tests, this was a relatively recent change to the HTML spec, and the axe rules had not been updated to allow it. Since then, the underlying axe rules have been updated to allow wrapping `
` elements [2], so we no longer need to disable the `dlitem` and `definition-list` (released in axe-core 3.2.0 [3]) [1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element [2]: https://github.com/dequelabs/axe-core/pull/1284 [3]: https://github.com/dequelabs/axe-core/blob/develop/CHANGELOG.md#320-2019-03-04 --- src/govuk/components/summary-list/template.test.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/govuk/components/summary-list/template.test.js b/src/govuk/components/summary-list/template.test.js index 8e1b69e4ee..a22c114792 100644 --- a/src/govuk/components/summary-list/template.test.js +++ b/src/govuk/components/summary-list/template.test.js @@ -14,14 +14,7 @@ describe('Summary list', () => { it('passes accessibility tests', async () => { const $ = render('summary-list', examples.default) - const results = await axe($.html(), { - rules: { - // In newer versions of the HTML specification wrapper - //
s are allowed in a definition list - dlitem: { enabled: false }, - 'definition-list': { enabled: false } - } - }) + const results = await axe($.html()) expect(results).toHaveNoViolations() }) }) From 792e27b293d2a9144bc85ea1bca882630e6bc07e Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Wed, 8 Sep 2021 13:41:47 +0100 Subject: [PATCH 2/6] Add failing test for invalid HTML in summary list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `
` 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 to be a direct child of a `
`. 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: > >
> > Received: > > "
elements must only directly contain properly-ordered
and
groups,