wvuweb / cleanslate-cms

A place to file issues and view releases for CleanSlate CMS. http://cleanslatecms.wvu.edu
6 stars 0 forks source link

Add `aria-current="page"` to active page in all menu tags #259

Closed adamjohnson closed 4 years ago

adamjohnson commented 4 years ago

Steps to reproduce the issue

  1. View any site that has an auto-generated navigation using r:site_menu, r:sub_menu, r:ancestor_menu, r:breadcrumbs or anywhere the active class is applied to the current page's <a> tag.
    • Code is a good example of this.

Results

The current output looks like this:

<ul class="cs-generatedNav">
  <li><a class="active" href="https://code.wvu.edu/home">Home</a></li>
  <li><a href="https://code.wvu.edu/test">test</a></li>
</ul>

Note how the first <a> tag has class="active" denoting the current page.

Expected results / feature request.

Add the aria-current="page" attribute to the anchor tag:

<ul class="cs-generatedNav">
  <li><a class="active" aria-current="page" href="https://code.wvu.edu/home">Home</a></li>
  <li><a href="https://code.wvu.edu/test">test</a></li>
</ul>

aria-current="page" helps screen reader users by announcing "Current Page" when they're tabbing through links. Here's a great article explaining aria-current.

nreckart commented 4 years ago

@adamjohnson The active/current page in a breadcrumb list does not have an <a>, it's just an <li>. Should aria-current="page' be applied to the <li>?

adamjohnson commented 4 years ago

Looking closely at the spec:

A page token used to indicate a link within a set of pagination links, where the link is visually styled to represent the currently-displayed page.

So that would lead me to believe we should omit aria-current from the breadcrumbs because the final "crumb" (aka the current page) is not a link.

nreckart commented 4 years ago

Changes for auto generated nav menus have been pushed to production. May not see the affect immediately for current sites due to caching, which is a little tricky here. It's not a 10 min cache. It basically caches menus until the site page structure is changed. So you may need to add/remove a page to bust it.

adamjohnson commented 4 years ago

Seems to be working. Thanks!

adamjohnson commented 4 years ago

Just found an issue with this today: External Link and Internal Link pages are adding a W3C invalid attribute iscurrent="false":

<a class="cs-external-link custom-class-applied-via-properties-modal" iscurrent="false" title="" target="_self" href="https:///test#yo-dog">Test External Link</a>

You can see this at the following URLs:

nreckart commented 4 years ago

Changes deployed. See my note above in a previous comment about caching.

You may be able to update/save site settings without actually changing anything to bust the menus cache, but this technique did not work in dev for me.

adamjohnson commented 4 years ago

Looks like these changes fixed the iscurrent stuff. Thanks!