hmrc / play-frontend-govuk

Apache License 2.0
5 stars 7 forks source link

Possible Bug with SummaryList when rendering 'None' actions #60

Open SteveSugden opened 4 years ago

SteveSugden commented 4 years ago

We had a SummaryList with two rows. The first with no actions, the second with a change link.

@govukSummaryList(SummaryList(
            rows = Seq(
                SummaryListRow(
                    key = Key(
                        content = Text(messages("associate.ucr.summary.consignmentReference"))
                    ),
                    value = Value(
                        content = Text(consignmentRef)
                    ),
                    actions = None
                ),
                SummaryListRow(
                    key = Key(
                        content = Text(messages(s"associate.ucr.summary.associate.with.${associateKind.formValue}"))
                    ),
                    value = Value(
                        content = Text(associateWith)
                    ),
                    actions = Some(Actions(
                        items = Seq(
                            ActionItem(
                                href = changeUrl,
                                content = HtmlContent(linkContent(messages("site.change")))
                            )
                        )
                    ))
                )
            ),
            classes = "govuk-!-margin-bottom-9"
        ))

This rendered the following html

<dl class="govuk-summary-list govuk-!-margin-bottom-9">
    <div class="govuk-summary-list__row">
      <dt class="govuk-summary-list__key">
        Consignment reference
      </dt>
      <dd class="govuk-summary-list__value">
        8GB123457359100-TEST0002
      </dd>

          <span class="govuk-summary-list__actions"></span>

    </div>

    <div class="govuk-summary-list__row">
      <dt class="govuk-summary-list__key">
        Associate with MUCR
      </dt>
      <dd class="govuk-summary-list__value">
        GB/1234-123ABC456DEFIIIII
      </dd>

        <dd class="govuk-summary-list__actions">

  <a class="govuk-link" href="/customs-movements/mucr-options"> 
<span aria-hidden="true">
  Change
</span><span class="govuk-visually-hidden"> Change MUCR</span> </a>

        </dd>

    </div>

</dl>

According to the Axe accessibility tool, (and advice on the #community-frontend channel) the empty for the first row is not a valid child for a

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

The appears to be generated here as a 'dummy column' - https://github.com/hmrc/play-frontend-govuk/blob/f2eaff9c251bb13249fb8b7601ef726e5014bba0/src/main/play-26/twirl/uk/gov/hmrc/govukfrontend/views/components/govukSummaryList.scala.html#L49

If the SummaryList component were to render and empty

element instead then I believe this Axe warning would be solved.

matthewmascord commented 4 years ago

This is an issue with govuk-frontend: https://github.com/alphagov/govuk-frontend/issues/1806