[Menu] Fix prop merging issues (#1445)

Co-authored-by: atomiks <[email protected]>
mui · Feb 21, 2025 · 698a8ee · 698a8ee
1 parent be07bd6
commit 698a8ee
Show file tree

Hide file tree

Showing 31 changed files with 566 additions and 327 deletions.
diff --git 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
@@ -12,23 +12,40 @@ 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<Settings>();
+
   const anchorRef = React.useRef<HTMLDivElement>(null);
 
+  const triggerRender = React.useMemo(
+    () => (settings.customTriggerElement ? <span /> : undefined),
+    [settings.customTriggerElement],
+  );
+
   return (
     <div>
-      <Menu.Root openOnHover={settings.openOnHover} modal={settings.modal}>
-        <Menu.Trigger className={classes.Button}>
+      <h1>Fully featured menu</h1>
+      <Menu.Root
+        openOnHover={settings.openOnHover}
+        modal={settings.modal}
+        disabled={settings.disabled}
+      >
+        <Menu.Trigger className={classes.Button} render={triggerRender}>
           Menu <ChevronDownIcon className={classes.ButtonIcon} />
         </Menu.Trigger>
         <Menu.Portal keepMounted>
           <Menu.Positioner
             className={classes.Positioner}
             sideOffset={8}
             anchor={settings.customAnchor ? anchorRef : undefined}
+            side={settings.side}
+            align={settings.align}
           >
             <Menu.Popup className={classes.Popup}>
               <Menu.Arrow className={classes.Arrow}>
@@ -59,7 +76,7 @@ export default function MenuFullyFeatured() {
                           Radio items
                         </Menu.GroupLabel>
                         <Menu.RadioGroup>
-                          <Menu.RadioItem className={classes.RadioItem} value="date">
+                          <Menu.RadioItem className={classes.RadioItem} value="o1">
                             <Menu.RadioItemIndicator
                               className={classes.RadioItemIndicator}
                             >
@@ -69,7 +86,7 @@ export default function MenuFullyFeatured() {
                             </Menu.RadioItemIndicator>
                             <span className={classes.RadioItemText}>Option 1</span>
                           </Menu.RadioItem>
-                          <Menu.RadioItem className={classes.RadioItem} value="name">
+                          <Menu.RadioItem className={classes.RadioItem} value="o2">
                             <Menu.RadioItemIndicator
                               className={classes.RadioItemIndicator}
                             >
@@ -81,7 +98,7 @@ export default function MenuFullyFeatured() {
                           </Menu.RadioItem>
                           <Menu.RadioItem
                             className={classes.RadioItem}
-                            value="type"
+                            value="o3"
                             closeOnClick
                           >
                             <Menu.RadioItemIndicator
@@ -95,6 +112,22 @@ export default function MenuFullyFeatured() {
                               Option 3 (close on click)
                             </span>
                           </Menu.RadioItem>
+                          <Menu.RadioItem
+                            className={classes.RadioItem}
+                            value="o4"
+                            disabled
+                          >
+                            <Menu.RadioItemIndicator
+                              className={classes.RadioItemIndicator}
+                            >
+                              <CheckIcon
+                                className={classes.RadioItemIndicatorIcon}
+                              />
+                            </Menu.RadioItemIndicator>
+                            <span className={classes.RadioItemText}>
+                              Disabled option
+                            </span>
+                          </Menu.RadioItem>
                         </Menu.RadioGroup>
                       </Menu.Group>
 
@@ -139,6 +172,18 @@ export default function MenuFullyFeatured() {
                             Option C (close on click)
                           </span>
                         </Menu.CheckboxItem>
+                        <Menu.CheckboxItem className={classes.CheckboxItem} disabled>
+                          <Menu.CheckboxItemIndicator
+                            className={classes.CheckboxItemIndicator}
+                          >
+                            <CheckIcon
+                              className={classes.CheckboxItemIndicatorIcon}
+                            />
+                          </Menu.CheckboxItemIndicator>
+                          <span className={classes.CheckboxItemText}>
+                            Disabled option
+                          </span>
+                        </Menu.CheckboxItem>
                       </Menu.Group>
 
                       <Menu.Separator className={classes.Separator} />
@@ -171,6 +216,22 @@ export default function MenuFullyFeatured() {
                   </Menu.Positioner>
                 </Menu.Portal>
               </Menu.Root>
+
+              <Menu.Root disabled>
+                <Menu.SubmenuTrigger className={classes.SubmenuTrigger}>
+                  Disabled nested menu
+                  <ChevronRightIcon />
+                </Menu.SubmenuTrigger>
+                <Menu.Portal>
+                  <Menu.Positioner className={classes.Positioner} sideOffset={8}>
+                    <Menu.Popup className={classes.Popup}>
+                      <Menu.Item className={classes.Item}>
+                        This should not appear
+                      </Menu.Item>
+                    </Menu.Popup>
+                  </Menu.Positioner>
+                </Menu.Portal>
+              </Menu.Root>
             </Menu.Popup>
           </Menu.Positioner>
         </Menu.Portal>
@@ -199,6 +260,26 @@ export const settingsMetadata: SettingsMetadata<Settings> = {
     type: 'boolean',
     label: 'Open on hover',
   },
+  disabled: {
+    type: 'boolean',
+    label: 'Disabled',
+  },
+  customTriggerElement: {
+    type: 'boolean',
+    label: 'Trigger as <span>',
+  },
+  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
@@ -82,7 +82,7 @@ export default function NestedMenu() {
                         Paragraph
                       </MenuItem>
                       <Menu.Root disabled>
-                        <SubmenuTrigger disabled>List</SubmenuTrigger>
+                        <SubmenuTrigger>List</SubmenuTrigger>
                         <Menu.Portal>
                           <Menu.Positioner
                             align="start"

diff --git a/docs/src/app/(private)/experiments/menu/menu.module.css b/docs/src/app/(private)/experiments/menu/menu.module.css
@@ -196,6 +196,14 @@
     border-radius: 0.25rem;
     background-color: var(--color-gray-900);
   }
+
+  &[data-disabled][data-highlighted]::before {
+    background-color: var(--color-gray-300);
+  }
+
+  &[data-disabled] {
+    color: var(--color-gray-400);
+  }
 }
 
 .CheckboxItemIndicator,

diff --git a/docs/src/app/(private)/experiments/popups/popups-in-popups.tsx b/docs/src/app/(private)/experiments/popups/popups-in-popups.tsx
@@ -181,7 +181,7 @@ function MenuDemo({ modal }: Props) {
                       Paragraph
                     </MenuItem>
                     <Menu.Root disabled>
-                      <SubmenuTrigger disabled>List</SubmenuTrigger>
+                      <SubmenuTrigger>List</SubmenuTrigger>
                       <Menu.Portal>
                         <Menu.Positioner
                           align="start"

diff --git a/packages/react/src/menu/arrow/MenuArrow.tsx b/packages/react/src/menu/arrow/MenuArrow.tsx
@@ -8,7 +8,6 @@ import { useForkRef } from '../../utils/useForkRef';
 import type { Side, Align } from '../../utils/useAnchorPositioning';
 import type { BaseUIComponentProps } from '../../utils/types';
 import { popupStateMapping } from '../../utils/popupStateMapping';
-import { mergeReactProps } from '../../utils/mergeReactProps';
 
 /**
  * Displays an element positioned against the menu anchor.
@@ -25,16 +24,6 @@ const MenuArrow = React.forwardRef(function MenuArrow(
   const { open } = useMenuRootContext();
   const { arrowRef, side, align, arrowUncentered, arrowStyles } = useMenuPositionerContext();
 
-  const getArrowProps = React.useCallback(
-    (externalProps = {}) => {
-      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
@@ -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
@@ -364,18 +364,18 @@ describe('<Menu.CheckboxItem />', () => {
       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
@@ -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<boolean>;
   typingRef: React.RefObject<boolean>;