diff --git a/docs/reference/generated/menu-submenu-trigger.json b/docs/reference/generated/menu-submenu-trigger.json index 0e5ad4665b..cf52883414 100644 --- a/docs/reference/generated/menu-submenu-trigger.json +++ b/docs/reference/generated/menu-submenu-trigger.json @@ -6,11 +6,6 @@ "type": "string", "description": "Overrides the text label to use when the item is matched during keyboard text navigation." }, - "disabled": { - "type": "boolean", - "default": "false", - "description": "Whether the component should ignore user interaction." - }, "className": { "type": "string | (state) => string", "description": "CSS class applied to the element, or a function that\nreturns a class based on the component’s state." diff --git a/docs/src/app/(private)/experiments/menu/menu-fully-featured.tsx b/docs/src/app/(private)/experiments/menu/menu-fully-featured.tsx index 36a68247b9..d3ccb455b0 100644 --- a/docs/src/app/(private)/experiments/menu/menu-fully-featured.tsx +++ b/docs/src/app/(private)/experiments/menu/menu-fully-featured.tsx @@ -12,16 +12,31 @@ interface Settings { customAnchor: boolean; modal: boolean; openOnHover: boolean; + disabled: boolean; + customTriggerElement: boolean; + side: Menu.Positioner.Props['side']; + align: Menu.Positioner.Props['align']; } export default function MenuFullyFeatured() { const { settings } = useExperimentSettings(); + const anchorRef = React.useRef(null); + const triggerRender = React.useMemo( + () => (settings.customTriggerElement ? : undefined), + [settings.customTriggerElement], + ); + return (
- - +

Fully featured menu

+ + Menu @@ -29,6 +44,8 @@ export default function MenuFullyFeatured() { className={classes.Positioner} sideOffset={8} anchor={settings.customAnchor ? anchorRef : undefined} + side={settings.side} + align={settings.align} > @@ -59,7 +76,7 @@ export default function MenuFullyFeatured() { Radio items - + @@ -69,7 +86,7 @@ export default function MenuFullyFeatured() { Option 1 - + @@ -81,7 +98,7 @@ export default function MenuFullyFeatured() { + + + + + + Disabled option + + @@ -139,6 +172,18 @@ export default function MenuFullyFeatured() { Option C (close on click) + + + + + + Disabled option + + @@ -171,6 +216,22 @@ export default function MenuFullyFeatured() { + + + + Disabled nested menu + + + + + + + This should not appear + + + + + @@ -199,6 +260,26 @@ export const settingsMetadata: SettingsMetadata = { type: 'boolean', label: 'Open on hover', }, + disabled: { + type: 'boolean', + label: 'Disabled', + }, + customTriggerElement: { + type: 'boolean', + label: 'Trigger as ', + }, + side: { + type: 'string', + label: 'Side', + options: ['top', 'right', 'bottom', 'left'], + default: 'bottom', + }, + align: { + type: 'string', + label: 'Align', + options: ['start', 'center', 'end'], + default: 'center', + }, }; function ChevronDownIcon(props: React.ComponentProps<'svg'>) { diff --git a/docs/src/app/(private)/experiments/menu/menu-nested.tsx b/docs/src/app/(private)/experiments/menu/menu-nested.tsx index 54b2ed18ed..fa097cb21a 100644 --- a/docs/src/app/(private)/experiments/menu/menu-nested.tsx +++ b/docs/src/app/(private)/experiments/menu/menu-nested.tsx @@ -82,7 +82,7 @@ export default function NestedMenu() { Paragraph - List + List - List + List { - return mergeReactProps<'div'>(externalProps, { - style: arrowStyles, - 'aria-hidden': true, - }); - }, - [arrowStyles], - ); - const state: MenuArrow.State = React.useMemo( () => ({ open, @@ -48,12 +37,15 @@ const MenuArrow = React.forwardRef(function MenuArrow( const mergedRef = useForkRef(arrowRef, forwardedRef); const { renderElement } = useComponentRenderer({ - propGetter: getArrowProps, render: render ?? 'div', className, state, ref: mergedRef, - extraProps: otherProps, + extraProps: { + style: arrowStyles, + 'aria-hidden': true, + ...otherProps, + }, customStyleHookMapping: popupStateMapping, }); diff --git a/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.tsx b/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.tsx index 1cbb378d0f..06cb2d422c 100644 --- a/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.tsx +++ b/packages/react/src/menu/checkbox-item-indicator/MenuCheckboxItemIndicator.tsx @@ -5,7 +5,6 @@ import { useMenuCheckboxItemContext } from '../checkbox-item/MenuCheckboxItemCon import { useComponentRenderer } from '../../utils/useComponentRenderer'; import { BaseUIComponentProps } from '../../utils/types'; import { itemMapping } from '../utils/styleHookMapping'; -import { mergeReactProps } from '../../utils/mergeReactProps'; import { TransitionStatus, useTransitionStatus } from '../../utils/useTransitionStatus'; import { useOpenChangeComplete } from '../../utils/useOpenChangeComplete'; import { useForkRef } from '../../utils/useForkRef'; @@ -29,14 +28,6 @@ const MenuCheckboxItemIndicator = React.forwardRef(function MenuCheckboxItemIndi const { transitionStatus, setMounted } = useTransitionStatus(item.checked); - const getItemProps = React.useCallback( - (externalProps = {}) => - mergeReactProps(externalProps, { - 'aria-hidden': true, - }), - [], - ); - useOpenChangeComplete({ open: item.checked, ref: indicatorRef, @@ -58,12 +49,14 @@ const MenuCheckboxItemIndicator = React.forwardRef(function MenuCheckboxItemIndi ); const { renderElement } = useComponentRenderer({ - propGetter: getItemProps, render: render || 'span', className, state, customStyleHookMapping: itemMapping, - extraProps: other, + extraProps: { + 'aria-hidden': true, + ...other, + }, ref: mergedRef, }); diff --git a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx index 4c9811122a..dba00aec47 100644 --- a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx +++ b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx @@ -364,18 +364,18 @@ describe('', () => { expect(item).toHaveFocus(); fireEvent.keyDown(item, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleCheckedChange.callCount).to.equal(0); fireEvent.keyUp(item, { key: 'Space' }); - expect(handleKeyUp.callCount).to.equal(1); + expect(handleKeyUp.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleCheckedChange.callCount).to.equal(0); fireEvent.click(item); - expect(handleKeyDown.callCount).to.equal(1); - expect(handleKeyUp.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); + expect(handleKeyUp.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleCheckedChange.callCount).to.equal(0); }); diff --git a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.tsx b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.tsx index 4daa70f132..11b5c9c6b3 100644 --- a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.tsx +++ b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.tsx @@ -11,6 +11,7 @@ import type { BaseUIComponentProps, GenericHTMLProps } from '../../utils/types'; import { useForkRef } from '../../utils/useForkRef'; import { itemMapping } from '../utils/styleHookMapping'; import { useCompositeListItem } from '../../composite/list/useCompositeListItem'; +import { mergeReactProps } from '../../utils/mergeReactProps'; const InnerMenuCheckboxItem = React.forwardRef(function InnerMenuItem( props: InnerMenuCheckboxItemProps, @@ -26,14 +27,14 @@ const InnerMenuCheckboxItem = React.forwardRef(function InnerMenuItem( highlighted, id, menuEvents, - propGetter, + itemProps, render, allowMouseUpTriggerRef, typingRef, ...other } = props; - const { getRootProps, checked } = useMenuCheckboxItem({ + const { getItemProps, checked } = useMenuCheckboxItem({ closeOnClick, disabled, highlighted, @@ -56,7 +57,7 @@ const InnerMenuCheckboxItem = React.forwardRef(function InnerMenuItem( render: render || 'div', className, state, - propGetter: (externalProps) => propGetter(getRootProps(externalProps)), + propGetter: (externalProps) => mergeReactProps(getItemProps, externalProps, itemProps), customStyleHookMapping: itemMapping, extraProps: other, }); @@ -118,6 +119,10 @@ InnerMenuCheckboxItem.propTypes /* remove-proptypes */ = { * @ignore */ id: PropTypes.string, + /** + * @ignore + */ + itemProps: PropTypes.object.isRequired, /** * Overrides the text label to use when the item is matched during keyboard text navigation. */ @@ -138,10 +143,6 @@ InnerMenuCheckboxItem.propTypes /* remove-proptypes */ = { * The click handler for the menu item. */ onClick: PropTypes.func, - /** - * @ignore - */ - propGetter: PropTypes.func.isRequired, /** * Allows you to replace the component’s HTML element * with a different tag, or compose it with another component. @@ -175,7 +176,7 @@ const MenuCheckboxItem = React.forwardRef(function MenuCheckboxItem( const listItem = useCompositeListItem({ label }); const mergedRef = useForkRef(forwardedRef, listItem.ref, itemRef); - const { getItemProps, activeIndex, allowMouseUpTriggerRef, typingRef } = useMenuRootContext(); + const { itemProps, activeIndex, allowMouseUpTriggerRef, typingRef } = useMenuRootContext(); const id = useBaseUiId(idProp); const highlighted = listItem.index === activeIndex; @@ -192,7 +193,7 @@ const MenuCheckboxItem = React.forwardRef(function MenuCheckboxItem( ref={mergedRef} highlighted={highlighted} menuEvents={menuEvents} - propGetter={getItemProps} + itemProps={itemProps} allowMouseUpTriggerRef={allowMouseUpTriggerRef} typingRef={typingRef} closeOnClick={closeOnClick} @@ -202,7 +203,7 @@ const MenuCheckboxItem = React.forwardRef(function MenuCheckboxItem( interface InnerMenuCheckboxItemProps extends MenuCheckboxItem.Props { highlighted: boolean; - propGetter: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + itemProps: GenericHTMLProps; menuEvents: FloatingEvents; allowMouseUpTriggerRef: React.RefObject; typingRef: React.RefObject; diff --git a/packages/react/src/menu/checkbox-item/useMenuCheckboxItem.ts b/packages/react/src/menu/checkbox-item/useMenuCheckboxItem.ts index 34abf43bf9..c6065c080d 100644 --- a/packages/react/src/menu/checkbox-item/useMenuCheckboxItem.ts +++ b/packages/react/src/menu/checkbox-item/useMenuCheckboxItem.ts @@ -16,31 +16,29 @@ export function useMenuCheckboxItem( state: 'checked', }); - const { getRootProps: getMenuItemRootProps, ...menuItem } = useMenuItem(other); + const { getItemProps: getMenuItemProps, ...menuItem } = useMenuItem(other); - const getRootProps = React.useCallback( + const getItemProps = React.useCallback( (externalProps?: GenericHTMLProps): GenericHTMLProps => { - return getMenuItemRootProps( - mergeReactProps(externalProps, { - role: 'menuitemcheckbox', - 'aria-checked': checked, - onClick: (event: React.MouseEvent) => { - setChecked((currentlyChecked) => !currentlyChecked); - onCheckedChange?.(!checked, event.nativeEvent); - }, - }), - ); + return mergeReactProps(getMenuItemProps, externalProps, { + role: 'menuitemcheckbox', + 'aria-checked': checked, + onClick: (event: React.MouseEvent) => { + setChecked((currentlyChecked) => !currentlyChecked); + onCheckedChange?.(!checked, event.nativeEvent); + }, + }); }, - [checked, getMenuItemRootProps, onCheckedChange, setChecked], + [checked, getMenuItemProps, onCheckedChange, setChecked], ); return React.useMemo( () => ({ ...menuItem, - getRootProps, + getItemProps, checked, }), - [checked, getRootProps, menuItem], + [checked, getItemProps, menuItem], ); } diff --git a/packages/react/src/menu/item/MenuItem.test.tsx b/packages/react/src/menu/item/MenuItem.test.tsx index 75b8f0ffa2..36fe0c05f6 100644 --- a/packages/react/src/menu/item/MenuItem.test.tsx +++ b/packages/react/src/menu/item/MenuItem.test.tsx @@ -268,16 +268,16 @@ describe('', () => { expect(item).toHaveFocus(); fireEvent.keyDown(item, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); fireEvent.keyUp(item, { key: 'Space' }); - expect(handleKeyUp.callCount).to.equal(1); + expect(handleKeyUp.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); fireEvent.click(item); - expect(handleKeyDown.callCount).to.equal(1); - expect(handleKeyUp.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); + expect(handleKeyUp.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); }); }); diff --git a/packages/react/src/menu/item/MenuItem.tsx b/packages/react/src/menu/item/MenuItem.tsx index 001c9994e4..3f3da35737 100644 --- a/packages/react/src/menu/item/MenuItem.tsx +++ b/packages/react/src/menu/item/MenuItem.tsx @@ -9,6 +9,7 @@ import { useBaseUiId } from '../../utils/useBaseUiId'; import type { BaseUIComponentProps, GenericHTMLProps } from '../../utils/types'; import { useForkRef } from '../../utils/useForkRef'; import { useCompositeListItem } from '../../composite/list/useCompositeListItem'; +import { mergeReactProps } from '../../utils/mergeReactProps'; const InnerMenuItem = React.forwardRef(function InnerMenuItem( props: InnerMenuItemProps, @@ -21,14 +22,14 @@ const InnerMenuItem = React.forwardRef(function InnerMenuItem( highlighted, id, menuEvents, - propGetter, + itemProps, render, allowMouseUpTriggerRef, typingRef, ...other } = props; - const { getRootProps } = useMenuItem({ + const { getItemProps } = useMenuItem({ closeOnClick, disabled, highlighted, @@ -48,7 +49,7 @@ const InnerMenuItem = React.forwardRef(function InnerMenuItem( render: render || 'div', className, state, - propGetter: (externalProps) => propGetter(getRootProps(externalProps)), + propGetter: (externalProps) => mergeReactProps(getItemProps, externalProps, itemProps), extraProps: other, }); @@ -103,6 +104,10 @@ InnerMenuItem.propTypes /* remove-proptypes */ = { * @ignore */ id: PropTypes.string, + /** + * @ignore + */ + itemProps: PropTypes.object.isRequired, /** * Overrides the text label to use when the item is matched during keyboard text navigation. */ @@ -119,10 +124,6 @@ InnerMenuItem.propTypes /* remove-proptypes */ = { * The click handler for the menu item. */ onClick: PropTypes.func, - /** - * @ignore - */ - propGetter: PropTypes.func.isRequired, /** * Allows you to replace the component’s HTML element * with a different tag, or compose it with another component. @@ -154,7 +155,7 @@ const MenuItem = React.forwardRef(function MenuItem( const listItem = useCompositeListItem({ label }); const mergedRef = useForkRef(forwardedRef, listItem.ref, itemRef); - const { getItemProps, activeIndex, allowMouseUpTriggerRef, typingRef } = useMenuRootContext(); + const { itemProps, activeIndex, allowMouseUpTriggerRef, typingRef } = useMenuRootContext(); const id = useBaseUiId(idProp); const highlighted = listItem.index === activeIndex; @@ -171,7 +172,7 @@ const MenuItem = React.forwardRef(function MenuItem( ref={mergedRef} highlighted={highlighted} menuEvents={menuEvents} - propGetter={getItemProps} + itemProps={itemProps} allowMouseUpTriggerRef={allowMouseUpTriggerRef} typingRef={typingRef} /> @@ -180,7 +181,7 @@ const MenuItem = React.forwardRef(function MenuItem( interface InnerMenuItemProps extends MenuItem.Props { highlighted: boolean; - propGetter: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + itemProps: GenericHTMLProps; menuEvents: FloatingEvents; allowMouseUpTriggerRef: React.RefObject; typingRef: React.RefObject; diff --git a/packages/react/src/menu/item/useMenuItem.ts b/packages/react/src/menu/item/useMenuItem.ts index 43ea05b2f2..c9b5d2b430 100644 --- a/packages/react/src/menu/item/useMenuItem.ts +++ b/packages/react/src/menu/item/useMenuItem.ts @@ -26,43 +26,41 @@ export function useMenuItem(params: useMenuItem.Parameters): useMenuItem.ReturnV buttonRef: useForkRef(externalRef, itemRef), }); - const getRootProps = React.useCallback( + const getItemProps = React.useCallback( (externalProps?: GenericHTMLProps): GenericHTMLProps => { - return getButtonProps( - mergeReactProps(externalProps, { - id, - role: 'menuitem', - tabIndex: highlighted ? 0 : -1, - onKeyUp: (event: BaseUIEvent) => { - if (event.key === ' ' && typingRef.current) { - event.preventBaseUIHandler(); - } - }, - onClick: (event: React.MouseEvent | React.KeyboardEvent) => { - if (closeOnClick) { - menuEvents.emit('close', event); - } - }, - onMouseUp: (event: React.MouseEvent) => { - if (itemRef.current && allowMouseUpTriggerRef.current) { - // This fires whenever the user clicks on the trigger, moves the cursor, and releases it over the item. - // We trigger the click and override the `closeOnClick` preference to always close the menu. - itemRef.current.click(); - menuEvents.emit('close', event); - } - }, - }), - ); + return mergeReactProps(getButtonProps, externalProps, { + id, + role: 'menuitem', + tabIndex: highlighted ? 0 : -1, + onKeyUp: (event: BaseUIEvent) => { + if (event.key === ' ' && typingRef.current) { + event.preventBaseUIHandler(); + } + }, + onClick: (event: React.MouseEvent | React.KeyboardEvent) => { + if (closeOnClick) { + menuEvents.emit('close', event); + } + }, + onMouseUp: (event: React.MouseEvent) => { + if (itemRef.current && allowMouseUpTriggerRef.current) { + // This fires whenever the user clicks on the trigger, moves the cursor, and releases it over the item. + // We trigger the click and override the `closeOnClick` preference to always close the menu. + itemRef.current.click(); + menuEvents.emit('close', event); + } + }, + }); }, [getButtonProps, id, highlighted, typingRef, closeOnClick, menuEvents, allowMouseUpTriggerRef], ); return React.useMemo( () => ({ - getRootProps, + getItemProps, rootRef: mergedRef, }), - [getRootProps, mergedRef], + [getItemProps, mergedRef], ); } @@ -108,7 +106,7 @@ export namespace useMenuItem { * @param externalProps event handlers for the root slot * @returns props that should be spread on the root slot */ - getRootProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + getItemProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; /** * The ref to the component's root DOM element. */ diff --git a/packages/react/src/menu/popup/MenuPopup.tsx b/packages/react/src/menu/popup/MenuPopup.tsx index e27f342575..1dd34915e8 100644 --- a/packages/react/src/menu/popup/MenuPopup.tsx +++ b/packages/react/src/menu/popup/MenuPopup.tsx @@ -21,6 +21,9 @@ const customStyleHookMapping: CustomStyleHookMapping = { ...transitionStatusMapping, }; +const DISABLED_TRANSITIONS_STYLE = { style: { transition: 'none' } }; +const EMPTY_OBJ = {}; + /** * A container for the menu items. * Renders a `
` element. @@ -39,7 +42,7 @@ const MenuPopup = React.forwardRef(function MenuPopup( popupRef, transitionStatus, nested, - getPopupProps, + popupProps, modal, mounted, instantType, @@ -79,16 +82,14 @@ const MenuPopup = React.forwardRef(function MenuPopup( ); const { renderElement } = useComponentRenderer({ - propGetter: getPopupProps, render: render || 'div', className, state, - extraProps: - transitionStatus === 'starting' - ? mergeReactProps(other, { - style: { transition: 'none' }, - }) - : other, + extraProps: mergeReactProps( + other, + popupProps, + transitionStatus === 'starting' ? DISABLED_TRANSITIONS_STYLE : EMPTY_OBJ, + ), customStyleHookMapping, ref: mergedRef, }); diff --git a/packages/react/src/menu/positioner/MenuPositioner.tsx b/packages/react/src/menu/positioner/MenuPositioner.tsx index f014621915..b62639633c 100644 --- a/packages/react/src/menu/positioner/MenuPositioner.tsx +++ b/packages/react/src/menu/positioner/MenuPositioner.tsx @@ -119,13 +119,15 @@ const MenuPositioner = React.forwardRef(function MenuPositioner( const mergedRef = useForkRef(forwardedRef, setPositionerElement); const { renderElement } = useComponentRenderer({ - propGetter: positioner.getPositionerProps, render: render ?? 'div', className, state, customStyleHookMapping: popupStateMapping, ref: mergedRef, - extraProps: otherProps, + extraProps: { + ...positioner.positionerProps, + ...otherProps, + }, }); return ( @@ -140,7 +142,7 @@ const MenuPositioner = React.forwardRef(function MenuPositioner( ); }); -export namespace MenuPositioner { +namespace MenuPositioner { export interface State { /** * Whether the menu is currently open. diff --git a/packages/react/src/menu/positioner/useMenuPositioner.ts b/packages/react/src/menu/positioner/useMenuPositioner.ts index e440a77f3a..7a46ebb9b0 100644 --- a/packages/react/src/menu/positioner/useMenuPositioner.ts +++ b/packages/react/src/menu/positioner/useMenuPositioner.ts @@ -1,7 +1,6 @@ 'use client'; import * as React from 'react'; import { useFloatingTree, type FloatingRootContext } from '@floating-ui/react'; -import { mergeReactProps } from '../../utils/mergeReactProps'; import { useAnchorPositioning } from '../../utils/useAnchorPositioning'; import type { GenericHTMLProps } from '../../utils/types'; import { useMenuRootContext } from '../root/MenuRootContext'; @@ -17,25 +16,22 @@ export function useMenuPositioner( const { events: menuEvents } = useFloatingTree()!; - const getPositionerProps: useMenuPositioner.ReturnValue['getPositionerProps'] = React.useCallback( - (externalProps = {}) => { - const hiddenStyles: React.CSSProperties = {}; + const positionerProps = React.useMemo(() => { + const hiddenStyles: React.CSSProperties = {}; - if (!open) { - hiddenStyles.pointerEvents = 'none'; - } + if (!open) { + hiddenStyles.pointerEvents = 'none'; + } - return mergeReactProps<'div'>(externalProps, { - role: 'presentation', - hidden: !mounted, - style: { - ...positioning.positionerStyles, - ...hiddenStyles, - }, - }); - }, - [open, mounted, positioning.positionerStyles], - ); + return { + role: 'presentation', + hidden: !mounted, + style: { + ...positioning.positionerStyles, + ...hiddenStyles, + }, + }; + }, [open, mounted, positioning.positionerStyles]); React.useEffect(() => { function onMenuOpenChange(event: { open: boolean; nodeId: string; parentNodeId: string }) { @@ -65,9 +61,9 @@ export function useMenuPositioner( return React.useMemo( () => ({ ...positioning, - getPositionerProps, + positionerProps, }), - [positioning, getPositionerProps], + [positioning, positionerProps], ); } @@ -90,6 +86,6 @@ export namespace useMenuPositioner { export interface SharedParameters extends useAnchorPositioning.SharedParameters {} export interface ReturnValue extends useAnchorPositioning.ReturnValue { - getPositionerProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + positionerProps: GenericHTMLProps; } } diff --git a/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.tsx b/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.tsx index d9a5967775..4cde3ad3e0 100644 --- a/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.tsx +++ b/packages/react/src/menu/radio-item-indicator/MenuRadioItemIndicator.tsx @@ -5,7 +5,6 @@ import { useMenuRadioItemContext } from '../radio-item/MenuRadioItemContext'; import { useComponentRenderer } from '../../utils/useComponentRenderer'; import { BaseUIComponentProps } from '../../utils/types'; import { itemMapping } from '../utils/styleHookMapping'; -import { mergeReactProps } from '../../utils/mergeReactProps'; import { TransitionStatus, useTransitionStatus } from '../../utils/useTransitionStatus'; import { useOpenChangeComplete } from '../../utils/useOpenChangeComplete'; import { useForkRef } from '../../utils/useForkRef'; @@ -29,14 +28,6 @@ const MenuRadioItemIndicator = React.forwardRef(function MenuRadioItemIndicator( const { transitionStatus, setMounted } = useTransitionStatus(item.checked); - const getItemProps = React.useCallback( - (externalProps = {}) => - mergeReactProps(externalProps, { - 'aria-hidden': true, - }), - [], - ); - useOpenChangeComplete({ open: item.checked, ref: indicatorRef, @@ -58,12 +49,14 @@ const MenuRadioItemIndicator = React.forwardRef(function MenuRadioItemIndicator( ); const { renderElement } = useComponentRenderer({ - propGetter: getItemProps, render: render || 'span', className, state, customStyleHookMapping: itemMapping, - extraProps: other, + extraProps: { + 'aria-hidden': true, + ...other, + }, ref: mergedRef, }); diff --git a/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx b/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx index 179ea23ae1..3e791923bc 100644 --- a/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx +++ b/packages/react/src/menu/radio-item/MenuRadioItem.test.tsx @@ -346,12 +346,12 @@ describe('', () => { expect(item1).toHaveFocus(); fireEvent.keyDown(item1, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleValueChange.callCount).to.equal(0); fireEvent.keyUp(item1, { key: 'Space' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleValueChange.callCount).to.equal(0); @@ -360,16 +360,16 @@ describe('', () => { expect(handleValueChange.callCount).to.equal(0); fireEvent.keyDown(item1, { key: 'ArrowDown' }); - expect(handleKeyDown.callCount).to.equal(2); + expect(handleKeyDown.callCount).to.equal(0); expect(item2).toHaveFocus(); fireEvent.keyDown(item2, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(3); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleValueChange.callCount).to.equal(0); fireEvent.keyUp(item2, { key: 'Space' }); - expect(handleKeyDown.callCount).to.equal(3); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleValueChange.callCount).to.equal(0); @@ -424,12 +424,12 @@ describe('', () => { expect(item1).toHaveFocus(); fireEvent.keyDown(item1, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleValueChange.callCount).to.equal(0); fireEvent.keyUp(item1, { key: 'Space' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); expect(handleValueChange.callCount).to.equal(0); @@ -438,11 +438,11 @@ describe('', () => { expect(handleValueChange.callCount).to.equal(0); fireEvent.keyDown(item1, { key: 'ArrowDown' }); - expect(handleKeyDown.callCount).to.equal(2); + expect(handleKeyDown.callCount).to.equal(0); expect(item2).toHaveFocus(); fireEvent.keyDown(item2, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(3); + expect(handleKeyDown.callCount).to.equal(1); expect(handleClick.callCount).to.equal(1); expect(handleValueChange.callCount).to.equal(1); expect(handleValueChange.args[0][0]).to.equal('two'); diff --git a/packages/react/src/menu/radio-item/MenuRadioItem.tsx b/packages/react/src/menu/radio-item/MenuRadioItem.tsx index fbdfe80221..2e5e7bdc46 100644 --- a/packages/react/src/menu/radio-item/MenuRadioItem.tsx +++ b/packages/react/src/menu/radio-item/MenuRadioItem.tsx @@ -12,6 +12,7 @@ import { useMenuRadioGroupContext } from '../radio-group/MenuRadioGroupContext'; import { MenuRadioItemContext } from './MenuRadioItemContext'; import { itemMapping } from '../utils/styleHookMapping'; import { useCompositeListItem } from '../../composite/list/useCompositeListItem'; +import { mergeReactProps } from '../../utils/mergeReactProps'; const InnerMenuRadioItem = React.forwardRef(function InnerMenuItem( props: InnerMenuRadioItemProps, @@ -26,14 +27,14 @@ const InnerMenuRadioItem = React.forwardRef(function InnerMenuItem( highlighted, id, menuEvents, - propGetter, + itemProps, render, allowMouseUpTriggerRef, typingRef, ...other } = props; - const { getRootProps } = useMenuRadioItem({ + const { getItemProps } = useMenuRadioItem({ checked, setChecked, closeOnClick, @@ -52,7 +53,7 @@ const InnerMenuRadioItem = React.forwardRef(function InnerMenuItem( render: render || 'div', className, state, - propGetter: (externalProps) => propGetter(getRootProps(externalProps)), + propGetter: (externalProps) => mergeReactProps(getItemProps, externalProps, itemProps), customStyleHookMapping: itemMapping, extraProps: other, }); @@ -108,6 +109,10 @@ InnerMenuRadioItem.propTypes /* remove-proptypes */ = { * @ignore */ id: PropTypes.string, + /** + * @ignore + */ + itemProps: PropTypes.object.isRequired, /** * Overrides the text label to use when the item is matched during keyboard text navigation. */ @@ -124,10 +129,6 @@ InnerMenuRadioItem.propTypes /* remove-proptypes */ = { * The click handler for the menu item. */ onClick: PropTypes.func, - /** - * @ignore - */ - propGetter: PropTypes.func.isRequired, /** * Allows you to replace the component’s HTML element * with a different tag, or compose it with another component. @@ -172,7 +173,7 @@ const MenuRadioItem = React.forwardRef(function MenuRadioItem( const listItem = useCompositeListItem({ label }); const mergedRef = useForkRef(forwardedRef, listItem.ref, itemRef); - const { getItemProps, activeIndex, allowMouseUpTriggerRef, typingRef } = useMenuRootContext(); + const { itemProps, activeIndex, allowMouseUpTriggerRef, typingRef } = useMenuRootContext(); const id = useBaseUiId(idProp); const highlighted = listItem.index === activeIndex; @@ -213,7 +214,7 @@ const MenuRadioItem = React.forwardRef(function MenuRadioItem( disabled={disabled} highlighted={highlighted} menuEvents={menuEvents} - propGetter={getItemProps} + itemProps={itemProps} allowMouseUpTriggerRef={allowMouseUpTriggerRef} checked={selectedValue === value} setChecked={setChecked} @@ -226,7 +227,7 @@ const MenuRadioItem = React.forwardRef(function MenuRadioItem( interface InnerMenuRadioItemProps extends Omit { highlighted: boolean; - propGetter: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + itemProps: GenericHTMLProps; menuEvents: FloatingEvents; allowMouseUpTriggerRef: React.RefObject; checked: boolean; diff --git a/packages/react/src/menu/radio-item/useMenuRadioItem.ts b/packages/react/src/menu/radio-item/useMenuRadioItem.ts index 4f9b28c8dd..3bc83db28e 100644 --- a/packages/react/src/menu/radio-item/useMenuRadioItem.ts +++ b/packages/react/src/menu/radio-item/useMenuRadioItem.ts @@ -8,26 +8,24 @@ export function useMenuRadioItem( ): useMenuRadioItem.ReturnValue { const { checked, setChecked, ...other } = params; - const { getRootProps: getMenuItemRootProps, ...menuItem } = useMenuItem(other); + const { getItemProps: getMenuItemProps, ...menuItem } = useMenuItem(other); - const getRootProps = React.useCallback( + const getItemProps = React.useCallback( (externalProps?: GenericHTMLProps): GenericHTMLProps => { - return getMenuItemRootProps( - mergeReactProps(externalProps, { - role: 'menuitemradio', - 'aria-checked': checked, - onClick: (event: React.MouseEvent) => { - setChecked(event.nativeEvent); - }, - }), - ); + return mergeReactProps(getMenuItemProps, externalProps, { + role: 'menuitemradio', + 'aria-checked': checked, + onClick: (event: React.MouseEvent) => { + setChecked(event.nativeEvent); + }, + }); }, - [checked, getMenuItemRootProps, setChecked], + [checked, getMenuItemProps, setChecked], ); return { ...menuItem, - getRootProps, + getItemProps, checked, }; } diff --git a/packages/react/src/menu/root/useMenuRoot.ts b/packages/react/src/menu/root/useMenuRoot.ts index ec226e9ebe..27dfaa70a2 100644 --- a/packages/react/src/menu/root/useMenuRoot.ts +++ b/packages/react/src/menu/root/useMenuRoot.ts @@ -13,7 +13,6 @@ import { useTypeahead, type FloatingRootContext, } from '@floating-ui/react'; -import { mergeReactProps } from '../../utils/mergeReactProps'; import { GenericHTMLProps } from '../../utils/types'; import { useTransitionStatus, type TransitionStatus } from '../../utils/useTransitionStatus'; import { useEventCallback } from '../../utils/useEventCallback'; @@ -228,45 +227,43 @@ export function useMenuRoot(parameters: useMenuRoot.Parameters): useMenuRoot.Ret typeahead, ]); - const getTriggerProps = React.useCallback( - (externalProps?: GenericHTMLProps) => - getReferenceProps( - mergeReactProps(externalProps, { - onMouseEnter() { - setHoverEnabled(true); - }, - }), - ), + const triggerProps = React.useMemo( + () => + getReferenceProps({ + onMouseEnter() { + setHoverEnabled(true); + }, + }), [getReferenceProps], ); - const getPopupProps = React.useCallback( - (externalProps?: GenericHTMLProps) => - getFloatingProps( - mergeReactProps(externalProps, { - onMouseEnter() { - if (!openOnHover || nested) { - setHoverEnabled(false); - } - }, - onClick() { - if (openOnHover) { - setHoverEnabled(false); - } - }, - }), - ), + const popupProps = React.useMemo( + () => + getFloatingProps({ + onMouseEnter() { + if (!openOnHover || nested) { + setHoverEnabled(false); + } + }, + onClick() { + if (openOnHover) { + setHoverEnabled(false); + } + }, + }), [getFloatingProps, openOnHover, nested], ); + const itemProps = React.useMemo(() => getItemProps(), [getItemProps]); + return React.useMemo( () => ({ activeIndex, allowMouseUpTriggerRef, floatingRootContext, - getItemProps, - getPopupProps, - getTriggerProps, + itemProps, + popupProps, + triggerProps, itemDomElements, itemLabels, mounted, @@ -285,9 +282,9 @@ export function useMenuRoot(parameters: useMenuRoot.Parameters): useMenuRoot.Ret [ activeIndex, floatingRootContext, - getItemProps, - getPopupProps, - getTriggerProps, + itemProps, + popupProps, + triggerProps, itemDomElements, itemLabels, mounted, @@ -376,9 +373,9 @@ export namespace useMenuRoot { export interface ReturnValue { activeIndex: number | null; floatingRootContext: FloatingRootContext; - getItemProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; - getPopupProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; - getTriggerProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + itemProps: GenericHTMLProps; + popupProps: GenericHTMLProps; + triggerProps: GenericHTMLProps; itemDomElements: React.MutableRefObject<(HTMLElement | null)[]>; itemLabels: React.MutableRefObject<(string | null)[]>; mounted: boolean; diff --git a/packages/react/src/menu/submenu-trigger/MenuSubmenuTrigger.tsx b/packages/react/src/menu/submenu-trigger/MenuSubmenuTrigger.tsx index 8eabcd8538..4556741737 100644 --- a/packages/react/src/menu/submenu-trigger/MenuSubmenuTrigger.tsx +++ b/packages/react/src/menu/submenu-trigger/MenuSubmenuTrigger.tsx @@ -10,6 +10,7 @@ import { useMenuSubmenuTrigger } from './useMenuSubmenuTrigger'; import { useForkRef } from '../../utils/useForkRef'; import { triggerOpenStateMapping } from '../../utils/popupStateMapping'; import { useCompositeListItem } from '../../composite/list/useCompositeListItem'; +import { mergeReactProps } from '../../utils/mergeReactProps'; /** * A menu item that opens a submenu. @@ -21,23 +22,24 @@ const MenuSubmenuTrigger = React.forwardRef(function SubmenuTriggerComponent( props: MenuSubmenuTrigger.Props, forwardedRef: React.ForwardedRef, ) { - const { render, className, disabled = false, label, id: idProp, ...other } = props; + const { render, className, label, id: idProp, ...other } = props; const id = useBaseUiId(idProp); const { - getTriggerProps, + triggerProps: rootTriggerProps, parentContext, setTriggerElement, allowMouseUpTriggerRef, open, typingRef, + disabled, } = useMenuRootContext(); if (parentContext === undefined) { throw new Error('Base UI: ItemTrigger must be placed in a nested Menu.'); } - const { activeIndex, getItemProps } = parentContext; + const { activeIndex, itemProps } = parentContext; const item = useCompositeListItem(); const highlighted = activeIndex === item.index; @@ -46,7 +48,7 @@ const MenuSubmenuTrigger = React.forwardRef(function SubmenuTriggerComponent( const { events: menuEvents } = useFloatingTree()!; - const { getRootProps } = useMenuSubmenuTrigger({ + const { getTriggerProps } = useMenuSubmenuTrigger({ id, highlighted, ref: mergedRef, @@ -67,7 +69,7 @@ const MenuSubmenuTrigger = React.forwardRef(function SubmenuTriggerComponent( className, state, propGetter: (externalProps: GenericHTMLProps) => - getTriggerProps(getItemProps(getRootProps(externalProps))), + mergeReactProps(getTriggerProps, externalProps, itemProps, rootTriggerProps), customStyleHookMapping: triggerOpenStateMapping, extraProps: other, }); @@ -79,11 +81,6 @@ namespace MenuSubmenuTrigger { export interface Props extends BaseUIComponentProps<'div', State> { children?: React.ReactNode; onClick?: React.MouseEventHandler; - /** - * Whether the component should ignore user interaction. - * @default false - */ - disabled?: boolean; /** * Overrides the text label to use when the item is matched during keyboard text navigation. */ @@ -121,11 +118,6 @@ MenuSubmenuTrigger.propTypes /* remove-proptypes */ = { * returns a class based on the component’s state. */ className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), - /** - * Whether the component should ignore user interaction. - * @default false - */ - disabled: PropTypes.bool, /** * @ignore */ diff --git a/packages/react/src/menu/submenu-trigger/useMenuSubmenuTrigger.ts b/packages/react/src/menu/submenu-trigger/useMenuSubmenuTrigger.ts index 3935162902..912d728121 100644 --- a/packages/react/src/menu/submenu-trigger/useMenuSubmenuTrigger.ts +++ b/packages/react/src/menu/submenu-trigger/useMenuSubmenuTrigger.ts @@ -6,8 +6,8 @@ import { useForkRef } from '../../utils/useForkRef'; import { GenericHTMLProps } from '../../utils/types'; export function useMenuSubmenuTrigger( - parameters: useSubmenuTrigger.Parameters, -): useSubmenuTrigger.ReturnValue { + parameters: useMenuSubmenuTrigger.Parameters, +): useMenuSubmenuTrigger.ReturnValue { const { id, highlighted, @@ -19,7 +19,7 @@ export function useMenuSubmenuTrigger( typingRef, } = parameters; - const { getRootProps: getMenuItemProps, rootRef: menuItemRef } = useMenuItem({ + const { getItemProps, rootRef: menuItemRef } = useMenuItem({ closeOnClick: false, disabled, highlighted, @@ -32,29 +32,27 @@ export function useMenuSubmenuTrigger( const menuTriggerRef = useForkRef(menuItemRef, setTriggerElement); - const getRootProps = React.useCallback( + const getTriggerProps = React.useCallback( (externalProps?: GenericHTMLProps) => { return { - ...getMenuItemProps({ - 'aria-haspopup': 'menu' as const, - ...externalProps, - }), + ...getItemProps(externalProps), + 'aria-haspopup': 'menu' as const, ref: menuTriggerRef, }; }, - [getMenuItemProps, menuTriggerRef], + [getItemProps, menuTriggerRef], ); return React.useMemo( () => ({ - getRootProps, + getTriggerProps, rootRef: menuTriggerRef, }), - [getRootProps, menuTriggerRef], + [getTriggerProps, menuTriggerRef], ); } -export namespace useSubmenuTrigger { +export namespace useMenuSubmenuTrigger { export interface Parameters { id: string | undefined; highlighted: boolean; @@ -85,6 +83,6 @@ export namespace useSubmenuTrigger { } export interface ReturnValue { - getRootProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; + getTriggerProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps; } } diff --git a/packages/react/src/menu/trigger/MenuTrigger.test.tsx b/packages/react/src/menu/trigger/MenuTrigger.test.tsx index 68040f68b7..558d0e0812 100644 --- a/packages/react/src/menu/trigger/MenuTrigger.test.tsx +++ b/packages/react/src/menu/trigger/MenuTrigger.test.tsx @@ -245,4 +245,46 @@ describe('', () => { expect(trigger).not.to.have.attribute('data-popup-open'); }); }); + + describe('preventBaseUIHandler', () => { + it('prevents opening the menu with a mouse when `preventBaseUIHandler` is called in onMouseDown', async () => { + const { getByRole, queryByRole } = await render( + + event.preventBaseUIHandler()} /> + + + + + + , + ); + + const button = getByRole('button'); + await user.click(button); + + expect(queryByRole('menu', { hidden: false })).to.equal(null); + }); + + it('prevents opening the menu with keyboard when `preventBaseUIHandler` is called in onClick', async () => { + const { getByRole, queryByRole } = await render( + + event.preventBaseUIHandler()} /> + + + + + + , + ); + + const button = getByRole('button'); + await act(async () => { + button.focus(); + }); + + await user.keyboard('[Enter]'); + + expect(queryByRole('menu', { hidden: false })).to.equal(null); + }); + }); }); diff --git a/packages/react/src/menu/trigger/MenuTrigger.tsx b/packages/react/src/menu/trigger/MenuTrigger.tsx index e69461e361..6eba1f3f31 100644 --- a/packages/react/src/menu/trigger/MenuTrigger.tsx +++ b/packages/react/src/menu/trigger/MenuTrigger.tsx @@ -5,7 +5,8 @@ import { useMenuTrigger } from './useMenuTrigger'; import { useMenuRootContext } from '../root/MenuRootContext'; import { pressableTriggerOpenStateMapping } from '../../utils/popupStateMapping'; import { useComponentRenderer } from '../../utils/useComponentRenderer'; -import { BaseUIComponentProps } from '../../utils/types'; +import { BaseUIComponentProps, GenericHTMLProps } from '../../utils/types'; +import { mergeReactProps } from '../../utils/mergeReactProps'; /** * A button that opens the menu. @@ -20,7 +21,7 @@ const MenuTrigger = React.forwardRef(function MenuTrigger( const { render, className, disabled = false, ...other } = props; const { - getTriggerProps: getRootTriggerProps, + triggerProps: rootTriggerProps, disabled: menuDisabled, setTriggerElement, open, @@ -41,11 +42,17 @@ const MenuTrigger = React.forwardRef(function MenuTrigger( const state: MenuTrigger.State = React.useMemo(() => ({ open }), [open]); + const propGetter = React.useCallback( + (externalProps: GenericHTMLProps) => + mergeReactProps(getTriggerProps, externalProps, rootTriggerProps), + [getTriggerProps, rootTriggerProps], + ); + const { renderElement } = useComponentRenderer({ render: render || 'button', className, state, - propGetter: (externalProps) => getRootTriggerProps(getTriggerProps(externalProps)), + propGetter, customStyleHookMapping: pressableTriggerOpenStateMapping, extraProps: other, }); diff --git a/packages/react/src/menu/trigger/useMenuTrigger.ts b/packages/react/src/menu/trigger/useMenuTrigger.ts index b9576884ba..aedd4b83ed 100644 --- a/packages/react/src/menu/trigger/useMenuTrigger.ts +++ b/packages/react/src/menu/trigger/useMenuTrigger.ts @@ -40,62 +40,60 @@ export function useMenuTrigger(parameters: useMenuTrigger.Parameters): useMenuTr const getTriggerProps = React.useCallback( (externalProps?: GenericHTMLProps): GenericHTMLProps => { - return getButtonProps( - mergeReactProps(externalProps, { - 'aria-haspopup': 'menu' as const, - tabIndex: 0, // this is needed to make the button focused after click in Safari - ref: handleRef, - onMouseDown: (event: React.MouseEvent) => { - if (open) { + return mergeReactProps(getButtonProps, externalProps, { + 'aria-haspopup': 'menu' as const, + tabIndex: 0, // this is needed to make the button focused after click in Safari + ref: handleRef, + onMouseDown: (event: React.MouseEvent) => { + if (open) { + return; + } + + // mousedown -> mouseup on menu item should not trigger it within 200ms. + allowMouseUpTriggerTimeoutRef.current = window.setTimeout(() => { + allowMouseUpTriggerRef.current = true; + }, 200); + + const doc = ownerDocument(event.currentTarget); + + function handleMouseUp(mouseEvent: MouseEvent) { + if (!triggerRef.current) { return; } - // mousedown -> mouseup on menu item should not trigger it within 200ms. - allowMouseUpTriggerTimeoutRef.current = window.setTimeout(() => { - allowMouseUpTriggerRef.current = true; - }, 200); - - const doc = ownerDocument(event.currentTarget); - - function handleMouseUp(mouseEvent: MouseEvent) { - if (!triggerRef.current) { - return; - } - - if (allowMouseUpTriggerTimeoutRef.current !== -1) { - clearTimeout(allowMouseUpTriggerTimeoutRef.current); - allowMouseUpTriggerTimeoutRef.current = -1; - } - allowMouseUpTriggerRef.current = false; - - const mouseUpTarget = mouseEvent.target as Element | null; - - if ( - contains(triggerRef.current, mouseUpTarget) || - contains(positionerRef.current, mouseUpTarget) || - mouseUpTarget === triggerRef.current - ) { - return; - } - - const bounds = getPseudoElementBounds(triggerRef.current); - - if ( - mouseEvent.clientX >= bounds.left - BOUNDARY_OFFSET && - mouseEvent.clientX <= bounds.right + BOUNDARY_OFFSET && - mouseEvent.clientY >= bounds.top - BOUNDARY_OFFSET && - mouseEvent.clientY <= bounds.bottom + BOUNDARY_OFFSET - ) { - return; - } - - setOpen(false, mouseEvent); + if (allowMouseUpTriggerTimeoutRef.current !== -1) { + clearTimeout(allowMouseUpTriggerTimeoutRef.current); + allowMouseUpTriggerTimeoutRef.current = -1; } + allowMouseUpTriggerRef.current = false; - doc.addEventListener('mouseup', handleMouseUp, { once: true }); - }, - }), - ); + const mouseUpTarget = mouseEvent.target as Element | null; + + if ( + contains(triggerRef.current, mouseUpTarget) || + contains(positionerRef.current, mouseUpTarget) || + mouseUpTarget === triggerRef.current + ) { + return; + } + + const bounds = getPseudoElementBounds(triggerRef.current); + + if ( + mouseEvent.clientX >= bounds.left - BOUNDARY_OFFSET && + mouseEvent.clientX <= bounds.right + BOUNDARY_OFFSET && + mouseEvent.clientY >= bounds.top - BOUNDARY_OFFSET && + mouseEvent.clientY <= bounds.bottom + BOUNDARY_OFFSET + ) { + return; + } + + setOpen(false, mouseEvent); + } + + doc.addEventListener('mouseup', handleMouseUp, { once: true }); + }, + }); }, [getButtonProps, handleRef, open, setOpen, positionerRef, allowMouseUpTriggerRef], ); diff --git a/packages/react/src/use-button/useButton.test.tsx b/packages/react/src/use-button/useButton.test.tsx index aede0952bc..7a86631574 100644 --- a/packages/react/src/use-button/useButton.test.tsx +++ b/packages/react/src/use-button/useButton.test.tsx @@ -48,16 +48,16 @@ describe('useButton', () => { expect(button).toHaveFocus(); fireEvent.keyDown(button, { key: 'Enter' }); - expect(handleKeyDown.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); fireEvent.keyUp(button, { key: 'Space' }); - expect(handleKeyUp.callCount).to.equal(1); + expect(handleKeyUp.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); fireEvent.click(button); - expect(handleKeyDown.callCount).to.equal(1); - expect(handleKeyUp.callCount).to.equal(1); + expect(handleKeyDown.callCount).to.equal(0); + expect(handleKeyUp.callCount).to.equal(0); expect(handleClick.callCount).to.equal(0); }); }); diff --git a/packages/react/src/use-button/useButton.ts b/packages/react/src/use-button/useButton.ts index c2fa8bd820..ba443905c6 100644 --- a/packages/react/src/use-button/useButton.ts +++ b/packages/react/src/use-button/useButton.ts @@ -1,11 +1,10 @@ 'use client'; import * as React from 'react'; -import { NOOP } from '../utils/noop'; import { useForkRef } from '../utils/useForkRef'; -import { mergeReactProps } from '../utils/mergeReactProps'; +import { makeEventPreventable, mergeReactProps } from '../utils/mergeReactProps'; import { useEventCallback } from '../utils/useEventCallback'; import { useRootElementName } from '../utils/useRootElementName'; -import { GenericHTMLProps } from '../utils/types'; +import { BaseUIEvent, GenericHTMLProps } from '../utils/types'; export function useButton(parameters: useButton.Parameters = {}): useButton.ReturnValue { const { @@ -67,22 +66,40 @@ export function useButton(parameters: useButton.Parameters = {}): useButton.Retu const getButtonProps = React.useCallback( (externalProps: GenericButtonProps = {}): GenericButtonProps => { - const onClickProp = externalProps?.onClick ?? NOOP; + const { + onClick: externalOnClick, + onMouseDown: externalOnMouseDown, + onKeyUp: externalOnKeyUp, + onKeyDown: externalOnKeyDown, + ...otherExternalProps + } = externalProps; - const otherExternalProps = { ...externalProps }; - delete otherExternalProps.onClick; return mergeReactProps(otherExternalProps, buttonProps, { type, onClick(event: React.MouseEvent) { if (!disabled) { - onClickProp(event); + externalOnClick?.(event); } }, - onKeyDown(event: React.KeyboardEvent) { + onMouseDown(event: React.MouseEvent) { + if (!disabled) { + externalOnMouseDown?.(event); + } + }, + onKeyDown(event: BaseUIEvent) { if (event.target === event.currentTarget && !isNativeButton() && event.key === ' ') { event.preventDefault(); } + if (!disabled) { + makeEventPreventable(event); + externalOnKeyDown?.(event); + } + + if (event.baseUIHandlerPrevented) { + return; + } + // Keyboard accessibility for non interactive elements if ( event.target === event.currentTarget && @@ -91,21 +108,30 @@ export function useButton(parameters: useButton.Parameters = {}): useButton.Retu event.key === 'Enter' && !disabled ) { - onClickProp(event); + externalOnClick?.(event); event.preventDefault(); } }, - onKeyUp(event: React.KeyboardEvent) { + onKeyUp(event: BaseUIEvent) { // calling preventDefault in keyUp on a