fix: Fix disabled MenuButton being activated when used as a menu item

packages/reakit/src/Menu/MenuButton.ts

@@ -12,6 +12,7 @@ import {
 } from "../Popover/PopoverDisclosure";
 import { useMenuState, MenuStateReturn } from "./MenuState";
 import { MenuContext } from "./__utils/MenuContext";
+import { findVisibleSubmenu } from "./__utils/findVisibleSubmenu";
 
 export type MenuButtonOptions = PopoverDisclosureOptions &
   Pick<Partial<MenuStateReturn>, "hide"> &
@@ -47,6 +48,7 @@ export const useMenuButton = createHook<MenuButtonOptions, MenuButtonHTMLProps>(
       const [dir] = options.placement.split("-");
       const hasParent = !!parent;
       const parentIsMenuBar = parent?.role === "menubar";
+      const disabled = options.disabled || htmlProps["aria-disabled"];
       const onClickRef = useLiveRef(htmlOnClick);
       const onKeyDownRef = useLiveRef(htmlOnKeyDown);
       const onFocusRef = useLiveRef(htmlOnFocus);
@@ -61,6 +63,7 @@ export const useMenuButton = createHook<MenuButtonOptions, MenuButtonHTMLProps>(
             // dialogs when MenuButton is focused
             preventDefault: (event) => event.key !== "Escape",
             stopPropagation: (event) => event.key !== "Escape",
+            shouldKeyDown: (event) => event.key === "Escape" || !disabled,
             onKey: () => options.show(),
             keyMap: () => {
               // prevents scroll jump
@@ -79,6 +82,7 @@ export const useMenuButton = createHook<MenuButtonOptions, MenuButtonHTMLProps>(
             },
           }),
         [
+          disabled,
           dir,
           hasParent,
           options.show,
@@ -99,10 +103,7 @@ export const useMenuButton = createHook<MenuButtonOptions, MenuButtonHTMLProps>(
           if (parentIsMenuBar) {
             // if MenuButton is an item inside a MenuBar, it'll only open
             // if there's already another sibling expanded MenuButton
-            const subjacentOpenMenu = parent.children.some(
-              (r) => r.current && !r.current.hidden
-            );
-            if (subjacentOpenMenu) {
+            if (findVisibleSubmenu(parent.children)) {
               self.focus();
             }
           } else {
@@ -131,11 +132,12 @@ export const useMenuButton = createHook<MenuButtonOptions, MenuButtonHTMLProps>(
         (event: React.FocusEvent) => {
           onFocusRef.current?.(event);
           if (event.defaultPrevented) return;
+          if (disabled) return;
           if (parentIsMenuBar && !hasPressedMouse.current) {
             options.show?.();
           }
         },
-        [parentIsMenuBar, options.show]
+        [parentIsMenuBar, options.show, disabled]
       );
 
       // If disclosure is rendered as a menu bar item, it's toggable

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/EditMenu.tsx

@@ -0,0 +1,33 @@
+import * as React from "react";
+import {
+  useMenuState,
+  Menu,
+  MenuButton,
+  MenuItem,
+  MenuSeparator,
+  MenuButtonProps,
+} from "reakit/Menu";
+import FindMenu from "./FindMenu";
+import SpellingAndGrammarMenu from "./SpellingAndGrammarMenu";
+
+const EditMenu = React.forwardRef<HTMLButtonElement, MenuButtonProps>(
+  (props, ref) => {
+    const menu = useMenuState();
+    return (
+      <>
+        <MenuButton {...menu} {...props} ref={ref}>
+          Edit
+        </MenuButton>
+        <Menu {...menu} aria-label="Edit">
+          <MenuItem {...menu}>Undo</MenuItem>
+          <MenuItem {...menu}>Redo</MenuItem>
+          <MenuSeparator {...menu} />
+          <MenuItem {...menu} as={FindMenu} />
+          <MenuItem {...menu} as={SpellingAndGrammarMenu} disabled focusable />
+        </Menu>
+      </>
+    );
+  }
+);
+
+export default EditMenu;

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/FileMenu.tsx

@@ -0,0 +1,36 @@
+import * as React from "react";
+import {
+  useMenuState,
+  Menu,
+  MenuButton,
+  MenuItem,
+  MenuSeparator,
+  MenuButtonProps,
+} from "reakit/Menu";
+
+const FileMenu = React.forwardRef<HTMLButtonElement, MenuButtonProps>(
+  (props, ref) => {
+    const menu = useMenuState();
+    return (
+      <>
+        <MenuButton {...menu} {...props} ref={ref}>
+          File
+        </MenuButton>
+        <Menu {...menu} aria-label="File">
+          <MenuItem {...menu}>New Tab</MenuItem>
+          <MenuItem {...menu}>New Window</MenuItem>
+          <MenuItem {...menu}>New Icognito Window</MenuItem>
+          <MenuItem {...menu}>Reopen Closed Tab</MenuItem>
+          <MenuItem {...menu}>Open File...</MenuItem>
+          <MenuItem {...menu}>Open Location...</MenuItem>
+          <MenuSeparator {...menu} />
+          <MenuItem {...menu}>Close Window</MenuItem>
+          <MenuItem {...menu}>Close Tab</MenuItem>
+          <MenuItem {...menu}>Save Page As...</MenuItem>
+        </Menu>
+      </>
+    );
+  }
+);
+
+export default FileMenu;

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/FindMenu.tsx

@@ -0,0 +1,35 @@
+import * as React from "react";
+import {
+  useMenuState,
+  Menu,
+  MenuButton,
+  MenuItem,
+  MenuSeparator,
+  MenuButtonProps,
+} from "reakit/Menu";
+
+const FindMenu = React.forwardRef<HTMLButtonElement, MenuButtonProps>(
+  (props, ref) => {
+    const menu = useMenuState();
+    return (
+      <>
+        <MenuButton {...menu} {...props} ref={ref}>
+          Find
+        </MenuButton>
+        <Menu {...menu} aria-label="Find">
+          <MenuItem {...menu}>Search the Web...</MenuItem>
+          <MenuSeparator {...menu} />
+          <MenuItem {...menu}>Find...</MenuItem>
+          <MenuItem {...menu}>Find Next</MenuItem>
+          <MenuItem {...menu}>Find previous</MenuItem>
+          <MenuItem {...menu}>Use Selection for Find</MenuItem>
+          <MenuItem {...menu} disabled>
+            Jump to Selection
+          </MenuItem>
+        </Menu>
+      </>
+    );
+  }
+);
+
+export default FindMenu;

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/SpellingAndGrammarMenu.tsx

@@ -0,0 +1,35 @@
+import * as React from "react";
+import {
+  useMenuState,
+  Menu,
+  MenuButton,
+  MenuItem,
+  MenuItemCheckbox,
+  MenuButtonProps,
+} from "reakit/Menu";
+
+const SpellingAndGrammarMenu = React.forwardRef<
+  HTMLButtonElement,
+  MenuButtonProps
+>((props, ref) => {
+  const menu = useMenuState();
+  return (
+    <>
+      <MenuButton {...menu} {...props} ref={ref}>
+        Spelling and Grammar
+      </MenuButton>
+      <Menu {...menu} aria-label="Spelling and Grammar">
+        <MenuItem {...menu}>Show Spelling and Grammar</MenuItem>
+        <MenuItem {...menu}>Check Document Now</MenuItem>
+        <MenuItemCheckbox {...menu} name="checkSpellingWhileTyping">
+          Check Spelling While Typing
+        </MenuItemCheckbox>
+        <MenuItemCheckbox {...menu} name="checkGrammarWithSpelling" disabled>
+          Check Grammar With Spelling
+        </MenuItemCheckbox>
+      </Menu>
+    </>
+  );
+});
+
+export default SpellingAndGrammarMenu;

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/ViewMenu.tsx

@@ -0,0 +1,37 @@
+import * as React from "react";
+import {
+  useMenuState,
+  Menu,
+  MenuButton,
+  MenuItem,
+  MenuItemCheckbox,
+  MenuSeparator,
+  MenuButtonProps,
+} from "reakit/Menu";
+
+const ViewMenu = React.forwardRef<HTMLButtonElement, MenuButtonProps>(
+  (props, ref) => {
+    const menu = useMenuState();
+    return (
+      <>
+        <MenuButton {...menu} {...props} ref={ref}>
+          View
+        </MenuButton>
+        <Menu {...menu} aria-label="View">
+          <MenuItemCheckbox {...menu} name="alwaysShowBookmarksBar">
+            Always Show Bookmarks Bar
+          </MenuItemCheckbox>
+          <MenuItemCheckbox {...menu} name="alwaysShowBookmarksBar">
+            Always Show Toolbar in Full Screen
+          </MenuItemCheckbox>
+          <MenuItem {...menu}>Customize Touch Bar...</MenuItem>
+          <MenuSeparator {...menu} />
+          <MenuItem {...menu}>Stop</MenuItem>
+          <MenuItem {...menu}>Reload This Page</MenuItem>
+        </Menu>
+      </>
+    );
+  }
+);
+
+export default ViewMenu;

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/__tests__/index-test.tsx

@@ -0,0 +1,92 @@
+import * as React from "react";
+import { render, press, hover, click, wait } from "reakit-test-utils";
+import MenuBarWithDisabledItems from "..";
+
+test("open menus with hover except the disabled one", async () => {
+  const { getByText: text, getByLabelText: label } = render(
+    <MenuBarWithDisabledItems />
+  );
+  expect(label("File")).not.toBeVisible();
+  click(text("File"));
+  await wait(expect(label("File")).toBeVisible);
+  expect(text("File")).toHaveFocus();
+  hover(text("View"));
+  expect(label("File")).toBeVisible();
+  expect(text("File")).toHaveFocus();
+  hover(text("Edit"));
+  await wait(expect(label("Edit")).toBeVisible);
+  expect(text("Edit")).toHaveFocus();
+});
+
+test("open menus with click except the disabled one", async () => {
+  const { getByText: text, getByLabelText: label, baseElement } = render(
+    <MenuBarWithDisabledItems />
+  );
+  click(text("File"));
+  await wait(expect(label("File")).toBeVisible);
+  expect(text("File")).toHaveFocus();
+  click(text("View"));
+  expect(baseElement).toHaveFocus();
+  click(text("Edit"));
+  await wait(expect(label("Edit")).toBeVisible);
+  expect(text("Edit")).toHaveFocus();
+});
+
+test("open menus with keyboard except the disabled one", async () => {
+  const { getByText: text, getByLabelText: label } = render(
+    <MenuBarWithDisabledItems />
+  );
+  press.Tab();
+  expect(text("File")).toHaveFocus();
+  await wait(expect(label("File")).toBeVisible);
+  press.ArrowRight();
+  expect(text("View")).toHaveFocus();
+  await wait(expect(label("File")).not.toBeVisible);
+  await wait(expect(label("View")).not.toBeVisible);
+  press.ArrowDown();
+  await wait(expect(label("File")).not.toBeVisible);
+  await wait(expect(label("View")).not.toBeVisible);
+  press.ArrowUp();
+  await wait(expect(label("File")).not.toBeVisible);
+  await wait(expect(label("View")).not.toBeVisible);
+  press.Enter();
+  await wait(expect(label("File")).not.toBeVisible);
+  await wait(expect(label("View")).not.toBeVisible);
+  press.Space();
+  await wait(expect(label("File")).not.toBeVisible);
+  await wait(expect(label("View")).not.toBeVisible);
+  press.ArrowRight();
+  expect(text("Edit")).toHaveFocus();
+  await wait(expect(label("Edit")).toBeVisible);
+  press.ArrowRight();
+  expect(text("File")).toHaveFocus();
+  await wait(expect(label("File")).toBeVisible);
+});
+
+test("open submenus with click except the disabled one", async () => {
+  const { getByText: text, getByLabelText: label } = render(
+    <MenuBarWithDisabledItems />
+  );
+  click(text("Edit"));
+  click(text("Find"));
+  expect(text("Find")).toHaveFocus();
+  await wait(expect(label("Find")).toBeVisible);
+  expect(text("Find")).toHaveFocus();
+  click(text("Spelling and Grammar"));
+  await wait(expect(label("Find")).not.toBeVisible);
+  expect(label("Edit")).toHaveFocus();
+});
+
+test("open submenus with hover except the disabled one", async () => {
+  const { getByText: text, getByLabelText: label } = render(
+    <MenuBarWithDisabledItems />
+  );
+  click(text("Edit"));
+  hover(text("Find"));
+  expect(text("Find")).toHaveFocus();
+  await wait(expect(label("Find")).toBeVisible);
+  expect(text("Find")).toHaveFocus();
+  hover(text("Spelling and Grammar"));
+  await wait(expect(label("Find")).not.toBeVisible);
+  expect(label("Edit")).toHaveFocus();
+});

packages/reakit/src/Menu/__examples__/MenuBarWithDisabledItems/index.tsx

@@ -0,0 +1,16 @@
+import * as React from "react";
+import { useMenuState, MenuBar, MenuItem } from "reakit/Menu";
+import FileMenu from "./FileMenu";
+import ViewMenu from "./ViewMenu";
+import EditMenu from "./EditMenu";
+
+export default function MenuBarWithDisabledItems() {
+  const menu = useMenuState({ loop: true, orientation: "horizontal" });
+  return (
+    <MenuBar {...menu} aria-label="Disabled items">
+      <MenuItem {...menu} as={FileMenu} />
+      <MenuItem {...menu} as={ViewMenu} disabled focusable />
+      <MenuItem {...menu} as={EditMenu} />
+    </MenuBar>
+  );
+}