alphagov / govuk-design-system

One place for service teams to find styles, components and patterns for designing government services.
https://www.gov.uk/design-system
MIT License
512 stars 232 forks source link

Options Table JavaScript sets unnecessary ARIA attributes on details element which can get out of sync #2257

Closed 36degrees closed 2 years ago

36degrees commented 2 years ago

When navigating to the Nunjucks options table for another component (for example going to https://design-system.service.gov.uk/components/fieldset/#options-fieldset-example) the Options Table JavaScript takes care of opening the relevant tab and the Details component in order to make the Nunjucks options visible:

https://github.com/alphagov/govuk-design-system/blob/7fbafde066a47c4c16ea03cf46c69d2ab72a1553/src/javascripts/components/options-table.js#L30-L44

As part of this aria-expanded is set on the summary element and aria-hidden is set on the div.govuk-details__text:

<details class="govuk-details app-options" data-module="govuk-details" id="options-fieldset-example-details" open="open">
  <summary class="govuk-details__summary" aria-expanded="true">
    <span class="govuk-details__summary-text">
      <span data-components="github-component-arguments">Nunjucks macro options</span>
    </span>
  </summary>
  <div class="govuk-details__text" aria-hidden="false">
    <!-- content here -->
  </div>
</details>

I suspect this dates from when the polyfill for the <details> element used to be applied in all browsers, and added extra ARIA attributes which then needed to be kept in sync when this script toggled the open state.

However, since GOV.UK Frontend v3.1.0 the polyfill now does nothing in browsers that natively support the <details> elements (https://github.com/alphagov/govuk-frontend/pull/1523).

This means that once these attributes are set on page load there is nothing to keep them in sync if the user toggles the details element:

36degrees commented 2 years ago

Given this has the potential to cause accessibility issues, and I think is relatively easy to fix, I'm going to add it to the sprint board.

We'll still need to add the ARIA attributes when the polyfill is used – I suspect the easiest way to do this would be to only set them if the attributes already exist on the respective elements?