fix(theme): add missing aria-labels to the main and sidebar navs #8366

r-lawrence · 2022-11-24T04:15:01Z

Pre-flight checklist

I have read the Contributing Guidelines on pull requests.
If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.

I did not add any tests. The PR is too add an aria-label to the main/sidebar nav's. I visually verified the arial-label was populating correctly on the sidebars and also noticed tests for the translate function located at packages/docusaurus/src/client/exports/__tests__/Translate.test.tsx

Motivation

Looking to make some open source contributions to this repository. I noticed bug Issue #8308 and figured I would take a stab at it.

This PR adds an aria-label attribute to navigation bars found on https://docusaurus.io/docs allowing assistive technologies like screen readers to identify unique navigation elements.

Test Plan

verify the problem:

navigate to https://docusaurus.io/docs
open your browser developer tools and search for <nav> and verify the first and second nav elements do not contain an aria-label attribute.

verify the fix:

download this branch to your local
checkout this branch and run npm run start
once the application has loaded, navigate to http://localhost:3000/docs, and open your browser inspection tools
search the html for <nav>
verify the first nav element has an aria-label="Main"
verify the second nav element has an aria-label="Docs sidebar"

before:

main nav:

sidebar nav:

after:

main nav:

sidebar nav:

Test links

Deploy preview: https://deploy-preview-8366--docusaurus-2.netlify.app/

Related issues/PRs

Fix #8308

facebook-github-bot · 2022-11-24T04:15:07Z

Hi @r-lawrence!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

netlify · 2022-11-24T04:18:26Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`7349b76`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/6381230dd2c49f000832771e
😎 Deploy Preview	https://deploy-preview-8366--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2022-11-24T04:18:51Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟢 90	🟢 97	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 84	🟢 100	🟢 100	🟢 100	🟢 90	Report

facebook-github-bot · 2022-11-24T07:25:59Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

slorber

Thanks, that's a good start 👍

As the unit test says, you have to run yarn workspace @docusaurus/theme-translations update once ids/labels are good, so that we can merge the PRs. This updates all our default translation files and make the test pass

packages/docusaurus-theme-classic/src/theme/Navbar/Layout/index.tsx

packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/Content/index.tsx

slorber · 2022-11-24T09:34:11Z

packages/docusaurus-theme-classic/src/theme/Navbar/Layout/index.tsx

  const {navbarRef, isNavbarVisible} = useHideableNavbar(hideOnScroll);
+  const ariaLabel = translate({
+    id: 'theme.NavBar.mainAriaLabel',
+    message: 'Main',


Again @Alena5041 is this "Main" label precise enough?

I'm not an expert in screen readers but what will they announce, and will their users understand what this nav is about?

Yes, it’s enough in this case.

r-lawrence · 2022-11-25T17:19:33Z

Thanks, that's a good start 👍

As the unit test says, you have to run yarn workspace @docusaurus/theme-translations update once ids/labels are good, so that we can merge the PRs. This updates all our default translation files and make the test pass

Thanks for all the feedback and information. I'll be working today to update the PR accordingly.

update: all changes have been made, translations have been updated with yarn workspace @docusaurus/theme-translations update (tyvm for command), and PR is ready to be reviewed again. Let me know if anything else needs to be changed. Thanks a bunch!

slorber · 2022-11-30T12:56:57Z

LGTM thanks

Fix #8308

r-lawrence force-pushed the adds_arial-label_to_main/sidebar_nav branch from d9698f1 to a4acfd1 Compare November 24, 2022 04:46

r-lawrence mentioned this pull request Nov 24, 2022

Missing aria-label in nav #8308

Closed

7 tasks

facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 24, 2022

slorber requested changes Nov 24, 2022

View reviewed changes

r-lawrence force-pushed the adds_arial-label_to_main/sidebar_nav branch from a4acfd1 to 800cf93 Compare November 25, 2022 19:51

r-lawrence marked this pull request as ready for review November 25, 2022 20:02

r-lawrence requested review from Alena5041 and slorber and removed request for Alena5041 and slorber November 25, 2022 20:03

r-lawrence force-pushed the adds_arial-label_to_main/sidebar_nav branch from 800cf93 to 213181d Compare November 25, 2022 20:11

fix: adds an aria-label to the main and sidebar navs

7349b76

r-lawrence force-pushed the adds_arial-label_to_main/sidebar_nav branch from 213181d to 7349b76 Compare November 25, 2022 20:18

slorber changed the title ~~bug: adds an aria-label to the main and sidebar navs~~ fix(theme): adds aria-label to the main and sidebar navs Nov 30, 2022

slorber changed the title ~~fix(theme): adds aria-label to the main and sidebar navs~~ fix(theme): add missing aria-labels to the main and sidebar navs Nov 30, 2022

slorber added the pr: bug fix This PR fixes a bug in a past release. label Nov 30, 2022

slorber merged commit eecfdeb into facebook:main Nov 30, 2022

slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Jan 26, 2023

slorber pushed a commit that referenced this pull request Jan 26, 2023

fix(theme): add missing aria-labels to the main and sidebar navs #8366

2d5d7bd

slorber added backported This PR has been backported to a stable version of Docusaurus and removed to backport This PR is planned to be backported to a stable version of Docusaurus labels Jan 27, 2023

slorber mentioned this pull request Oct 19, 2023

chore: v3.0.0-rc.0 release #9418

Merged

Uh oh!

fix(theme): add missing aria-labels to the main and sidebar navs #8366

fix(theme): add missing aria-labels to the main and sidebar navs #8366

Uh oh!

Conversation

r-lawrence commented Nov 24, 2022 • edited by slorber Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-flight checklist

Motivation

Test Plan

verify the problem:

verify the fix:

before:

after:

Test links

Related issues/PRs

Uh oh!

facebook-github-bot commented Nov 24, 2022

Action Required

Process

Uh oh!

netlify bot commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ [V2]

Uh oh!

github-actions bot commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚡️ Lighthouse report for the deploy preview of this PR

Uh oh!

facebook-github-bot commented Nov 24, 2022

Uh oh!

slorber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slorber Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

Alena5041 Nov 24, 2022

Choose a reason for hiding this comment

Uh oh!

r-lawrence commented Nov 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber commented Nov 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

r-lawrence commented Nov 24, 2022 •

edited by slorber

Loading

netlify bot commented Nov 24, 2022 •

edited

Loading

github-actions bot commented Nov 24, 2022 •

edited

Loading

r-lawrence commented Nov 25, 2022 •

edited

Loading