diff --git a/CHANGELOG.md b/CHANGELOG.md index 387aed0071..8ec6437dac 100644 --- a/CHANGELOG.md +++ b/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 `` elements within `
` 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 `` 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 `` within a `
` in summary list](https://github.com/alphagov/govuk-frontend/pull/2323) ## 3.14.0 (Feature release) diff --git a/src/govuk/components/summary-list/_index.scss b/src/govuk/components/summary-list/_index.scss index 3e9b60ac5d..1670a39c90 100644 --- a/src/govuk/components/summary-list/_index.scss +++ b/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; } } } diff --git a/src/govuk/components/summary-list/template.njk b/src/govuk/components/summary-list/template.njk index 8434378922..e6ad25cf2e 100644 --- a/src/govuk/components/summary-list/template.njk +++ b/src/govuk/components/summary-list/template.njk @@ -16,7 +16,7 @@
{% for row in params.rows %} {% if row %} -
+
{{ row.key.html | safe if row.key.html else row.key.text }}
@@ -37,9 +37,6 @@ {% endif %}
- {% elseif anyRowHasActions %} - {# Add dummy column to extend border #} - {% endif %} {% endif %} diff --git a/src/govuk/components/summary-list/template.test.js b/src/govuk/components/summary-list/template.test.js index 8e1b69e4ee..12a6f38c42 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() }) }) @@ -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() + }) }) }) })