fix(ui): Improve Floating UI usage

.changeset/floating-ui-aria-fixes.md

@@ -0,0 +1,5 @@
+---
+'@clerk/ui': patch
+---
+
+Improve Floating UI usage: fix `arialLabel` typo in `MenuTrigger`, replace imperative floating ref in `MenuList` with `useMergeRefs`, remove manual position offset in `SelectOptionList`, add `aria-haspopup` to `MenuTrigger`, and add missing ARIA attributes (`aria-expanded`, `aria-haspopup`, `role`, `aria-selected`) to `Select` components.

packages/ui/src/elements/Menu.tsx

@@ -2,7 +2,9 @@ import { createContextAndHook } from '@clerk/shared/react';
 import type { MenuId } from '@clerk/shared/types';
 import type { Placement } 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 { useMergeRefs } from '@floating-ui/react';
 
 import type { Button } from '../customizables';
 import { Col, descriptors, SimpleButton } from '../customizables';
@@ -44,14 +46,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 { children, ariaLabel } = props;
   const { popoverCtx, elementId } = useMenuState();
   const { reference, toggle, isOpen } = popoverCtx;
 
-  const normalizedAriaLabel = typeof arialLabel === 'function' ? arialLabel(isOpen) : arialLabel;
+  const normalizedAriaLabel = typeof ariaLabel === 'function' ? ariaLabel(isOpen) : ariaLabel;
 
   if (!isValidElement(children)) {
     return null;
@@ -64,6 +66,7 @@ export const MenuTrigger = (props: MenuTriggerProps) => {
     elementId: children.props.elementId || descriptors.menuButton.setId(elementId),
     'aria-label': normalizedAriaLabel,
     'aria-expanded': isOpen,
+    'aria-haspopup': 'menu',
     onClick: (e: React.MouseEvent) => {
       children.props?.onClick?.(e);
       toggle();
@@ -90,11 +93,7 @@ export const MenuList = (props: MenuListProps) => {
   const { popoverCtx, elementId } = useMenuState();
   const { floating, styles, isOpen, context, nodeId } = popoverCtx;
   const containerRef = useRef<HTMLDivElement | null>(null);
-
-  useLayoutEffect(() => {
-    const current = containerRef.current;
-    floating(current);
-  }, [isOpen]);
+  const mergedRef = useMergeRefs([containerRef, floating]);
 
   const onKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
     const current = containerRef.current;
@@ -122,7 +121,7 @@ export const MenuList = (props: MenuListProps) => {
       <Col
         elementDescriptor={descriptors.menuList}
         elementId={descriptors.menuList.setId(elementId)}
-        ref={containerRef}
+        ref={mergedRef}
         role='menu'
         onKeyDown={onKeyDown}
         sx={[

packages/ui/src/elements/Select.tsx

@@ -46,6 +46,12 @@ type SelectState<O extends Option> = Pick<
   select: (option: O) => void;
   focusedItemRef: React.RefObject<HTMLDivElement>;
   onTriggerClick: () => void;
+  generatedTriggerId: string;
+  triggerId: string;
+  setTriggerId: React.Dispatch<React.SetStateAction<string>>;
+  generatedListboxId: string;
+  listboxId: string;
+  setListboxId: React.Dispatch<React.SetStateAction<string>>;
   portal?: boolean;
 };
 
@@ -102,6 +108,10 @@ export const Select = withFloatingTree(<O extends Option>(props: PropsWithChildr
   });
   const togglePopover = popoverCtx.toggle;
   const focusedItemRef = React.useRef<HTMLDivElement>(null);
+  const generatedTriggerId = React.useId();
+  const generatedListboxId = React.useId();
+  const [triggerId, setTriggerId] = React.useState(generatedTriggerId);
+  const [listboxId, setListboxId] = React.useState(generatedListboxId);
   const searchInputCtx = useSearchInput({
     items: options,
     comparator: comparator || (() => true),
@@ -139,6 +149,12 @@ export const Select = withFloatingTree(<O extends Option>(props: PropsWithChildr
           comparator,
           select,
           onTriggerClick: togglePopover,
+          generatedTriggerId,
+          triggerId,
+          setTriggerId,
+          generatedListboxId,
+          listboxId,
+          setListboxId,
           elementId,
           portal,
         },
@@ -167,6 +183,8 @@ const SelectRenderOption = React.memo(
     return (
       <Flex
         ref={ref}
+        role='option'
+        aria-selected={isSelected}
         sx={{
           userSelect: 'none',
           cursor: 'pointer',
@@ -240,7 +258,16 @@ type SelectOptionListProps = PropsOfComponent<typeof Flex> & {
 };
 
 export const SelectOptionList = (props: SelectOptionListProps) => {
-  const { containerSx, sx, footer, onReachEnd, ...rest } = props;
+  const {
+    containerSx,
+    sx,
+    footer,
+    onReachEnd,
+    id,
+    'aria-label': ariaLabel,
+    'aria-labelledby': ariaLabelledBy,
+    ...rest
+  } = props;
   const {
     popoverCtx,
     searchInputCtx,
@@ -253,12 +280,21 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
     select,
     onTriggerClick,
     elementId,
+    triggerId,
+    generatedListboxId,
+    setListboxId,
     portal,
   } = useSelectState();
   const { filteredItems: options, searchInputProps } = searchInputCtx;
   const [focusedIndex, setFocusedIndex] = useState(0);
   const { isOpen, floating, styles, nodeId, context } = popoverCtx;
   const containerRef = React.useRef<HTMLDivElement>(null);
+  const effectiveListboxId = id ?? generatedListboxId;
+  const effectiveAriaLabelledBy = ariaLabelledBy ?? (ariaLabel ? undefined : triggerId);
+
+  React.useEffect(() => {
+    setListboxId(effectiveListboxId);
+  }, [effectiveListboxId, setListboxId]);
 
   const scrollToItemOnSelectedIndexChange = () => {
     if (!isOpen) {
@@ -338,8 +374,7 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
           }),
           sx,
         ]}
-        // eslint-disable-next-line custom-rules/no-physical-css-properties -- Floating UI library positioning
-        style={{ ...styles, left: styles.left - 1 }}
+        style={styles}
       >
         {comparator && (
           <SelectSearchbar
@@ -349,7 +384,11 @@ export const SelectOptionList = (props: SelectOptionListProps) => {
         )}
         <Flex
           ref={containerRef}
+          id={effectiveListboxId}
           direction='col'
+          role='listbox'
+          aria-label={ariaLabel}
+          aria-labelledby={effectiveAriaLabelledBy}
           tabIndex={comparator ? undefined : 0}
           sx={[
             theme => ({
@@ -395,9 +434,24 @@ export const SelectButton = (
     iconSx?: ThemableCssProp;
   },
 ) => {
-  const { sx, children, icon, iconSx, ...rest } = props;
-  const { popoverCtx, onTriggerClick, buttonRenderOption, selectedOption, placeholder, elementId } = useSelectState();
-  const { reference } = popoverCtx;
+  const { sx, children, icon, iconSx, id, 'aria-controls': ariaControls, ...rest } = props;
+  const {
+    popoverCtx,
+    onTriggerClick,
+    buttonRenderOption,
+    selectedOption,
+    placeholder,
+    elementId,
+    generatedTriggerId,
+    listboxId,
+    setTriggerId,
+  } = useSelectState();
+  const { reference, isOpen } = popoverCtx;
+  const effectiveTriggerId = id ?? generatedTriggerId;
+
+  React.useEffect(() => {
+    setTriggerId(effectiveTriggerId);
+  }, [effectiveTriggerId, setTriggerId]);
 
   let show: React.ReactNode = children;
   if (!children) {
@@ -409,9 +463,13 @@ export const SelectButton = (
       elementDescriptor={descriptors.selectButton}
       elementId={descriptors.selectButton.setId(elementId)}
       ref={reference}
+      id={effectiveTriggerId}
       variant='outline'
       textVariant='buttonLarge'
       onClick={onTriggerClick}
+      aria-expanded={isOpen}
+      aria-haspopup='listbox'
+      aria-controls={ariaControls ?? (isOpen ? listboxId : undefined)}
       sx={[
         theme => ({
           gap: theme.space.$2,

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 => `${isOpen ? 'Close' : 'Open'} menu`}>
         <Button
           sx={t =>
             !isBordered

packages/ui/src/elements/__tests__/Select.test.tsx

@@ -0,0 +1,121 @@
+import { ClerkInstanceContext } from '@clerk/shared/react';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import type { PropsWithChildren, ReactElement } from 'react';
+import { describe, expect, it, vi } from 'vitest';
+
+import { EnvironmentProvider } from '../../contexts';
+import { AppearanceProvider } from '../../customizables';
+import { InternalThemeProvider } from '../../styledSystem';
+import { Select, SelectButton, SelectOptionList } from '../Select';
+
+const options = [
+  { value: 'one', label: 'One' },
+  { value: 'two', label: 'Two' },
+];
+
+const TestProviders = ({ children }: PropsWithChildren) => (
+  <ClerkInstanceContext.Provider value={{ value: { client: {}, user: {} } as any }}>
+    <EnvironmentProvider value={{ displayConfig: { applicationName: 'TestApp' } } as any}>
+      <AppearanceProvider>
+        <InternalThemeProvider>{children}</InternalThemeProvider>
+      </AppearanceProvider>
+    </EnvironmentProvider>
+  </ClerkInstanceContext.Provider>
+);
+
+const renderSelect = (ui: ReactElement) => {
+  return render(ui, { wrapper: TestProviders });
+};
+
+describe('Select', () => {
+  it('labels the listbox with the generated trigger id by default', async () => {
+    const user = userEvent.setup();
+    renderSelect(
+      <Select
+        options={options}
+        value={null}
+        onChange={vi.fn()}
+      />,
+    );
+
+    const button = screen.getByRole('button', { name: 'Select an option' });
+
+    await user.click(button);
+
+    const listbox = await screen.findByRole('listbox');
+
+    expect(button).toHaveAttribute('id');
+    expect(listbox).toHaveAttribute('id');
+    expect(listbox).toHaveAttribute('aria-labelledby', button.id);
+    expect(button).toHaveAttribute('aria-controls', listbox.id);
+  });
+
+  it('labels the listbox with an explicit trigger id', async () => {
+    const user = userEvent.setup();
+    renderSelect(
+      <Select
+        options={options}
+        value={null}
+        onChange={vi.fn()}
+      >
+        <SelectButton id='expiration-field' />
+        <SelectOptionList />
+      </Select>,
+    );
+
+    const button = document.getElementById('expiration-field') as HTMLButtonElement;
+
+    await user.click(button);
+
+    const listbox = await screen.findByRole('listbox');
+
+    expect(listbox).toHaveAttribute('aria-labelledby', 'expiration-field');
+    expect(button).toHaveAttribute('aria-controls', listbox.id);
+  });
+
+  it('preserves an explicit listbox aria-label', async () => {
+    const user = userEvent.setup();
+    renderSelect(
+      <Select
+        options={options}
+        value={null}
+        onChange={vi.fn()}
+      >
+        <SelectButton />
+        <SelectOptionList aria-label='Expiration options' />
+      </Select>,
+    );
+
+    await user.click(screen.getByRole('button', { name: 'Select an option' }));
+
+    const listbox = await screen.findByRole('listbox', { name: 'Expiration options' });
+
+    expect(listbox).toHaveAttribute('aria-label', 'Expiration options');
+    expect(listbox).not.toHaveAttribute('aria-labelledby');
+  });
+
+  it('preserves an explicit listbox aria-labelledby', async () => {
+    const user = userEvent.setup();
+    renderSelect(
+      <Select
+        options={options}
+        value={null}
+        onChange={vi.fn()}
+      >
+        <span id='external-label'>External options</span>
+        <SelectButton />
+        <SelectOptionList aria-labelledby='external-label' />
+      </Select>,
+    );
+
+    const button = screen.getByRole('button', { name: 'Select an option' });
+
+    await user.click(button);
+
+    const listbox = await screen.findByRole('listbox', { name: 'External options' });
+
+    expect(listbox).toHaveAttribute('aria-labelledby', 'external-label');
+    expect(listbox).not.toHaveAttribute('aria-labelledby', button.id);
+  });
+});