diff --git a/.changeset/floating-ui-aria-fixes.md b/.changeset/floating-ui-aria-fixes.md new file mode 100644 index 00000000000..7637f302635 --- /dev/null +++ b/.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. diff --git a/packages/ui/src/elements/Menu.tsx b/packages/ui/src/elements/Menu.tsx index 0579758d082..89351dc0ce0 100644 --- a/packages/ui/src/elements/Menu.tsx +++ b/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(null); - - useLayoutEffect(() => { - const current = containerRef.current; - floating(current); - }, [isOpen]); + const mergedRef = useMergeRefs([containerRef, floating]); const onKeyDown = (e: React.KeyboardEvent) => { const current = containerRef.current; @@ -122,7 +121,7 @@ export const MenuList = (props: MenuListProps) => { { sx, ]} // eslint-disable-next-line custom-rules/no-physical-css-properties -- Floating UI library positioning - style={{ ...styles, left: styles.left - 1 }} + style={styles} > {comparator && ( { ({ @@ -397,7 +400,7 @@ export const SelectButton = ( ) => { const { sx, children, icon, iconSx, ...rest } = props; const { popoverCtx, onTriggerClick, buttonRenderOption, selectedOption, placeholder, elementId } = useSelectState(); - const { reference } = popoverCtx; + const { reference, isOpen } = popoverCtx; let show: React.ReactNode = children; if (!children) { @@ -412,6 +415,8 @@ export const SelectButton = ( variant='outline' textVariant='buttonLarge' onClick={onTriggerClick} + aria-expanded={isOpen} + aria-haspopup='listbox' sx={[ theme => ({ gap: theme.space.$2, diff --git a/packages/ui/src/elements/ThreeDotsMenu.tsx b/packages/ui/src/elements/ThreeDotsMenu.tsx index eda7d587c30..f276fa34c3f 100644 --- a/packages/ui/src/elements/ThreeDotsMenu.tsx +++ b/packages/ui/src/elements/ThreeDotsMenu.tsx @@ -33,7 +33,7 @@ export const ThreeDotsMenu = (props: ThreeDotsMenuProps) => { return ( - `${isOpen ? 'Close' : 'Open'} menu`}> + `${isOpen ? 'Close' : 'Open'} menu`}>