refactor(ui): Menu interaction and accessibility improvements

.changeset/menu-keyboard-a11y.md

@@ -0,0 +1,5 @@
+---
+'@clerk/ui': patch
+---
+
+Improve Menu keyboard navigation and accessibility. Menus now support `Enter`/`Space` to open from the trigger, `ArrowDown`/`ArrowUp`/`Home`/`End` to move focus, `Escape` to close and return focus to the trigger, and skip disabled items during arrow-key navigation. Menus no longer mark the rest of the page as `aria-hidden` while open, so assistive technologies can still reach surrounding content.

packages/ui/src/components/UserProfile/__tests__/ConnectedAccountsSection.test.tsx

@@ -1,12 +1,31 @@
 import type { ExternalAccountResource } from '@clerk/shared/types';
 import { act, waitFor } from '@testing-library/react';
+import type { ComponentType, PropsWithChildren } from 'react';
 import { describe, expect, it } from 'vitest';
 
 import { bindCreateFixtures } from '@/test/create-fixtures';
 import { render, screen } from '@/test/utils';
 
+import { AppearanceProvider } from '../../../customizables';
 import { ConnectedAccountsSection } from '../ConnectedAccountsSection';
 
+// jsdom never fires CSS `animationend`/`transitionend` events, so
+// `@formkit/auto-animate` (used by ProfileSection.ItemList) can never complete
+// its exit animation and retains unmounted DOM nodes indefinitely. This
+// prevents tests from deterministically asserting that a subtree was removed
+// after a state change. Disabling animations via appearance options makes the
+// Animated wrapper a no-op so React's unmount is reflected in the DOM
+// synchronously. The real-browser animation path is covered by manual testing
+// and by `Collapsible.test.tsx`.
+const withAnimationsDisabled = (Wrapper: ComponentType<PropsWithChildren>) => {
+  const WithAnimationsDisabled = ({ children }: PropsWithChildren) => (
+    <Wrapper>
+      <AppearanceProvider appearance={{ options: { animations: false } }}>{children}</AppearanceProvider>
+    </Wrapper>
+  );
+  return WithAnimationsDisabled;
+};
+
 const { createFixtures } = bindCreateFixtures('UserProfile');
 
 const withoutConnections = createFixtures.config(f => {
@@ -280,7 +299,9 @@ describe('ConnectedAccountsSection ', () => {
   describe('Handles opening/closing actions', () => {
     it('closes remove account form when connect account action is clicked', async () => {
       const { wrapper } = await createFixtures(withSomeConnections);
-      const { userEvent, getByText, getByRole, queryByRole } = render(<ConnectedAccountsSection />, { wrapper });
+      const { userEvent, getByText, getByRole, queryByRole } = render(<ConnectedAccountsSection />, {
+        wrapper: withAnimationsDisabled(wrapper),
+      });
 
       const item = getByText(/google/i);
       const menuButton = item.parentElement?.parentElement?.parentElement?.parentElement?.children?.[1];

packages/ui/src/elements/Menu.tsx

@@ -1,8 +1,17 @@
 import { createContextAndHook } from '@clerk/shared/react';
 import type { MenuId } from '@clerk/shared/types';
 import type { Placement } from '@floating-ui/react';
+import {
+  FloatingList,
+  useClick,
+  useInteractions,
+  useListItem,
+  useListNavigation,
+  useMergeRefs,
+  useRole,
+} from '@floating-ui/react';
 import type { PropsWithChildren } from 'react';
-import React, { cloneElement, isValidElement, useLayoutEffect, useRef } from 'react';
+import React, { cloneElement, isValidElement, useRef } from 'react';
 
 import type { Button } from '../customizables';
 import { Col, descriptors, SimpleButton } from '../customizables';
@@ -14,9 +23,16 @@ import { colors } from '../utils/colors';
 import { withFloatingTree } from './contexts';
 import { Popover } from './Popover';
 
+type UseInteractionsReturn = ReturnType<typeof useInteractions>;
+
 type MenuState = {
   popoverCtx: UsePopoverReturn;
   elementId?: MenuId;
+  getReferenceProps: UseInteractionsReturn['getReferenceProps'];
+  getFloatingProps: UseInteractionsReturn['getFloatingProps'];
+  getItemProps: UseInteractionsReturn['getItemProps'];
+  activeIndex: number | null;
+  elementsRef: React.MutableRefObject<Array<HTMLElement | null>>;
 };
 
 export const [MenuStateCtx, useMenuState] = createContextAndHook<MenuState>('MenuState');
@@ -33,8 +49,41 @@ export const Menu = withFloatingTree((props: MenuProps) => {
     offset: 8,
     shoudFlip: true,
   });
+  const { context, isOpen } = popoverCtx;
+
+  const [activeIndex, setActiveIndex] = React.useState<number | null>(null);
+  const elementsRef = useRef<Array<HTMLElement | null>>([]);
+
+  React.useEffect(() => {
+    if (!isOpen) {
+      setActiveIndex(null);
+    }
+  }, [isOpen]);
 
-  const value = React.useMemo(() => ({ value: { popoverCtx, elementId } }), [{ ...popoverCtx }, elementId]);
+  const click = useClick(context);
+  const role = useRole(context, { role: 'menu' });
+  const listNavigation = useListNavigation(context, {
+    listRef: elementsRef,
+    activeIndex,
+    onNavigate: setActiveIndex,
+    loop: true,
+  });
+  const { getReferenceProps, getFloatingProps, getItemProps } = useInteractions([click, role, listNavigation]);
+
+  const value = React.useMemo(
+    () => ({
+      value: {
+        popoverCtx,
+        elementId,
+        getReferenceProps,
+        getFloatingProps,
+        getItemProps,
+        activeIndex,
+        elementsRef,
+      },
+    }),
+    [popoverCtx, elementId, getReferenceProps, getFloatingProps, getItemProps, activeIndex],
+  );
 
   return (
     <MenuStateCtx.Provider
@@ -44,14 +93,14 @@ export const Menu = withFloatingTree((props: MenuProps) => {
   );
 });
 
-type MenuTriggerProps = React.PropsWithChildren<{ arialLabel?: string | ((open: boolean) => string) }>;
+type MenuTriggerProps = React.PropsWithChildren<{ ariaLabel?: string | ((open: boolean) => string) }>;
 
 export const MenuTrigger = (props: MenuTriggerProps) => {
-  const { children, arialLabel } = props;
-  const { popoverCtx, elementId } = useMenuState();
-  const { reference, toggle, isOpen } = popoverCtx;
+  const { children, ariaLabel } = props;
+  const { popoverCtx, elementId, getReferenceProps } = useMenuState();
+  const { reference, isOpen } = popoverCtx;
 
-  const normalizedAriaLabel = typeof arialLabel === 'function' ? arialLabel(isOpen) : arialLabel;
+  const normalizedAriaLabel = typeof ariaLabel === 'function' ? ariaLabel(isOpen) : ariaLabel;
 
   if (!isValidElement(children)) {
     return null;
@@ -63,89 +112,60 @@ export const MenuTrigger = (props: MenuTriggerProps) => {
     elementDescriptor: children.props.elementDescriptor || descriptors.menuButton,
     elementId: children.props.elementId || descriptors.menuButton.setId(elementId),
     'aria-label': normalizedAriaLabel,
-    'aria-expanded': isOpen,
-    onClick: (e: React.MouseEvent) => {
-      children.props?.onClick?.(e);
-      toggle();
-    },
+    ...getReferenceProps({
+      onClick: children.props?.onClick,
+    }),
   });
 };
 
-const findMenuItem = (el: Element, siblingType: 'prev' | 'next', options?: { countSelf?: boolean }) => {
-  let tagName = options?.countSelf ? el.tagName : '';
-  let sibling: Element | null = el;
-  while (sibling && tagName.toUpperCase() !== 'BUTTON') {
-    sibling = sibling[siblingType === 'prev' ? 'previousElementSibling' : 'nextElementSibling'];
-    tagName = sibling?.tagName ?? '';
-  }
-  return sibling;
-};
-
 type MenuListProps = PropsOfComponent<typeof Col> & {
   asPortal?: boolean;
 };
 
 export const MenuList = (props: MenuListProps) => {
   const { sx, asPortal, ...rest } = props;
-  const { popoverCtx, elementId } = useMenuState();
+  const { popoverCtx, elementId, getFloatingProps, elementsRef } = useMenuState();
   const { floating, styles, isOpen, context, nodeId } = popoverCtx;
   const containerRef = useRef<HTMLDivElement | null>(null);
-
-  useLayoutEffect(() => {
-    const current = containerRef.current;
-    floating(current);
-  }, [isOpen]);
-
-  const onKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
-    const current = containerRef.current;
-    if (!current) {
-      return;
-    }
-
-    if (current !== document.activeElement) {
-      return;
-    }
-
-    if (e.key === 'ArrowDown') {
-      e.preventDefault();
-      return (findMenuItem(current.children[0], 'next', { countSelf: true }) as HTMLElement)?.focus();
-    }
-  };
+  const mergedRef = useMergeRefs([containerRef, floating]);
 
   return (
     <Popover
       context={context}
       nodeId={nodeId}
       isOpen={isOpen}
       portal={asPortal}
+      order={['content']}
+      modal={false}
     >
-      <Col
-        elementDescriptor={descriptors.menuList}
-        elementId={descriptors.menuList.setId(elementId)}
-        ref={containerRef}
-        role='menu'
-        onKeyDown={onKeyDown}
-        sx={[
-          t => ({
-            backgroundColor: colors.makeSolid(t.colors.$colorBackground),
-            borderWidth: t.borderWidths.$normal,
-            borderStyle: t.borderStyles.$solid,
-            borderColor: t.colors.$borderAlpha150,
-            outline: 'none',
-            borderRadius: t.radii.$md,
-            padding: t.space.$0x5,
-            overflow: 'hidden',
-            top: `calc(100% + ${t.space.$2})`,
-            animation: `${animations.dropdownSlideInScaleAndFade} ${t.transitionDuration.$slower} ${t.transitionTiming.$slowBezier}`,
-            transformOrigin: 'top center',
-            zIndex: t.zIndices.$dropdown,
-            gap: t.space.$0x5,
-          }),
-          sx,
-        ]}
-        style={styles}
-        {...rest}
-      />
+      <FloatingList elementsRef={elementsRef}>
+        <Col
+          elementDescriptor={descriptors.menuList}
+          elementId={descriptors.menuList.setId(elementId)}
+          ref={mergedRef}
+          sx={[
+            t => ({
+              backgroundColor: colors.makeSolid(t.colors.$colorBackground),
+              borderWidth: t.borderWidths.$normal,
+              borderStyle: t.borderStyles.$solid,
+              borderColor: t.colors.$borderAlpha150,
+              outline: 'none',
+              borderRadius: t.radii.$md,
+              padding: t.space.$0x5,
+              overflow: 'hidden',
+              top: `calc(100% + ${t.space.$2})`,
+              animation: `${animations.dropdownSlideInScaleAndFade} ${t.transitionDuration.$slower} ${t.transitionTiming.$slowBezier}`,
+              transformOrigin: 'top center',
+              zIndex: t.zIndices.$dropdown,
+              gap: t.space.$0x5,
+            }),
+            sx,
+          ]}
+          style={styles}
+          {...getFloatingProps()}
+          {...rest}
+        />
+      </FloatingList>
     </Popover>
   );
 };
@@ -157,43 +177,30 @@ type MenuItemProps = PropsOfComponent<typeof Button> & {
 
 export const MenuItem = (props: MenuItemProps) => {
   const { sx, onClick, destructive, closeAfterClick = true, ...rest } = props;
-  const { popoverCtx, elementId } = useMenuState();
+  const { popoverCtx, elementId, getItemProps, activeIndex } = useMenuState();
   const { toggle } = popoverCtx;
-  const buttonRef = useRef<HTMLButtonElement>(null);
-
-  const onKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => {
-    const current = buttonRef.current;
-    if (!current) {
-      return;
-    }
-
-    const key = e.key;
-    if (key !== 'ArrowUp' && key !== 'ArrowDown') {
-      return;
-    }
-
-    e.preventDefault();
-    const sibling = findMenuItem(current, key === 'ArrowUp' ? 'prev' : 'next');
-    (sibling as HTMLElement)?.focus();
-  };
+  const item = useListItem();
+  const isActive = item.index === activeIndex;
 
   return (
     <SimpleButton
-      ref={buttonRef}
+      ref={item.ref}
       elementDescriptor={descriptors.menuItem}
       elementId={descriptors.menuItem.setId(elementId)}
       hoverAsFocus
       variant='ghost'
       colorScheme={destructive ? 'danger' : 'neutral'}
       role='menuitem'
-      onKeyDown={onKeyDown}
+      tabIndex={isActive ? 0 : -1}
       focusRing={false}
-      onClick={e => {
-        onClick?.(e);
-        if (closeAfterClick) {
-          toggle();
-        }
-      }}
+      {...getItemProps({
+        onClick: (e: React.MouseEvent<HTMLButtonElement>) => {
+          onClick?.(e);
+          if (closeAfterClick) {
+            toggle();
+          }
+        },
+      })}
       sx={[
         theme => ({
           justifyContent: 'start',

packages/ui/src/elements/Popover.tsx

@@ -15,6 +15,13 @@ type PopoverProps = PropsWithChildren<{
    */
   outsideElementsInert?: boolean;
   order?: Array<'reference' | 'floating' | 'content'>;
+  /**
+   * Whether focus is trapped inside the floating element and outside elements
+   * are marked `aria-hidden`. Dialogs should be `true`; menus / dropdowns should
+   * be `false` so the trigger remains in the accessibility tree.
+   * @default true
+   */
+  modal?: boolean;
   portal?: boolean;
   /**
    * The root element to render the portal into.
@@ -29,6 +36,7 @@ export const Popover = (props: PopoverProps) => {
     initialFocus,
     outsideElementsInert = false,
     order = ['reference', 'content'],
+    modal = true,
     nodeId,
     isOpen,
     portal = true,
@@ -49,6 +57,7 @@ export const Popover = (props: PopoverProps) => {
               initialFocus={initialFocus}
               outsideElementsInert={outsideElementsInert}
               order={order}
+              modal={modal}
             >
               <>{children}</>
             </FloatingFocusManager>
@@ -65,6 +74,7 @@ export const Popover = (props: PopoverProps) => {
           context={context}
           initialFocus={initialFocus}
           order={order}
+          modal={modal}
         >
           <>{children}</>
         </FloatingFocusManager>

packages/ui/src/elements/ThreeDotsMenu.tsx

@@ -33,7 +33,7 @@ export const ThreeDotsMenu = (props: ThreeDotsMenuProps) => {
 
   return (
     <Menu elementId={elementId}>
-      <MenuTrigger arialLabel={isOpen => `${isOpen ? 'Close' : 'Open'} menu`}>
+      <MenuTrigger ariaLabel={(isOpen: boolean) => `${isOpen ? 'Close' : 'Open'} menu`}>
         <Button
           sx={t =>
             !isBordered