Avoid invalid nesting of <span> within a <dd> in summary list

CHANGELOG.md

@@ -12,9 +12,18 @@ You must remove this setting. Otherwise, you would have to conditionally add ove
 
 This was added in [pull request 1963: Remove deprecated `$govuk-border-width-form-element-error` setting](https://github.com/alphagov/govuk-frontend/pull/1963).
 
+#### Update the HTML for summary lists
+
+We've updated the HTML for the summary list component to avoid nesting `<span>` elements within `<dd>` elements, which is invalid HTML. This update only affects summary lists that include a mix of rows with and without actions.
+
+Do not include an empty `<span class="govuk-summary-list__actions"></span>` within the rows that do not have any actions. Instead, add the `govuk-summary-list__row--no-actions` modifier class to the row.
+
+This change was introduced in [pull request #2323](https://github.com/alphagov/govuk-frontend/pull/2323).
+
 ## Fixes
 
 - [#2255: Fix conditionally revealed questions getting out of sync when multiple sets of radios and checkboxes contain inputs with the same name](https://github.com/alphagov/govuk-frontend/pull/2255)
+- [#2323: Avoid invalid nesting of `<span>` within a `<dd>` in summary list](https://github.com/alphagov/govuk-frontend/pull/2323)
 
 ## 3.14.0 (Feature release)
 

src/govuk/components/summary-list/_index.scss

@@ -6,21 +6,30 @@
  display: table;
  width: 100%;
  table-layout: fixed; // Required to allow us to wrap words that overflow.
+ border-collapse: collapse;
  }
  margin: 0; // Reset default user agent styles
  @include govuk-responsive-margin(6, "bottom");
  }
 
  .govuk-summary-list__row {
+ border-bottom: 1px solid $govuk-border-colour;
+
  @include govuk-media-query($until: tablet) {
  margin-bottom: govuk-spacing(3);
- border-bottom: 1px solid $govuk-border-colour;
  }
  @include govuk-media-query($from: tablet) {
  display: table-row;
  }
  }
 
+ // Provide an empty 'cell' for rows that don't have actions – otherwise the
+ // bottom border is not drawn for that part of the row in some browsers.
+ .govuk-summary-list__row--no-actions:after {
+ content: "";
+ display: table-cell;
+ }
+
  .govuk-summary-list__key,
  .govuk-summary-list__value,
  .govuk-summary-list__actions {
@@ -31,7 +40,6 @@
  padding-top: govuk-spacing(2);
  padding-right: govuk-spacing(4);
  padding-bottom: govuk-spacing(2);
- border-bottom: 1px solid $govuk-border-colour;
  }
  }
 
@@ -63,16 +71,6 @@
  @include govuk-media-query($until: tablet) {
  margin-bottom: govuk-spacing(3);
  }
- @include govuk-media-query($from: tablet) {
- width: 50%;
- }
- }
-
- // Expand width when value is last column (no action)
- .govuk-summary-list__value:last-child {
- @include govuk-media-query($from: tablet) {
- width: 70%;
- }
  }
 
  .govuk-summary-list__value > p {
@@ -109,36 +107,30 @@
 
  // No border on entire summary list
  .govuk-summary-list--no-border {
- @include govuk-media-query($until: tablet) {
- .govuk-summary-list__row {
- border: 0;
- }
+ .govuk-summary-list__row {
+ border: 0;
  }
 
+ // Increase padding by 1px to compensate for 'missing' border
  @include govuk-media-query($from: tablet) {
  .govuk-summary-list__key,
  .govuk-summary-list__value,
  .govuk-summary-list__actions {
- // Remove 1px border, add 1px height back on
  padding-bottom: govuk-spacing(2) + 1px;
- border: 0;
  }
  }
  }
 
  // No border on specific rows
  .govuk-summary-list__row--no-border {
- @include govuk-media-query($until: tablet) {
- border: 0;
- }
+ border: 0;
 
+ // Increase padding by 1px to compensate for 'missing' border
  @include govuk-media-query($from: tablet) {
  .govuk-summary-list__key,
  .govuk-summary-list__value,
  .govuk-summary-list__actions {
- // Remove 1px border, add 1px height back on
  padding-bottom: govuk-spacing(2) + 1px;
- border: 0;
  }
  }
  }

src/govuk/components/summary-list/template.njk

@@ -16,7 +16,7 @@
 <dl class="govuk-summary-list {%- if params.classes %} {{ params.classes }}{% endif %}"{% for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}>
  {% for row in params.rows %}
  {% if row %}
- <div class="govuk-summary-list__row {%- if row.classes %} {{ row.classes }}{% endif %}">
+ <div class="govuk-summary-list__row {%- if anyRowHasActions and not row.actions.items %} govuk-summary-list__row--no-actions{% endif %} {%- if row.classes %} {{ row.classes }}{% endif %}">
  <dt class="govuk-summary-list__key {%- if row.key.classes %} {{ row.key.classes }}{% endif %}">
  {{ row.key.html | safe if row.key.html else row.key.text }}
  </dt>
@@ -37,9 +37,6 @@
  </ul>
  {% endif %}
  </dd>
- {% elseif anyRowHasActions %}
- {# Add dummy column to extend border #}
- <span class="govuk-summary-list__actions"></span>
  {% endif %}
  </div>
  {% endif %}

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
- // <div>s are allowed in a definition list
- dlitem: { enabled: false },
- 'definition-list': { enabled: false }
- }
- })
+ const results = await axe($.html())
  expect(results).toHaveNoViolations()
  })
  })
@@ -217,19 +210,42 @@ describe('Summary list', () => {
  expect($action.length).toEqual(0)
  })
 
- it('adds dummy action columns when only some rows have actions', async () => {
+ describe('when only some rows have actions', () => {
  const $ = render('summary-list', examples['with some actions'])
+ const $component = $('.govuk-summary-list')
+
+ it('passes accessibility tests', async () => {
+ const results = await axe($.html())
+ expect(results).toHaveNoViolations()
+ })
+
+ it('does not add no-actions modifier class to rows with actions', () => {
+ // The first row has actions
+ const $firstRow = $component.find('.govuk-summary-list__row:first-child')
+ expect($firstRow.hasClass('govuk-summary-list__row--no-actions')).toBeFalsy()
+ })
 
+ it('adds no-actions modifier class to rows without actions', () => {
+ // The second row does not have actions
+ const $secondRow = $component.find('.govuk-summary-list__row:nth-child(2)')
+ expect($secondRow.hasClass('govuk-summary-list__row--no-actions')).toBeTruthy()
+ })
+ })
+
+ describe('when no rows have actions', () => {
+ const $ = render('summary-list', examples.default)
  const $component = $('.govuk-summary-list')
- const $action = $component.find('.govuk-summary-list__actions')
 
- // First action column contains link text
- expect($action[0].tagName).toEqual('dd')
- expect($($action[0]).text()).toContain('Edit')
+ it('passes accessibility tests', async () => {
+ const results = await axe($.html())
+ expect(results).toHaveNoViolations()
+ })
 
- // Second action column is a dummy
- expect($action[1].tagName).toEqual('span')
- expect($($action[1]).text()).toEqual('')
+ it('does not add no-actions modifier class to any of the rows', () => {
+ // The first row has actions
+ const $rows = $component.find('.govuk-summary-list__row')
+ expect($rows.hasClass('govuk-summary-list__row--no-actions')).toBeFalsy()
+ })
  })
  })
  })