Improve popup-menu a11y (#3394)

AudiusProject · May 22, 2023 · 84a195b · 84a195b
1 parent 7b496b8
commit 84a195b
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 31 deletions.
diff --git a/apps/audius-client/packages/stems/src/components/PopupMenu/PopupMenu.module.css b/apps/audius-client/packages/stems/src/components/PopupMenu/PopupMenu.module.css
@@ -1,3 +1,7 @@
+.menu {
+  all: unset;
+}
+
 .item {
   display: flex;
   align-items: center;

diff --git a/apps/audius-client/packages/stems/src/components/PopupMenu/PopupMenu.stories.tsx b/apps/audius-client/packages/stems/src/components/PopupMenu/PopupMenu.stories.tsx
@@ -1,5 +1,3 @@
-import * as React from 'react'
-
 import { Story } from '@storybook/react'
 
 import { Button } from 'components/Button'
@@ -15,19 +13,20 @@ export default {
 
 const Template: Story<PopupMenuProps> = (args) => {
   return (
-    <>
-      <PopupMenu
-        {...args}
-        renderTrigger={(
-          anchorRef: React.MutableRefObject<any>,
-          triggerPopup: () => void
-        ) => {
-          return (
-            <Button text='Click me' ref={anchorRef} onClick={triggerPopup} />
-          )
-        }}
-      />
-    </>
+    <PopupMenu
+      {...args}
+      renderTrigger={(anchorRef, triggerPopup, triggerProps) => {
+        return (
+          <Button
+            text='Click me'
+            // @ts-ignore
+            ref={anchorRef}
+            onClick={triggerPopup}
+            {...triggerProps}
+          />
+        )
+      }}
+    />
   )
 }
 
@@ -47,7 +46,8 @@ const primaryProps: Omit<PopupMenuProps, 'renderTrigger'> = {
       text: 'Item 3',
       onClick: () => {}
     }
-  ]
+  ],
+  id: 'primary'
 }
 
 Primary.args = primaryProps
@@ -76,7 +76,8 @@ const withIconsProps: Omit<PopupMenuProps, 'renderTrigger'> = {
       icon: <IconLock />,
       onClick: () => {}
     }
-  ]
+  ],
+  id: 'with-icons'
 }
 
 WithIcons.args = withIconsProps
diff --git a/apps/audius-client/packages/stems/src/components/PopupMenu/PopupMenu.tsx b/apps/audius-client/packages/stems/src/components/PopupMenu/PopupMenu.tsx
@@ -12,8 +12,8 @@ import { PopupMenuItem, PopupMenuProps } from './types'
  * A menu that shows on top of the UI. Ideal for overflow menus, dropdowns, etc
  */
 export const PopupMenu = forwardRef<HTMLDivElement, PopupMenuProps>(
-  function PopupMenu(
-    {
+  function PopupMenu(props, ref) {
+    const {
       items,
       onClose,
       position,
@@ -24,12 +24,11 @@ export const PopupMenu = forwardRef<HTMLDivElement, PopupMenuProps>(
       zIndex,
       containerRef,
       anchorOrigin,
-      transformOrigin
-    },
-    ref
-  ) {
+      transformOrigin,
+      id
+    } = props
     const clickInsideRef = useRef<any>()
-    const anchorRef = useRef<HTMLElement | null>(null)
+    const anchorRef = useRef<HTMLElement>(null)
 
     const [isPopupVisible, setIsPopupVisible] = useState<boolean>(false)
 
@@ -52,9 +51,18 @@ export const PopupMenu = forwardRef<HTMLDivElement, PopupMenuProps>(
       [handlePopupClose]
     )
 
+    const triggerId = id ? `${id}-trigger` : undefined
+
+    const triggerProps = {
+      'aria-controls': isPopupVisible ? id : undefined,
+      'aria-haspopup': true,
+      'aria-expanded': isPopupVisible ? ('true' as const) : undefined,
+      id: triggerId
+    }
+
     return (
       <div ref={clickInsideRef}>
-        {renderTrigger(anchorRef, triggerPopup)}
+        {renderTrigger(anchorRef, triggerPopup, triggerProps)}
         <Popup
           anchorRef={anchorRef}
           checkIfClickInside={(target: EventTarget) => {
@@ -76,22 +84,29 @@ export const PopupMenu = forwardRef<HTMLDivElement, PopupMenuProps>(
           transformOrigin={transformOrigin}
           anchorOrigin={anchorOrigin}
         >
-          <div className={styles.menu}>
+          <ul
+            className={styles.menu}
+            role='menu'
+            aria-labelledby={triggerId}
+            tabIndex={-1}
+          >
             {items.map((item, i) => (
-              <div
-                key={typeof item.text === 'string' ? `${item.text}_${i}` : i}
+              <li
+                key={typeof item.text === 'string' ? item.text : i}
+                role='menuitem'
                 className={cn(styles.item, item.className)}
                 onClick={handleMenuItemClick(item)}
+                tabIndex={i === 0 ? 0 : -1}
               >
                 {item.icon && (
                   <div className={cn(styles.icon, item.iconClassName)}>
                     {item.icon}
                   </div>
                 )}
                 {item.text}
-              </div>
+              </li>
             ))}
-          </div>
+          </ul>
         </Popup>
       </div>
     )

diff --git a/apps/audius-client/packages/stems/src/components/PopupMenu/types.tsx b/apps/audius-client/packages/stems/src/components/PopupMenu/types.tsx
@@ -1,3 +1,5 @@
+import { ComponentProps } from 'react'
+
 import { PopupProps } from '../Popup'
 
 type ApplicablePopupProps = Pick<
@@ -30,8 +32,13 @@ export type PopupMenuProps = {
    */
   renderTrigger: (
     anchorRef: React.MutableRefObject<any>,
-    triggerPopup: () => void
+    triggerPopup: () => void,
+    triggerProps: Partial<ComponentProps<'button'>>
   ) => React.ReactNode
+  /**
+   * Providing an id is necessary for proper a11y
+   */
+  id?: string
 } & ApplicablePopupProps
 
 export type PopupMenuItem = {