Sitelet https://web.archive.org/web/20200814041020/https://github.com/nodejs/nodejs.dev/pull/835
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add aria-labels to nav elements for accessibility #835

Merged
merged 5 commits into from Aug 13, 2020

Conversation

@jendowns
Copy link
Contributor

jendowns commented Aug 12, 2020

Description

Both the Docs and Learn pages have two <nav> landmarks. It would be helpful for assistive tech to distinguish between these two navigation areas.

Since neither landmark appears to have a unique heading to point to (i.e., something like "Site navigation" versus "Learn"/"Documentation") I think going with aria-label attributes as described in WCAG 2.1 aria-label guidelines would work. So in this PR, I have added aria-label attributes to the <nav> landmarks, indicating that they are either the "Primary" nav (main site nav) or "Secondary" nav (inner page nav, like for "Learn" or "Docs" content specifically).

Here's an example from the WCAG 2.1
docs following a similar method:

Example 1: Distinguishing navigation landmarks
The following example shows how aria-label could be used to distinguish two navigation landmarks in a HTML4 and XHTML 1.0 document, where there are more than two of the same type of landmark on the same page, and there is no existing text on the page that can be referenced as the label.

<div role="navigation" aria-label="Primary">
<ul><li>...a list of links here ...</li></ul> </div>
<div role="navigation" aria-label="Secondary">
<ul><li>...a list of links here ...</li> </ul></div>

I'm totally open to switching up this label text -- just let me know!

Or if you prefer to follow the WCAG 2.1 aria-labelledby guidelines & point to some existing text/heading node on the page, that would also work -- just let me know where/what those headings are (I just very new to this repo & didn't see any existing nodes that fit the criteria 😅).

Thank you! 💖

jendowns added 3 commits Aug 12, 2020
Copy link
Contributor

benhalverson left a comment

👍

@MylesBorins
Copy link
Member

MylesBorins commented Aug 12, 2020

/preview

@jendowns
Copy link
Contributor Author

jendowns commented Aug 13, 2020

/preview

@github-actions
Copy link

github-actions bot commented Aug 13, 2020

Please find a preview at: https://staging.nodejs.dev/835/

@benhalverson benhalverson merged commit daa9d2b into nodejs:master Aug 13, 2020
2 checks passed
2 checks passed
test-ci
Details
test-ci
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.