Avoid invalid empty actions span in summary list

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)

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

@@ -6,15 +6,17 @@
       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;
@@ -31,7 +33,6 @@
       padding-top: govuk-spacing(2);
       padding-right: govuk-spacing(4);
       padding-bottom: govuk-spacing(2);
-      border-bottom: 1px solid $govuk-border-colour;
     }
   }
 
@@ -109,37 +110,32 @@
 
   // 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;
     }
 
-    @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;
-      }
+    .govuk-summary-list__key,
+    .govuk-summary-list__value,
+    .govuk-summary-list__actions {
+      padding-bottom: govuk-spacing(2) + 1px;
     }
   }
 
   // No border on specific rows
   .govuk-summary-list__row--no-border {
-    @include govuk-media-query($until: tablet) {
-      border: 0;
-    }
+    border: 0;
 
-    @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;
-      }
+    .govuk-summary-list__key,
+    .govuk-summary-list__value,
+    .govuk-summary-list__actions {
+      padding-bottom: govuk-spacing(2) + 1px;
     }
   }
+
+  // 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;
+  }
 }

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

@@ -217,19 +217,32 @@ 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')
-        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('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()
+        })
 
-        // Second action column is a dummy
-        expect($action[1].tagName).toEqual('span')
-        expect($($action[1]).text()).toEqual('')
+        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')
+
+        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()
+        })
       })
     })
   })