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

Axe error: <dl> elements must only directly contain properly-ordered <dt> and <dd> groups, <script> or <template> elements #1806

Closed
Tracked by #2274
malcolmhire opened this issue May 11, 2020 · 5 comments · Fixed by #2323
Assignees
Milestone

Comments

@malcolmhire
Copy link

malcolmhire commented May 11, 2020

As the title explains, when using the summary list component and using the action element within the Nunjucks component only on some of the summary rows we receive axe-core violations.

The example below shows the bottom row missing an action.

rows: [
  {
    key: { text: 'First name' },
    value: { text: 'Joe' },
    action: { 
      items: [
         {
            href: '/first-name',
            text: 'Change'
         }
      ]
    }
  }, {
    key: { text: 'Last name' },
    value: { text: 'Bloggs' },
    action: { 
      items: [
         {
            href: '/last-name',
            text: 'Change'
         }
      ]
    }
  }, {
    key: { text: 'Date of Birth' },
    value: { text: '06/01/1953' }
  }
];

We receive this axe-core error from the genarate HTML that relates to the bottom row that has no action:

<dl> elements must only directly contain properly-ordered <dt> and <dd> groups, <script> or <template> elements

.govuk-summary-list__row:nth-child(3) > .govuk-summary-list__actions

It looks like the problem lies in the empty <span class="govuk-summary-list__actions"> instead of <dd class="govuk-summary-list__actions">

@matthewmascord
Copy link
Contributor

This has been raised by one of our service teams at HMRC, hmrc/play-frontend-govuk#60 - it's causing automated accessibility tests to fail.

@36degrees 36degrees added the awaiting triage Needs triaging by team label Jul 13, 2020
@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label Jul 13, 2020
@adamliptrot-oc
Copy link

This could possibly be fixed by using a pseudo-element on the parent row to stand-in for the missing dd and so maintain the rest of the layout (as just removing this will cause issues where only one of the items has no action element).

For example, adding a modifier class to the parent row:

.govuk-summary-list__row--noaction:after {
  content: "";
  width: 20%;
  display: table-cell;
  border-bottom: 1px solid #b1b4b6;
}

It becomes a bit more tricky where width modifier classes might also be sent down to the component however.

Screenshot 2020-08-03 at 13 22 40

@philsherry
Copy link

This could also be fixed by using more appropriate markup.

Are we saying that a link with the word “Change” is an acceptable value for a dd here, or is it merely convenient to add it into this handy element?

The HTML <dd> element provides the description, definition, or value for the preceding term (<dt>) in a description list (<dl>).

Term Description Description
Business name Claim7Business (empty)
Registration reference number SOME/REF-001 (empty)
Claim period 25 July to 28 July 2020 [A Change link]

Assuming we’re fine with that, then we also have to be fine with this error:

<dl> elements must only directly contain properly-ordered <dt> and <dd> groups, <script> or <template> elements

So, if I’m reading this right, any teams using this pattern will need to address these issues in their accessibility statement because they’ll be failing on the following two points:

@36degrees 36degrees added the awaiting triage Needs triaging by team label Aug 26, 2020
@trang-erskine trang-erskine removed the awaiting triage Needs triaging by team label Sep 7, 2020
@malcolmhire
Copy link
Author

Do we know when this might be worked on? I'm probably going to have a go fixing this myself, initially as a custom macro in our project but can share my finding once I have a solution

@hannalaakso hannalaakso added the awaiting triage Needs triaging by team label Sep 21, 2020
@kellylee-gds kellylee-gds added ⚠️ high priority Used by the team when triaging and removed awaiting triage Needs triaging by team labels Sep 21, 2020
@hannalaakso
Copy link
Member

@malcolmhire We've now given this a high priority label so we hope to be able to pick it up soon. However it's not possible to give a time frame for this at the moment. Thanks a lot for your patience 🙏

@36degrees 36degrees changed the title Summary list axe-core violations when action element used on some but not all rows Invalid nesting of elements in summary list when actions are included on some but not all rows (WCAG 2.1 4.1.1 Parsing) Sep 28, 2020
@36degrees 36degrees added this to the v4.0.0 milestone Mar 17, 2021
36degrees added a commit that referenced this issue Aug 18, 2021
When a summary list has actions in only some of the rows, we currently 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 was previously 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 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.' [1]

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.

Avoid the need for the empty 'cell' by moving the bottom border to the row itself. This requires us to set `border-collapse: collapse` in order to use the collapsing border model [2] as in the separated borders model (the initial border model) – 'rows, columns, row groups, and column groups cannot have borders' [3]

Remove the empty `span.govuk-summary-list__actions` from the template – although it does still work perfectly well with the existing markup, so I don't believe this is technically a breaking change.

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
[2]: https://drafts.csswg.org/css2/#collapsing-borders
[3]: https://drafts.csswg.org/css2/#:~:text=Rows%2C%20columns%2C%20row%20groups%2C%20and%20column%20groups%20cannot%20have%20borders%20(i.e.%2C%20user%20agents%20must%20ignore%20the%20border%20properties%20for%20those%20elements).
36degrees added a commit that referenced this issue Aug 18, 2021
When a summary list has actions in only some of the rows, we currently 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 was previously 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.

Avoid the need for the empty 'cell' by moving the bottom border to the row itself. This requires us to set `border-collapse: collapse` in order to use the collapsing border model [2] as in the separated borders model (the initial border model) – 'rows, columns, row groups, and column groups cannot have borders' [3]

Remove the empty `span.govuk-summary-list__actions` from the template – although it does still work perfectly well with the existing markup, so I don't believe this is technically a breaking change.

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
[2]: https://drafts.csswg.org/css2/#collapsing-borders
[3]: https://drafts.csswg.org/css2/#:~:text=Rows%2C%20columns%2C%20row%20groups%2C%20and%20column%20groups%20cannot%20have%20borders%20(i.e.%2C%20user%20agents%20must%20ignore%20the%20border%20properties%20for%20those%20elements).
@36degrees 36degrees self-assigned this Aug 18, 2021
36degrees added a commit that referenced this issue Aug 20, 2021
When a summary list has actions in only some of the rows, we currently 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.

Simplify the code by moving the bottom border to the row itself. This requires us to set `border-collapse: collapse` in order to use the collapsing border model [2] as in the separated borders model (the initial border model) – 'rows, columns, row groups, and column groups cannot have borders' [3]

Unfortunately some browsers, including IE11 and Safari, still omit the border where a row is missing cells – so we still need to provide an empty cell for rows without actions.

Replace the empty `span.govuk-summary-list__actions` with an `:after` pseudo-element as suggested by @adamliptrot-oc [4]. This requires a new modifier class `govuk-summary-list__row--no-actions` to be applied to any rows that do not have actions (unless none of the rows in the summary list have actions)

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
[2]: https://drafts.csswg.org/css2/#collapsing-borders
[3]: https://drafts.csswg.org/css2/#:~:text=Rows%2C%20columns%2C%20row%20groups%2C%20and%20column%20groups%20cannot%20have%20borders%20(i.e.%2C%20user%20agents%20must%20ignore%20the%20border%20properties%20for%20those%20elements).
[4]: #1806 (comment)
36degrees added a commit that referenced this issue Aug 20, 2021
When a summary list has actions in only some of the rows, we currently 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.

Simplify the code by moving the bottom border to the row itself. This requires us to set `border-collapse: collapse` in order to use the collapsing border model [2] as in the separated borders model (the initial border model) – 'rows, columns, row groups, and column groups cannot have borders' [3]

Unfortunately some browsers, including IE11 and Safari, still omit the border where a row is missing cells – so we still need to provide an empty cell for rows without actions.

Replace the empty `span.govuk-summary-list__actions` with an `:after` pseudo-element as suggested by @adamliptrot-oc [4]. This requires a new modifier class `govuk-summary-list__row--no-actions` to be applied to any rows that do not have actions (unless none of the rows in the summary list have actions)

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
[2]: https://drafts.csswg.org/css2/#collapsing-borders
[3]: https://drafts.csswg.org/css2/#:~:text=Rows%2C%20columns%2C%20row%20groups%2C%20and%20column%20groups%20cannot%20have%20borders%20(i.e.%2C%20user%20agents%20must%20ignore%20the%20border%20properties%20for%20those%20elements).
[4]: #1806 (comment)
36degrees added a commit that referenced this issue Aug 20, 2021
When a summary list has actions in only some of the rows, we currently 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.

Simplify the code by moving the bottom border to the row itself. This requires us to set `border-collapse: collapse` in order to use the collapsing border model [2] as in the separated borders model (the initial border model) – 'rows, columns, row groups, and column groups cannot have borders' [3]

Unfortunately some browsers, including IE11 and Safari, still omit the border where a row is missing cells – so we still need to provide an empty cell for rows without actions.

Replace the empty `span.govuk-summary-list__actions` with an `:after` pseudo-element as suggested by @adamliptrot-oc [4]. This requires a new modifier class `govuk-summary-list__row--no-actions` to be applied to any rows that do not have actions (unless none of the rows in the summary list have actions)

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
[2]: https://drafts.csswg.org/css2/#collapsing-borders
[3]: https://drafts.csswg.org/css2/#:~:text=Rows%2C%20columns%2C%20row%20groups%2C%20and%20column%20groups%20cannot%20have%20borders%20(i.e.%2C%20user%20agents%20must%20ignore%20the%20border%20properties%20for%20those%20elements).
[4]: #1806 (comment)
36degrees added a commit that referenced this issue Aug 20, 2021
When a summary list has actions in only some of the rows, we currently 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.

Simplify the code by moving the bottom border to the row itself. This requires us to set `border-collapse: collapse` in order to use the collapsing border model [2] as in the separated borders model (the initial border model) – 'rows, columns, row groups, and column groups cannot have borders' [3]

Unfortunately some browsers, including IE11 and Safari, still omit the border where a row is missing cells – so we still need to provide an empty cell for rows without actions.

Replace the empty `span.govuk-summary-list__actions` with an `:after` pseudo-element as suggested by @adamliptrot-oc [4]. This requires a new modifier class `govuk-summary-list__row--no-actions` to be applied to any rows that do not have actions (unless none of the rows in the summary list have actions)

[1]: https://html.spec.whatwg.org/multipage/grouping-content.html#the-dl-element
[2]: https://drafts.csswg.org/css2/#collapsing-borders
[3]: https://drafts.csswg.org/css2/#:~:text=Rows%2C%20columns%2C%20row%20groups%2C%20and%20column%20groups%20cannot%20have%20borders%20(i.e.%2C%20user%20agents%20must%20ignore%20the%20border%20properties%20for%20those%20elements).
[4]: #1806 (comment)
36degrees added a commit that referenced this issue Sep 8, 2021
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
36degrees added a commit that referenced this issue Sep 8, 2021
As described in the previous commit, when a summary list has actions in only some of the rows, we currently include an empty `span` with the `govuk-summary-list__actions` class in the rows that do not have actions. However, it's not valid for a `<span>` to be a direct child of a `<dl>`.

Replace the empty `span.govuk-summary-list__actions` with an `:after` pseudo-element as suggested by @adamliptrot-oc [1]. This requires a new modifier class `govuk-summary-list__row--no-actions` to be applied to any rows that do not have actions (unless none of the rows in the summary list have actions)

[1]: #1806 (comment)
36degrees added a commit that referenced this issue Oct 14, 2021
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
36degrees added a commit that referenced this issue Oct 14, 2021
As described in the previous commit, when a summary list has actions in only some of the rows, we currently include an empty `span` with the `govuk-summary-list__actions` class in the rows that do not have actions. However, it's not valid for a `<span>` to be a direct child of a `<dl>`.

Replace the empty `span.govuk-summary-list__actions` with an `:after` pseudo-element as suggested by @adamliptrot-oc [1]. This requires a new modifier class `govuk-summary-list__row--no-actions` to be applied to any rows that do not have actions (unless none of the rows in the summary list have actions)

[1]: #1806 (comment)
@36degrees 36degrees changed the title Invalid nesting of elements in summary list when actions are included on some but not all rows (WCAG 2.1 4.1.1 Parsing) Axe error: <dl> elements must only directly contain properly-ordered <dt> and <dd> groups, <script> or <template> elements Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment