Repository: nodejs/nodejs.dev
Pull request #2864 (2022-10-03 18:25:32)

Fix: secondary nav accessibility
<!--
Please read the [Code of Conduct](https://github.com/nodejs/nodejs.dev/blob/main/CODE_OF_CONDUCT.md) and the [Contributing Guidelines](https://github.com/nodejs/nodejs.dev/blob/main/CONTRIBUTING.md) before opening a pull request.
-->

## Description

The sidebar navigation was not behaving correctly for keyboard users or screen reader users. Therefore I made some accessibility updates.

Made divs into buttons so they are semantically correct and the tab order works correctly (before tabbing on the main headings would automatically open the whole list and this is not expected behaviour) Now the buttons only open when enter is pressed.

Changed the button styling so the styles remain as before.

Added and removed aria where needed

Added focus indicator to hamburger menu

## Related Issues

<!--
  Link to the issue that is fixed by this PR (if there is one)
  e.g. Fixes #1234, Addresses #1234, Related to #1234, etc.
-->

### Check List

<!--
ATTENTION
Please follow this check list to ensure that you've followed all items before opening this PR
-->

- [x] I have read the [Contributing Guidelines](https://github.com/nodejs/nodejs.dev/blob/main/CONTRIBUTING.md) and made commit messages that follow the guideline.
- [x] I have run `npm run lint:js -- --fix` and/or `npm run lint:md -- --fix` for my JavaScript and/or Markdown changes.
  - This is important as most of the cases your code changes might not be correctly linted
- [x] I have run `npm run test` to check if all tests are passing, and/or `npm run test -- -u` to update snapshots if I created and/or updated React Components.
- [ ] I have checked that the build works locally and that `npm run build` work fine.
- [ ] I've covered new added functionality with unit tests if necessary.


github-actions[bot]: Please find a preview at: https://staging.nodejs.dev/2864/
codecov-commenter: # [Codecov](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
Base: **66.02**% // Head: **63.77**% // Decreases project coverage by **`-2.25%`** :warning:
> Coverage data is based on head [(`efcf57d`)](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) compared to base [(`fd57b87`)](https://codecov.io/gh/nodejs/nodejs.dev/commit/fd57b871c2b1ad2f6fb812ed066dec074d76db22?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> Patch has no changes to coverable lines.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2864      +/-   ##
==========================================
- Coverage   66.02%   63.77%   -2.26%     
==========================================
  Files         118      124       +6     
  Lines        1351     1463     +112     
  Branches      342      365      +23     
==========================================
+ Hits          892      933      +41     
- Misses        422      490      +68     
- Partials       37       40       +3     
```


| [Impacted Files](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [src/\_\_fixtures\_\_/hooks.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL19fZml4dHVyZXNfXy9ob29rcy50c3g=) | `100.00% <ø> (ø)` | |
| [src/\_\_fixtures\_\_/page.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL19fZml4dHVyZXNfXy9wYWdlLnRzeA==) | `94.11% <ø> (-5.89%)` | :arrow_down: |
| [...components/ApiComponents/Components/ApiHeading.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQXBpQ29tcG9uZW50cy9Db21wb25lbnRzL0FwaUhlYWRpbmcudHN4) | `0.00% <ø> (ø)` | |
| [...rc/components/ApiComponents/Components/ApiLink.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQXBpQ29tcG9uZW50cy9Db21wb25lbnRzL0FwaUxpbmsudHN4) | `0.00% <ø> (ø)` | |
| [...ents/ApiComponents/Components/SourceLink/index.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQXBpQ29tcG9uZW50cy9Db21wb25lbnRzL1NvdXJjZUxpbmsvaW5kZXgudHN4) | `0.00% <ø> (ø)` | |
| [...ApiComponents/Components/VersionSelector/index.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQXBpQ29tcG9uZW50cy9Db21wb25lbnRzL1ZlcnNpb25TZWxlY3Rvci9pbmRleC50c3g=) | `0.00% <ø> (ø)` | |
| [src/components/ApiComponents/index.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQXBpQ29tcG9uZW50cy9pbmRleC50c3g=) | `0.00% <ø> (ø)` | |
| [src/components/Article/index.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQXJ0aWNsZS9pbmRleC50c3g=) | `100.00% <ø> (ø)` | |
| [src/components/Banner/index.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQmFubmVyL2luZGV4LnRzeA==) | `100.00% <ø> (ø)` | |
| [src/components/Codebox/index.tsx](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbXBvbmVudHMvQ29kZWJveC9pbmRleC50c3g=) | `100.00% <ø> (ø)` | |
| ... and [78 more](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/nodejs/nodejs.dev/pull/2864?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).

github-actions[bot]: Please find a preview at: https://staging.nodejs.dev/2864/

Patch:
diff --git a/src/components/NavigationSection/index.module.scss b/src/components/NavigationSection/index.module.scss
index 05c7b207..16fe2a2c 100644
--- a/src/components/NavigationSection/index.module.scss
+++ b/src/components/NavigationSection/index.module.scss
@@ -24,12 +24,14 @@
 
   &__pure-title,
   &%__pure-title {
+    border: none;
     color: var(--color-text-primary);
     display: flex;
     flex-direction: row;
     font-weight: var(--font-weight-semibold);
     justify-content: space-between;
     margin: var(--space-24) 0 var(--space-16) var(--space-24);
+    width: 100%;
 
     span {
       align-items: center;
diff --git a/src/components/NavigationSection/index.tsx b/src/components/NavigationSection/index.tsx
index b55c052f..5b9e3bc3 100644
--- a/src/components/NavigationSection/index.tsx
+++ b/src/components/NavigationSection/index.tsx
@@ -3,7 +3,6 @@ import classnames from 'classnames';
 import ArrowDropDownIcon from '@mui/icons-material/ArrowDropDown';
 import ArrowDropUpIcon from '@mui/icons-material/ArrowDropUp';
 import OfflineBoltIcon from '@mui/icons-material/OfflineBolt';
-import { useMediaQuery } from '../../hooks/useMediaQuery';
 import { NavigationSectionItem } from '../../types';
 import NavigationItem from '../NavigationItem';
 import styles from './index.module.scss';
@@ -28,7 +27,6 @@ const NavigationSection = ({
 }: Props): JSX.Element => {
   const isActive = (item: NavigationSectionItem) => item.slug === currentSlug;
   const [isOpen, setIsOpen] = useState(!!section.find(isActive));
-  const isMobile = useMediaQuery('(max-width: 870px)');
   const toggle = (): void => setIsOpen(!isOpen);
 
   const titleClassNames = classnames('t-body2', styles.navigationSectionTitle, {
@@ -39,25 +37,23 @@ const NavigationSection = ({
 
   return (
     <div className={navigationSectionClasses}>
-      <div
+      <button
+        type="button"
         className={titleClassNames}
         onClick={toggle}
-        onKeyDown={toggle}
-        tabIndex={0}
-        role="menuitem"
+        aria-expanded={isOpen}
       >
         <span>
           {isApiDocs && <OfflineBoltIcon />}
           {title}
         </span>
-        {!isMobile &&
-          (isOpen ? (
-            <ArrowDropDownIcon style={{ padding: 0 }} />
-          ) : (
-            <ArrowDropUpIcon style={{ padding: 0 }} />
-          ))}
-      </div>
-      <div style={{ display: isOpen || isMobile ? 'block' : 'none' }}>
+        {isOpen ? (
+          <ArrowDropDownIcon style={{ padding: 0 }} />
+        ) : (
+          <ArrowDropUpIcon style={{ padding: 0 }} />
+        )}
+      </button>
+      <div role="region" style={{ display: isOpen ? 'block' : 'none' }}>
         {section.map((item: NavigationSectionItem): JSX.Element => {
           const isRead: boolean = readSections.has(item.slug);
 
diff --git a/src/containers/Navigation/index.module.scss b/src/containers/Navigation/index.module.scss
index ba25e999..01ec878d 100644
--- a/src/containers/Navigation/index.module.scss
+++ b/src/containers/Navigation/index.module.scss
@@ -33,7 +33,6 @@
     height: calc(24px + 3px);
     line-height: 0;
     margin-left: -0.8rem;
-    outline: none;
     overflow: visible;
     padding: 12px 0;
     position: sticky;
diff --git a/src/containers/Navigation/index.tsx b/src/containers/Navigation/index.tsx
index 45b3282f..29de1955 100644
--- a/src/containers/Navigation/index.tsx
+++ b/src/containers/Navigation/index.tsx
@@ -53,7 +53,12 @@ const Navigation = ({
 
   return (
     <nav aria-label={label} className={navigationClasses}>
-      <button type="button" className={styles.navigationOpen} onClick={toggle}>
+      <button
+        type="button"
+        className={styles.navigationOpen}
+        onClick={toggle}
+        aria-expanded={isOpen}
+      >
         <FormattedMessage id="containers.navigation.title" />
       </button>
       {children}


Test patch:
diff --git a/src/components/NavigationSection/__tests__/__snapshots__/index.test.tsx.snap b/src/components/NavigationSection/__tests__/__snapshots__/index.test.tsx.snap
index ab0ff232..60ad109f 100644
--- a/src/components/NavigationSection/__tests__/__snapshots__/index.test.tsx.snap
+++ b/src/components/NavigationSection/__tests__/__snapshots__/index.test.tsx.snap
@@ -5,10 +5,10 @@ exports[`NavigationSection component renders correctly 1`] = `
   <div
     class="navigationSection"
   >
-    <div
+    <button
+      aria-expanded="false"
       class="t-body2 navigationSectionTitle"
-      role="menuitem"
-      tabindex="0"
+      type="button"
     >
       <span>
         Introduction
@@ -25,8 +25,9 @@ exports[`NavigationSection component renders correctly 1`] = `
           d="m7 14 5-5 5 5z"
         />
       </svg>
-    </div>
+    </button>
     <div
+      role="region"
       style="display: none;"
     >
       <a
diff --git a/src/containers/Navigation/__tests__/__snapshots__/navigation.test.tsx.snap b/src/containers/Navigation/__tests__/__snapshots__/navigation.test.tsx.snap
index c48077c9..e2fc11f1 100644
--- a/src/containers/Navigation/__tests__/__snapshots__/navigation.test.tsx.snap
+++ b/src/containers/Navigation/__tests__/__snapshots__/navigation.test.tsx.snap
@@ -7,6 +7,7 @@ exports[`Navigation component renders correctly 1`] = `
     class="navigation"
   >
     <button
+      aria-expanded="false"
       class="navigationOpen"
       type="button"
     >
@@ -15,10 +16,10 @@ exports[`Navigation component renders correctly 1`] = `
     <div
       class="navigationSection"
     >
-      <div
+      <button
+        aria-expanded="true"
         class="t-body2 navigationSectionTitle"
-        role="menuitem"
-        tabindex="0"
+        type="button"
       >
         <span>
           Getting Started
@@ -35,8 +36,9 @@ exports[`Navigation component renders correctly 1`] = `
             d="m7 10 5 5 5-5z"
           />
         </svg>
-      </div>
+      </button>
       <div
+        role="region"
         style="display: block;"
       >
         <a
diff --git a/src/templates/__tests__/__snapshots__/learn.test.tsx.snap b/src/templates/__tests__/__snapshots__/learn.test.tsx.snap
index 31359b8b..537acb55 100644
--- a/src/templates/__tests__/__snapshots__/learn.test.tsx.snap
+++ b/src/templates/__tests__/__snapshots__/learn.test.tsx.snap
@@ -197,6 +197,7 @@ exports[`Learn Template renders correctly 1`] = `
         class="navigation"
       >
         <button
+          aria-expanded="false"
           class="navigationOpen"
           type="button"
         >