Skip to content

a11y - separate nav expander from link #7868

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

a11y - separate nav expander from link #7868

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ef98507
separate nav expander from link
Jul 31, 2024
43de576
adding aria props
Jul 31, 2024
2e7d363
toggle/expand nav links
Aug 8, 2024
05b5ea7
fix currentstyling
Aug 9, 2024
bbba01e
Merge branch 'main' into a11y-toggle_expand-nav-links
Sep 26, 2024
6b41d06
Merge branch 'main' into a11y-toggle_expand-nav-links
Sep 26, 2024
0235a96
trying to fix be havoir
Sep 26, 2024
f4b3336
change attribute logic is basd on
Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 78 additions & 16 deletions src/components/Menu/MenuItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { usePathWithoutHash } from '@/utils/usePathWithoutHash';
import { ReactElement, useContext, useEffect, useState, useMemo } from 'react';
import { Link as AmplifyUILink, Flex } from '@aws-amplify/ui-react';
import {
ReactElement,
useContext,
useEffect,
useState,
useMemo,
useRef
} from 'react';
import { Link as AmplifyUILink, Button, Flex } from '@aws-amplify/ui-react';
import { IconExternalLink, IconChevron } from '@/components/Icons';
import Link from 'next/link';
import { JS_PLATFORMS, Platform, JSPlatform } from '@/data/platforms';
Expand All @@ -19,6 +26,7 @@ type MenuItemProps = {
level: number;
currentPlatform?: Platform;
hideChildren?: boolean;
tabIndex?: number;
};

function getPathname(route, currentPlatform: Platform | undefined) {
Expand All @@ -43,11 +51,38 @@ export function MenuItem({
const { menuOpen, toggleMenuOpen } = useContext(LayoutContext);
const asPathWithoutHash = usePathWithoutHash();
const [open, setOpen] = useState(false);
const ref = useRef(null);
const children = useMemo(
() => (hideChildren ? [] : pageNode.children),
[hideChildren, pageNode.children]
);

const setSelectability = () => {
const current = ref.current;
const items = current?.parentElement?.nextSibling?.children;
if (items) {
for (const item of items) {
const links = item.getElementsByTagName('a');
const buttons = item.getElementsByTagName('button');
if (links[0].getAttribute('tabIndex') == 0) {
for (const link of links) {
link.setAttribute('tabIndex', -1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the desired tab behavior only works when opening the link after landing on a page, reloading while already on a page leaves these visible menu links untabbable

Screen.Recording.2024-08-15.at.9.18.18.AM.mov

}
for (const button of buttons) {
button.setAttribute('tabIndex', -1);
}
} else if (-links[0].getAttribute('tabIndex')) {
links[0]?.setAttribute('tabIndex', 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the tabIndex can get out of sync with the display by clicking on the button and the link. I wonder if there is another attribute that could be used to determine if this should be tabbable. Something like tracking the open state in the parent and using that isOpen state to determine the tabIndex of the menu items.

Screen.Recording.2024-08-15.at.9.29.31.AM.mov

buttons[0]?.setAttribute('tabIndex', 0);
}
}
}
current?.focus();
};

const onLinkClick = () => {
setSelectability();

// Category shouldn't be collapsible
if (
level > Levels.Category &&
Expand All @@ -62,6 +97,18 @@ export function MenuItem({
}
};

const onCheveronClick = () => {
setSelectability();
setOpen((prevOpen) => !prevOpen);

if (menuOpen) {
// Close the menu after clicking a link (applies to the mobile menu)
toggleMenuOpen(false);
} else {
toggleMenuOpen(true);
}
};

const handleFocus = () => {
if (parentSetOpen) {
parentSetOpen(true);
Expand Down Expand Up @@ -157,6 +204,7 @@ export function MenuItem({
href={pageNode.route}
isExternal={true}
onClick={onLinkClick}
tabIndex={level > Levels.Subcategory ? -1 : 0}
>
<Flex
className={`menu__list-item__link__inner ${listItemLinkInnerStyle}`}
Expand Down Expand Up @@ -188,22 +236,35 @@ export function MenuItem({
key={pageNode.route}
className={`menu__list-item ${listItemStyle}`}
>
<Link
className={`menu__list-item__link ${listItemLinkStyle} ${currentStyle}`}
aria-current={current ? 'page' : null}
href={href}
onClick={onLinkClick}
passHref
>
<Flex
className={`menu__list-item__link__inner ${listItemLinkInnerStyle}`}
<Flex className={`menu__list-item__inner`}>
<Link
className={`menu__list-item__link ${listItemLinkStyle} ${current ? currentStyle : null}`}
aria-current={current ? 'page' : null}
href={href}
onClick={onLinkClick}
ref={ref}
tabIndex={level > Levels.Subcategory ? -1 : 0}
passHref
>
{pageNode.title}
{children && hasVisibleChildren && level !== Levels.Category && (
<Flex
className={`menu__list-item__link__inner ${listItemLinkInnerStyle}`}
>
{pageNode.title}
</Flex>
</Link>
{children && hasVisibleChildren && level !== Levels.Category && (
<Button
className={`${listItemLinkStyle} expand-button`}
onClick={onCheveronClick}
ref={ref}
aria-expanded="true"
aria-labelledby="li"
tabIndex={level > Levels.Subcategory ? -1 : 0}
>
<IconChevron className={open ? '' : 'icon-rotate-90-reverse'} />
)}
</Flex>
</Link>
</Button>
)}
</Flex>
{children && (
<ul
className={`menu__list ${
Expand All @@ -217,6 +278,7 @@ export function MenuItem({
parentSetOpen={setOpen}
level={level + 1}
currentPlatform={currentPlatform}
tabIndex={level > Levels.Subcategory ? -1 : 0}
/>
))}
</ul>
Expand Down
24 changes: 20 additions & 4 deletions src/styles/menu.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@
margin: 0;
padding: 0;

&__inner {
gap: 0;

&:hover {
background-color: var(--amplify-colors-overlay-5);
}
}

&--category {
&:not(:first-child) {
margin-top: 25px;
Expand All @@ -27,6 +35,7 @@
&__link {
color: var(--amplify-colors-font-primary);
text-decoration: none;
width: 100%;

&:focus {
outline: 2px auto var(--amplify-colors-border-focus);
Expand All @@ -48,10 +57,6 @@
justify-content: space-between;
align-items: center;

&:hover {
background-color: var(--amplify-colors-overlay-5);
}

&--subpage {
padding-left: calc(
var(--amplify-space-large) + var(--amplify-space-small)
Expand Down Expand Up @@ -88,5 +93,16 @@
}
}
}

.expand-button {
border: none;
margin-right: 8px;
background-color: transparent;
border-radius: 0;

&:hover {
background-color: var(--amplify-colors-overlay-5);
}
}
}
}