Merge pull request #3191 from tespin/tespin-fix/menu-ally

Adds menubar ARIA semantics to editor menu

Author: raclim

Diff for: client/components/Dropdown/DropdownMenu.jsx

@@ -38,7 +38,7 @@ const DropdownMenu = forwardRef(
     };
 
     return (
-      <div ref={anchorRef} className={className}>
+      <div ref={anchorRef} className={className} aria-haspopup="menu">
         <button
           className={classes.button}
           aria-label={ariaLabel}
@@ -51,6 +51,7 @@ const DropdownMenu = forwardRef(
         </button>
         {isOpen && (
           <DropdownWrapper
+            role="menu"
             className={classes.list}
             align={align}
             onMouseUp={() => {

Diff for: client/components/Dropdown/MenuItem.jsx

@@ -10,7 +10,7 @@ function MenuItem({ hideIf, ...rest }) {
   }
 
   return (
-    <li>
+    <li role="menuitem">
       <ButtonOrLink {...rest} />
     </li>
   );

Diff for: client/components/Nav/NavBar.jsx

@@ -69,11 +69,11 @@ function NavBar({ children, className }) {
   return (
     <NavBarContext.Provider value={contextValue}>
       <header>
-        <nav className={className} ref={nodeRef}>
+        <div className={className} ref={nodeRef}>
           <MenuOpenContext.Provider value={dropdownOpen}>
             {children}
           </MenuOpenContext.Provider>
-        </nav>
+        </div>
       </header>
     </NavBarContext.Provider>
   );

Diff for: client/components/Nav/NavDropdownMenu.jsx

@@ -24,15 +24,20 @@ function NavDropdownMenu({ id, title, children }) {
 
   return (
     <li className={classNames('nav__item', isOpen && 'nav__item--open')}>
-      <button {...handlers}>
+      <button
+        {...handlers}
+        role="menuitem"
+        aria-haspopup="menu"
+        aria-expanded={isOpen}
+      >
         <span className="nav__item-header">{title}</span>
         <TriangleIcon
           className="nav__item-header-triangle"
           focusable="false"
           aria-hidden="true"
         />
       </button>
-      <ul className="nav__dropdown">
+      <ul className="nav__dropdown" role="menu">
         <ParentMenuContext.Provider value={id}>
           {children}
         </ParentMenuContext.Provider>

Diff for: client/components/Nav/NavMenuItem.jsx

@@ -19,7 +19,7 @@ function NavMenuItem({ hideIf, className, ...rest }) {
 
   return (
     <li className={className}>
-      <ButtonOrLink {...rest} {...handlers} />
+      <ButtonOrLink {...rest} {...handlers} role="menuitem" />
     </li>
   );
 }

Diff for: client/index.integration.test.jsx

@@ -73,15 +73,15 @@ describe('index.jsx integration', () => {
   });
 
   it('navbar items and the dropdowns in the navbar exist', () => {
-    const navigation = screen.getByRole('navigation');
+    const navigation = screen.getByRole('menubar');
     expect(navigation).toBeInTheDocument();
 
-    const fileButton = within(navigation).getByRole('button', {
+    const fileButton = within(navigation).getByRole('menuitem', {
       name: /^file$/i
     });
     expect(fileButton).toBeInTheDocument();
 
-    const newFileButton = within(navigation).getByRole('button', {
+    const newFileButton = within(navigation).getByRole('menuitem', {
       name: /^new$/i
     });
     expect(newFileButton).toBeInTheDocument();
@@ -91,17 +91,17 @@ describe('index.jsx integration', () => {
     // const exampleFileButton = within(navigation).getByRole('link', {name: /^examples$/i});
     // expect(exampleFileButton).toBeInTheDocument();
 
-    const editButton = within(navigation).getByRole('button', {
+    const editButton = within(navigation).getByRole('menuitem', {
       name: /^edit$/i
     });
     expect(editButton).toBeInTheDocument();
 
-    const sketchButton = within(navigation).getByRole('button', {
+    const sketchButton = within(navigation).getByRole('menuitem', {
       name: /^sketch$/i
     });
     expect(sketchButton).toBeInTheDocument();
 
-    const helpButton = within(navigation).getByRole('button', {
+    const helpButton = within(navigation).getByRole('menuitem', {
       name: /^help$/i
     });
     expect(helpButton).toBeInTheDocument();

Diff for: client/modules/IDE/components/Header/Nav.jsx

@@ -135,7 +135,7 @@ const ProjectMenu = () => {
     metaKey === 'Ctrl' ? `${metaKeyName}+H` : `${metaKeyName}+⌥+F`;
 
   return (
-    <ul className="nav__items-left">
+    <ul className="nav__items-left" role="menubar">
       <li className="nav__item-logo">
         {user && user.username !== undefined ? (
           <Link to={userSketches}>
@@ -272,18 +272,18 @@ const LanguageMenu = () => {
 const UnauthenticatedUserMenu = () => {
   const { t } = useTranslation();
   return (
-    <ul className="nav__items-right" title="user-menu">
+    <ul className="nav__items-right" title="user-menu" role="navigation">
       {getConfig('TRANSLATIONS_ENABLED') && <LanguageMenu />}
       <li className="nav__item">
-        <Link to="/login" className="nav__auth-button">
+        <Link to="/login" className="nav__auth-button" role="menuitem">
           <span className="nav__item-header" title="Login">
             {t('Nav.Login')}
           </span>
         </Link>
       </li>
       <li className="nav__item-or">{t('Nav.LoginOr')}</li>
       <li className="nav__item">
-        <Link to="/signup" className="nav__auth-button">
+        <Link to="/signup" className="nav__auth-button" role="menuitem">
           <span className="nav__item-header" title="SignUp">
             {t('Nav.SignUp')}
           </span>
@@ -300,7 +300,7 @@ const AuthenticatedUserMenu = () => {
   const dispatch = useDispatch();
 
   return (
-    <ul className="nav__items-right" title="user-menu">
+    <ul className="nav__items-right" title="user-menu" role="navigation">
       {getConfig('TRANSLATIONS_ENABLED') && <LanguageMenu />}
       <NavDropdownMenu
         id="account"