fix: clicking MenuItem closes MenuButton (#1820)

* fix: clicking MenuItem didn't close MenuButton * fix: incorrect prop name in MenuButton stories
okta · Jun 14, 2023 · bde9d88 · bde9d88
1 parent ab766e2
commit bde9d88
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 57 deletions.
diff --git a/packages/odyssey-react-mui/src/Button.tsx b/packages/odyssey-react-mui/src/Button.tsx
@@ -93,7 +93,7 @@ const Button = ({
   return (
     <>
       {tooltipText && (
-        <Tooltip ariaType="description" placement="top" text={tooltipText}>
+        <Tooltip ariaType="description" text={tooltipText}>
           {button}
         </Tooltip>
       )}

diff --git a/packages/odyssey-react-mui/src/MenuButton.tsx b/packages/odyssey-react-mui/src/MenuButton.tsx
@@ -20,7 +20,9 @@ import {
   MenuItem,
   useUniqueId,
 } from "./";
-import { memo, MouseEvent, ReactElement, useMemo, useState } from "react";
+import { memo, type ReactElement, useCallback, useMemo, useState } from "react";
+
+import { MenuContext, MenuContextType } from "./MenuContext";
 
 export type MenuButtonProps = {
   /**
@@ -29,10 +31,6 @@ export type MenuButtonProps = {
   children: Array<
     ReactElement<typeof MenuItem | typeof Divider | typeof ListSubheader>
   >;
-  /**
-   * The end Icon on the trigggering Button
-   */
-  buttonEndIcon?: ReactElement;
   /**
    * The label on the triggering Button
    */
@@ -41,6 +39,10 @@ export type MenuButtonProps = {
    * The variant of the triggering Button
    */
   buttonVariant?: ButtonProps["variant"];
+  /**
+   * The end Icon on the trigggering Button
+   */
+  endIcon?: ReactElement;
   /**
    * The id of the `input` element.
    */
@@ -53,23 +55,23 @@ export type MenuButtonProps = {
 
 const MenuButton = ({
   buttonLabel = "",
-  children,
-  buttonEndIcon = <ChevronDownIcon />,
   buttonVariant = "secondary",
+  children,
+  endIcon = <ChevronDownIcon />,
   id: idOverride,
   ariaLabel,
 }: MenuButtonProps) => {
   const [anchorEl, setAnchorEl] = useState<null | HTMLElement>(null);
 
-  const open = Boolean(anchorEl);
-
-  const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
-    setAnchorEl(event.currentTarget);
-  };
+  const isOpen = Boolean(anchorEl);
 
-  const handleClose = () => {
+  const closeMenu = useCallback<MenuContextType["closeMenu"]>(() => {
     setAnchorEl(null);
-  };
+  }, []);
+
+  const openMenu = useCallback<MenuContextType["openMenu"]>((event) => {
+    setAnchorEl(event.currentTarget);
+  }, []);
 
   const uniqueId = useUniqueId(idOverride);
 
@@ -78,27 +80,38 @@ const MenuButton = ({
     [uniqueId]
   );
 
+  const providerValue = useMemo<MenuContextType>(
+    () => ({
+      closeMenu,
+      openMenu,
+    }),
+    [closeMenu, openMenu]
+  );
+
   return (
     <div>
       <Button
-        endIcon={buttonEndIcon}
-        id={`${uniqueId}-button`}
-        aria-controls={open ? `${uniqueId}-menu` : undefined}
+        aria-controls={isOpen ? `${uniqueId}-menu` : undefined}
+        aria-expanded={isOpen ? "true" : undefined}
         aria-haspopup="true"
-        aria-expanded={open ? "true" : undefined}
-        onClick={handleClick}
+        endIcon={endIcon}
+        id={`${uniqueId}-button`}
+        onClick={openMenu}
         text={buttonLabel}
         ariaLabel={ariaLabel}
         variant={buttonVariant}
       />
+
       <Menu
-        id={`${uniqueId}-menu`}
         anchorEl={anchorEl}
-        open={open}
-        onClose={handleClose}
+        id={`${uniqueId}-menu`}
         MenuListProps={menuListProps}
+        onClose={closeMenu}
+        open={isOpen}
       >
-        {children}
+        <MenuContext.Provider value={providerValue}>
+          {children}
+        </MenuContext.Provider>
       </Menu>
     </div>
   );

diff --git a/packages/odyssey-react-mui/src/MenuContext.ts b/packages/odyssey-react-mui/src/MenuContext.ts
@@ -0,0 +1,25 @@
+/*!
+ * Copyright (c) 2023-present, Okta, Inc. and/or its affiliates. All rights reserved.
+ * The Okta software accompanied by this notice is provided pursuant to the Apache License, Version 2.0 (the "License.")
+ *
+ * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0.
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *
+ * See the License for the specific language governing permissions and limitations under the License.
+ */
+
+import { createContext, MouseEventHandler } from "react";
+
+export type MenuContextType = {
+  closeMenu: () => void;
+  openMenu: MouseEventHandler<HTMLElement>;
+};
+
+export const MenuContext = createContext<MenuContextType>({
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  closeMenu: () => {},
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  openMenu: () => {},
+});
diff --git a/packages/odyssey-react-mui/src/MenuItem.tsx b/packages/odyssey-react-mui/src/MenuItem.tsx
@@ -10,10 +10,12 @@
  * See the License for the specific language governing permissions and limitations under the License.
  */
 
-import { memo, forwardRef } from "react";
 import { MenuItem as MuiMenuItem } from "@mui/material";
 import { menuItemClasses } from "@mui/material/MenuItem";
 import type { MenuItemProps as MuiMenuItemProps } from "@mui/material";
+import { memo, useContext } from "react";
+
+import { MenuContext } from "./MenuContext";
 
 export interface MenuItemProps
   extends Omit<
@@ -30,19 +32,22 @@ export interface MenuItemProps
   isDestructive?: boolean;
 }
 
-const MenuItem = forwardRef<HTMLLIElement, MenuItemProps>(
-  ({ isDestructive, ...props }, ref) => (
+const MenuItem = ({ isDestructive, ...props }: MenuItemProps) => {
+  const { closeMenu } = useContext(MenuContext);
+  console.log(closeMenu);
+
+  return (
     <MuiMenuItem
       {...props}
-      ref={ref}
+      onClick={closeMenu}
       className={
         isDestructive ? `${menuItemClasses.root}-destructive` : undefined
       }
     >
       {props.children}
     </MuiMenuItem>
-  )
-);
+  );
+};
 
 const MemoizedMenuItem = memo(MenuItem);
 MemoizedMenuItem.displayName = "MenuItem";

diff --git a/packages/odyssey-storybook/src/components/odyssey-mui/MenuButton/MenuButton.stories.tsx b/packages/odyssey-storybook/src/components/odyssey-mui/MenuButton/MenuButton.stories.tsx
@@ -36,10 +36,13 @@ const storybookMeta: Meta<MenuButtonProps> = {
     buttonLabel: {
       control: "text",
     },
-    buttonEndIcon: {
+    buttonVariant: {
       control: "text",
     },
-    buttonVariant: {
+    endIcon: {
+      control: "text",
+    },
+    id: {
       control: "text",
     },
   },
@@ -51,36 +54,33 @@ const storybookMeta: Meta<MenuButtonProps> = {
 
 export default storybookMeta;
 
-const DefaultTemplate: StoryObj<MenuButtonProps> = {};
-
 export const Simple: StoryObj<MenuButtonProps> = {
-  ...DefaultTemplate,
   args: {
     buttonLabel: "More actions",
     children: [
-      <MenuItem>View details</MenuItem>,
-      <MenuItem>Edit configuration</MenuItem>,
-      <MenuItem>Launch</MenuItem>,
+      <MenuItem key="1">View details</MenuItem>,
+      <MenuItem key="2">Edit configuration</MenuItem>,
+      <MenuItem key="3">Launch</MenuItem>,
     ],
   },
 };
 
 export const ActionIcons: StoryObj<MenuButtonProps> = {
   args: {
     children: [
-      <MenuItem>
+      <MenuItem key="1">
         <ListItemIcon>
           <UserGroupIcon />
         </ListItemIcon>
         <ListItemText>Assign crew</ListItemText>
       </MenuItem>,
-      <MenuItem>
+      <MenuItem key="2">
         <ListItemIcon>
           <GlobeIcon />
         </ListItemIcon>
         <ListItemText>View destination</ListItemText>
       </MenuItem>,
-      <MenuItem>
+      <MenuItem key="3">
         <ListItemIcon>
           <CalendarIcon />
         </ListItemIcon>
@@ -95,9 +95,9 @@ export const ButtonVariant: StoryObj<MenuButtonProps> = {
     buttonLabel: "More actions",
     buttonVariant: "floating",
     children: [
-      <MenuItem>View details</MenuItem>,
-      <MenuItem>Edit configuration</MenuItem>,
-      <MenuItem>Launch</MenuItem>,
+      <MenuItem key="1">View details</MenuItem>,
+      <MenuItem key="2">Edit configuration</MenuItem>,
+      <MenuItem key="3">Launch</MenuItem>,
     ],
   },
 };
@@ -107,13 +107,13 @@ export const Groupings: StoryObj<MenuButtonProps> = {
     buttonLabel: "More actions",
     children: [
       <ListSubheader>Crew</ListSubheader>,
-      <MenuItem>Assign captain</MenuItem>,
-      <MenuItem>View roster</MenuItem>,
+      <MenuItem key="1">Assign captain</MenuItem>,
+      <MenuItem key="2">View roster</MenuItem>,
       <ListSubheader>Ship</ListSubheader>,
-      <MenuItem>Configure thrusters</MenuItem>,
-      <MenuItem>View cargo</MenuItem>,
+      <MenuItem key="3">Configure thrusters</MenuItem>,
+      <MenuItem key="4">View cargo</MenuItem>,
       <Divider />,
-      <MenuItem>Logout</MenuItem>,
+      <MenuItem key="5">Logout</MenuItem>,
     ],
   },
 };
@@ -122,22 +122,24 @@ export const WithDestructive: StoryObj<MenuButtonProps> = {
   args: {
     buttonLabel: "Cargo options",
     children: [
-      <MenuItem>View details</MenuItem>,
-      <MenuItem>Edit inventory</MenuItem>,
-      <MenuItem isDestructive>Jettison cargo</MenuItem>,
+      <MenuItem key="1">View details</MenuItem>,
+      <MenuItem key="2">Edit inventory</MenuItem>,
+      <MenuItem isDestructive key="3">
+        Jettison cargo
+      </MenuItem>,
     ],
   },
 };
 
 export const IconButton: StoryObj<MenuButtonProps> = {
   args: {
+    ariaLabel: "Add",
     children: [
-      <MenuItem>View details</MenuItem>,
-      <MenuItem>Edit configuration</MenuItem>,
-      <MenuItem>Launch</MenuItem>,
+      <MenuItem key="1">View details</MenuItem>,
+      <MenuItem key="2">Edit configuration</MenuItem>,
+      <MenuItem key="3">Launch</MenuItem>,
     ],
     buttonLabel: "",
-    buttonEndIcon: <OverflowVerticalIcon />,
-    ariaLabel: "Add",
+    endIcon: <OverflowVerticalIcon />,
   },
 };