change html to comply with ARIA patterns for breadcrumbs site-wide 2pts

paregorios commented 1 year ago

Remember the contractor NYU hired to tell us what to change and how to change it to make the site WCAG-compliant? Either they led us astray on the proper pattern for breadcrumbs or the ARIA guidance has changed since then. As a result, we are suddenly failing automated checks for accessibility compliance.

Current

We have patterns like this:

<div id="viewlet-above-content">
    <nav id="portal-breadcrumbs" role="breadcrumbs">
        <span id="breadcrumbs-you-are-here">You are here:</span>
        <span id="breadcrumbs-home">
            <a href="https://isaw.nyu.edu">Home</a>
            <span class="breadcrumbSeparator">&gt;</span>
        </span>
        <span id="breadcrumbs-1" dir="ltr">
            <span id="breadcrumbs-current">Graduate Studies</span>
        </span>
    </nav>
    ⋮
</div>

Desired

According to the ARIA Authoring Practices Guide (APG), we should follow this breadcrumb pattern instead. My read is that our breadcrumbs should therefore look like this:

<div id="viewlet-above-content">
    <nav id="portal-breadcrumbs" aria-label="Breadcrumb">
        <span id="breadcrumbs-you-are-here">You are here:</span>
        <span id="breadcrumbs-home">
            <a href="https://isaw.nyu.edu">Home</a>
            <span class="breadcrumbSeparator">&gt;</span>
        </span>
        <span id="breadcrumbs-1" dir="ltr">
            <a href="" id="breadcrumbs-current" aria-current="page">Graduate Studies</span>
        </span>
    </nav>
    ⋮
</div>

Summary of changes

Remove role="breadcrumbs" from the <nav id="portal-breadcrumbs"> element.
Add aria-label="Breadcrumb" to the <nav id="portal-breadcrumbs"> element. Note that "Breadcrumb" is title-capitalized and singular.
Change <span id="breadcrumbs-current"> into an <a id="breadcrumbs-current" href=""> element (including matching closing tag).
Add aria-current="page" attribute to the <a id="breadcrumbs-current" href=""> element.
Verify that visual appearance of our breadcrumbs has not changed and adjust CSS rules to remove any changes found.

How messy will this be?

The string "breadcrumbs" appears in 12 files in our code: https://github.com/search?q=repo%3Aisawnyu%2Fisaw.web%20breadcrumbs&type=code. I'm not sure which of these are actually used. If they are all used, some of them seem to conform to different, also incorrect patterns. We need one pattern throughout the whole site, compliant with "Desired" above.

alecpm commented 1 year ago

@paregorios This is up on staging for review.

One question, are you sure we need the href="" on the current-page link. It makes the link clickable unless we do something like CSS pointer-events: none? Also, FWIW the USWDS folks seems to think using a span for the current item is OK. They also recommend hiding the separator elements from screen readers (e.g. by putting aria-hidden="true" on our breadcrumbSeparator elements, which I think may be a good idea.

alecpm commented 1 year ago

@paregorios Should be updated on staging.

paregorios commented 1 year ago

@alecpm This is working as desired on staging. Please merge to master and deploy to production.

paregorios commented 1 year ago

Looks great on produciton. Thank you.

isawnyu / isaw.web