[useButton] Fix focusableWhenDisabled components (#1313)

mui · Jan 10, 2025 · 47bf76d · 47bf76d
1 parent c4addb1
commit 47bf76d
Show file tree

Hide file tree

Showing 18 changed files with 492 additions and 64 deletions.
diff --git a/docs/reference/generated/menu-radio-group.json b/docs/reference/generated/menu-radio-group.json
@@ -15,6 +15,11 @@
       "default": "() => {}",
       "description": "Function called when the selected value changes."
     },
+    "disabled": {
+      "type": "boolean",
+      "default": "false",
+      "description": "Whether the component should ignore user interaction."
+    },
     "children": {
       "type": "React.ReactNode",
       "description": "The content of the component."

diff --git a/packages/react/src/accordion/root/useAccordionRoot.ts b/packages/react/src/accordion/root/useAccordionRoot.ts
@@ -28,9 +28,7 @@ function getActiveTriggers(accordionItemRefs: {
 
 function isDisabled(element: HTMLElement | null) {
   return (
-    element === null ||
-    element.hasAttribute('disabled') ||
-    element.getAttribute('data-disabled') === 'true'
+    element === null || element.hasAttribute('disabled') || element.hasAttribute('data-disabled')
   );
 }
 

diff --git a/packages/react/src/collapsible/trigger/useCollapsibleTrigger.ts b/packages/react/src/collapsible/trigger/useCollapsibleTrigger.ts
@@ -20,21 +20,17 @@ export function useCollapsibleTrigger(
 
   const getRootProps: useCollapsibleTrigger.ReturnValue['getRootProps'] = React.useCallback(
     (externalProps: GenericHTMLProps = {}) =>
-      mergeReactProps(
-        externalProps,
-        mergeReactProps(
-          {
-            type: 'button',
-            'aria-controls': panelId,
-            'aria-expanded': open,
-            disabled,
-            onClick() {
-              setOpen(!open);
-            },
-            ref: handleRef,
+      getButtonProps(
+        mergeReactProps(externalProps, {
+          type: 'button',
+          'aria-controls': panelId,
+          'aria-expanded': open,
+          disabled,
+          onClick() {
+            setOpen(!open);
           },
-          getButtonProps(),
-        ),
+          ref: handleRef,
+        }),
       ),
     [panelId, disabled, getButtonProps, handleRef, open, setOpen],
   );

diff --git a/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx b/packages/react/src/menu/checkbox-item/MenuCheckboxItem.test.tsx
@@ -331,4 +331,53 @@ describe('<Menu.CheckboxItem />', () => {
       expect(queryByRole('menu')).not.to.equal(null);
     });
   });
+
+  describe('focusableWhenDisabled', () => {
+    it('can be focused but not interacted with when disabled', async () => {
+      const handleCheckedChange = spy();
+      const handleClick = spy();
+      const handleKeyDown = spy();
+      const handleKeyUp = spy();
+
+      const { getByRole } = await render(
+        <Menu.Root open>
+          <Menu.Portal>
+            <Menu.Positioner>
+              <Menu.Popup>
+                <Menu.CheckboxItem
+                  disabled
+                  onCheckedChange={handleCheckedChange}
+                  onClick={handleClick}
+                  onKeyDown={handleKeyDown}
+                  onKeyUp={handleKeyUp}
+                >
+                  Item
+                </Menu.CheckboxItem>
+              </Menu.Popup>
+            </Menu.Positioner>
+          </Menu.Portal>
+        </Menu.Root>,
+      );
+
+      const item = getByRole('menuitemcheckbox');
+      await act(() => item.focus());
+      expect(item).toHaveFocus();
+
+      fireEvent.keyDown(item, { key: 'Enter' });
+      expect(handleKeyDown.callCount).to.equal(1);
+      expect(handleClick.callCount).to.equal(0);
+      expect(handleCheckedChange.callCount).to.equal(0);
+
+      fireEvent.keyUp(item, { key: 'Space' });
+      expect(handleKeyUp.callCount).to.equal(1);
+      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(handleClick.callCount).to.equal(0);
+      expect(handleCheckedChange.callCount).to.equal(0);
+    });
+  });
 });
diff --git a/packages/react/src/menu/item/MenuItem.test.tsx b/packages/react/src/menu/item/MenuItem.test.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 import { expect } from 'chai';
 import { spy } from 'sinon';
 import { MemoryRouter, Route, Routes, Link, useLocation } from 'react-router-dom';
-import { act, screen, waitFor } from '@mui/internal-test-utils';
+import { act, fireEvent, screen, waitFor } from '@mui/internal-test-utils';
 import { Menu } from '@base-ui-components/react/menu';
 import { describeConformance, createRenderer, isJSDOM } from '#test-utils';
 
@@ -237,4 +237,48 @@ describe('<Menu.Item />', () => {
       expect(locationDisplay).to.have.text('/');
     });
   });
+
+  describe('disabled state', () => {
+    it('can be focused but not interacted with when disabled', async () => {
+      const handleClick = spy();
+      const handleKeyDown = spy();
+      const handleKeyUp = spy();
+
+      const { getByRole } = await render(
+        <Menu.Root open>
+          <Menu.Portal>
+            <Menu.Positioner>
+              <Menu.Popup>
+                <Menu.Item
+                  disabled
+                  onClick={handleClick}
+                  onKeyDown={handleKeyDown}
+                  onKeyUp={handleKeyUp}
+                >
+                  Item
+                </Menu.Item>
+              </Menu.Popup>
+            </Menu.Positioner>
+          </Menu.Portal>
+        </Menu.Root>,
+      );
+
+      const item = getByRole('menuitem');
+      await act(() => item.focus());
+      expect(item).toHaveFocus();
+
+      fireEvent.keyDown(item, { key: 'Enter' });
+      expect(handleKeyDown.callCount).to.equal(1);
+      expect(handleClick.callCount).to.equal(0);
+
+      fireEvent.keyUp(item, { key: 'Space' });
+      expect(handleKeyUp.callCount).to.equal(1);
+      expect(handleClick.callCount).to.equal(0);
+
+      fireEvent.click(item);
+      expect(handleKeyDown.callCount).to.equal(1);
+      expect(handleKeyUp.callCount).to.equal(1);
+      expect(handleClick.callCount).to.equal(0);
+    });
+  });
 });
diff --git a/packages/react/src/menu/radio-group/MenuRadioGroup.tsx b/packages/react/src/menu/radio-group/MenuRadioGroup.tsx
@@ -1,14 +1,12 @@
 import * as React from 'react';
 import PropTypes from 'prop-types';
 import { MenuRadioGroupContext } from './MenuRadioGroupContext';
+import { NOOP } from '../../utils/noop';
 import { BaseUIComponentProps } from '../../utils/types';
 import { useComponentRenderer } from '../../utils/useComponentRenderer';
 import { useControlled } from '../../utils/useControlled';
 import { useEventCallback } from '../../utils/useEventCallback';
 
-const EMPTY_OBJECT = {};
-const NOOP = () => {};
-
 /**
  * Groups related radio items.
  * Renders a `<div>` element.
@@ -22,7 +20,8 @@ const MenuRadioGroup = React.forwardRef(function MenuRadioGroup(
     className,
     value: valueProp,
     defaultValue,
-    onValueChange: onValueChangeProp = NOOP,
+    onValueChange: onValueChangeProp,
+    disabled = false,
     ...other
   } = props;
 
@@ -32,7 +31,7 @@ const MenuRadioGroup = React.forwardRef(function MenuRadioGroup(
     name: 'MenuRadioGroup',
   });
 
-  const onValueChange = useEventCallback(onValueChangeProp);
+  const onValueChange = useEventCallback(onValueChangeProp ?? NOOP);
 
   const setValue = React.useCallback(
     (newValue: any, event: Event) => {
@@ -42,23 +41,27 @@ const MenuRadioGroup = React.forwardRef(function MenuRadioGroup(
     [onValueChange, setValueUnwrapped],
   );
 
+  const state = React.useMemo(() => ({ disabled }), [disabled]);
+
   const { renderElement } = useComponentRenderer({
     render: render || 'div',
     className,
-    state: EMPTY_OBJECT,
+    state,
     extraProps: {
       role: 'group',
+      'aria-disabled': disabled || undefined,
       ...other,
     },
     ref: forwardedRef,
   });
 
-  const context = React.useMemo(
+  const context: MenuRadioGroupContext = React.useMemo(
     () => ({
       value,
       setValue,
+      disabled,
     }),
-    [value, setValue],
+    [value, setValue, disabled],
   );
 
   return (
@@ -92,9 +95,17 @@ namespace MenuRadioGroup {
      * @default () => {}
      */
     onValueChange?: (value: any, event: Event) => void;
+    /**
+     * Whether the component should ignore user interaction.
+     *
+     * @default false
+     */
+    disabled?: boolean;
   }
 
-  export type State = {};
+  export type State = {
+    disabled: boolean;
+  };
 }
 
 MenuRadioGroup.propTypes /* remove-proptypes */ = {
@@ -117,6 +128,12 @@ MenuRadioGroup.propTypes /* remove-proptypes */ = {
    * To render a controlled radio group, use the `value` prop instead.
    */
   defaultValue: PropTypes.any,
+  /**
+   * Whether the component should ignore user interaction.
+   *
+   * @default false
+   */
+  disabled: PropTypes.bool,
   /**
    * Function called when the selected value changes.
    *

diff --git a/packages/react/src/menu/radio-group/MenuRadioGroupContext.ts b/packages/react/src/menu/radio-group/MenuRadioGroupContext.ts
@@ -3,6 +3,7 @@ import * as React from 'react';
 export interface MenuRadioGroupContext {
   value: any;
   setValue: (newValue: any, event: Event) => void;
+  disabled: boolean;
 }
 
 export const MenuRadioGroupContext = React.createContext<MenuRadioGroupContext | undefined>(