alphagov / tech-docs-gem

Gem to distribute the tech docs project
https://tdt-documentation.london.cloudapps.digital/
MIT License
15 stars 38 forks source link

Add headings to TOC #237

Closed colinbm closed 3 years ago

colinbm commented 3 years ago

What

Add an h2 heading around TOC links where there is a sub-level ul following.

Why

The recent DAC survey raised this as an accessibility issue. This highlights that there is further content below

⚠️ Don't forget to update the gem version in the CHANGELOG before merging! When you're ready to release bump version file and generate a tag. ⚠️

colinbm commented 3 years ago

That's odd - the CSS override was working for me - will check again...

Re more details of the issue, here's the content from the report - note the suggested solution includes a h2 for "Contents" and h3s separating different uls. I've skipped the top heading (as this should already be covered by the aria-label attribute, and included the headings within the existing nested uls.


The bold page links introduce further links which are internal and navigate users to specific regions within that higher level page. The bold links are not marked-up as headings, so screen reader users will not be presented the purpose of this information as introductory.

Instead of marking content up as a heading, it appears that to associate the bold link with its child-links, each accordion has been implemented as a list with a single list item. This includes the top-level link, and then a further nested list containing the within-page links.

Current Code Ref(s): #toc > ul:nth-child(1)

<ul>
   <li class="collapsible is-open">
<a href="/#offer-govwifi" class="collapsible__heading"><span>Offer GovWifi</span></a>
<button class="collapsible__toggle" aria-controls="toc-offer-govwifi- 0"><span class="collapsible__toggle-label">Collapse Offer GovWifi</span><span class="collapsible__toggle-icon" aria-hidden="true"></span></button>
<ul class="collapsible__body" id="toc-offer-govwifi-0" aria-expanded="true"> <li>
            <a href="/#how-govwifi-works" class=""><span>How GovWifi
works</span></a>
</li> <li>
            <a href="/#who-it-s-for" class="toc-link--in-view"><span>Who it’s
for</span></a>
</li> </ul>
</li> </ul>

Solution:

Please ensure semantic mark-up is used where appropriate so that the purpose of information is clear. Screen reader users rely on correct semantic mark-up to determine relationships within the page; headings provide context to the information they introduce and indicate page regions. Screen reader users are also able to use heading shortcuts to quickly get an idea of the contents of the page and to navigate directly to the regions on the page that they require. For more information, please refer to F2: Failure of Success Criterion 1.3.1 due to using changes in text presentation to convey information without using the appropriate markup or text. Example:

<nav id="toc" class="js-toc-list toc__list" aria-labelledby="contents-heading" data-module="collapsible-navigation">
<h2 id="contents-heading">Contents</h2>
<h3><a href="/#offer-govwifi" class="collapsible__heading">
<span>Offer GovWifi</span></a></h3> [...]
<ul class="collapsible__body" id="toc-offer-govwifi-0" aria-expanded="true"> [...]
</ul> [...]
</nav>
36degrees commented 3 years ago

This change currently only affects navigation items that have 'children'. Trying this branch out with the Pay tech docs, for example, it wraps 'Customise your payment pages', 'Setting up multiple services' and 'Go live' in H2s, but no other pages.

More generally, I disagree with DAC's proposed changes here. I think this adds way too much noise to the headings rotor for users. When looking at GovWifi's tech docs with only 6 top-level headings it might just about be alright, but for example the heading structure of Pay's 'Refund a payment' page would look like this:

  h2. Contents
    h3. GOV.UK Pay technical documentation
    h3. Quick start
    h3. How GOV.UK Pay works
    h3. Take a payment
    h3. Take a digital wallet payment
    h3. Take a payment over the phone (‘MOTO’ payments)
    h3. Take a Direct Debit payment
    h3. Report on a payment
    h3. Refund a payment
    h3. Delay taking a payment
    h3. Add custom metadata
    h3. Customise your payment pages
    h3. Add corporate card fees
    h3. Block prepaid cards
    h3. Integrate with the GOV.UK Pay API
    h3. Setting up multiple services
    h3. Test your integration
    h3. Go live
    h3. Set up Strong Customer Authentication (3DS1 and 3DS2)
    h3. API reference
    h3. Stay up to date
    h3. Security
    h3. Contribute
    h3. Support
h1. Refund a payment
  h2. Check if you can refund a payment
    h3. amount_available
    h3. amount_submitted
    h3. status
  h2. Creating a refund
    h3. amount
    h3. refund_amount_available (optional)
    h3. Response
    h3. created_date
    h3. refund_id
    h3. status
  h2. Checking the status of a refund
    h3. settlement_summary
    h3. status
  h2. Where your PSP takes the refund amount from
    h3. Send email notifications about refunds
  h2. Getting a list of refunds
  h2. Get all refunds for a single payment
  h2. Searching refunds
    h3. Filtering by date
    h3. Filter refunds by when your PSP took refunds
    h3. Splitting results into pages

Over 50% of the page headings are to do with content not on this page. I've deliberately picked an example of a page with quite a few headings as well – some pages have only 2 or 3 headings so the navigation headings are then the vast majority of the page structure.

Using nested lists to represent the hierarchy of the navigation is a perfect valid thing to do, as demonstrated in the WAI example of a 'fly-out menu'.

The navigation region is also already labelled with by the #toc-heading element, and appears in Voiceover landmarks rotor as 'Table of contents navigation'. The #toc-heading is not a heading itself though, which might be something worth reviewing.

That's not to say there's not room for improvement here, but I do think we should be raising these concerns with DAC and trying to understand exactly what problem they're trying to solve with their suggested changes.

colinbm commented 3 years ago

This change currently only affects navigation items that have 'children'. Trying this branch out with the Pay tech docs, for example, it wraps 'Customise your payment pages', 'Setting up multiple services' and 'Go live' in H2s, but no other pages.

That's intentional... my interpretation was that where there was further navigation (hidden or not) then we should use a heading, but otherwise not.

Using nested lists to represent the hierarchy of the navigation is a perfect valid thing to do, as demonstrated in the WAI example of a 'fly-out menu'.

++Agree. Though there were other issues with the structure of the nav (see other PR) and fixing that may mean we can ditch this.

36degrees commented 3 years ago

That's intentional... my interpretation was that where there was further navigation (hidden or not) then we should use a heading, but otherwise not.

Sorry, I think that was more than a little ambiguous on my part. Maybe a better way to phrase it is that this only affects pages that have 'nested pages'.

In the example docs, the only page that gets wrapped in an h2 is the 'nested page'. Other pages that only have the headings on the page as child navigation (like 'Hello, world!') do not.

jonathanglassman commented 3 years ago

@36degrees @colinbm would it be helpful for me to try to arrange a chat with DAC to discuss this issue?

colinbm commented 3 years ago

@jonathanglassman @36degrees I'm emailing with DAC on this - will let you know how I get on. I think part of the problem is the ul structure, which I've fixed separately on another PR... I'm hoping with that in place we can discard this.

colinbm commented 3 years ago

Approval from DAC for the alternate method, with some additional flair for accessibility... This PR is no longer relevant so closing.