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) => { = Pick< select: (option: O) => void; focusedItemRef: React.RefObject; onTriggerClick: () => void; + generatedTriggerId: string; + triggerId: string; + setTriggerId: React.Dispatch>; + generatedListboxId: string; + listboxId: string; + setListboxId: React.Dispatch>; portal?: boolean; }; @@ -102,6 +108,10 @@ export const Select = withFloatingTree((props: PropsWithChildr }); const togglePopover = popoverCtx.toggle; const focusedItemRef = React.useRef(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((props: PropsWithChildr comparator, select, onTriggerClick: togglePopover, + generatedTriggerId, + triggerId, + setTriggerId, + generatedListboxId, + listboxId, + setListboxId, elementId, portal, }, @@ -167,6 +183,8 @@ const SelectRenderOption = React.memo( return ( & { }; 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(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 && ( { )} ({ @@ -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, 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`}>