[LG-2187] feat: Menu Prop Updates (#3240)

* [LG-2187] feat: Menu Prop Updates

* fixed contextMenu

* cleanup

* updated changeset

* rm old changeset

* updated ContextMenu implementation

Author: adamrasheed

.changeset/cyan-taxes-cry.md

@@ -0,0 +1,6 @@
+---
+'@leafygreen-ui/menu': major
+'@leafygreen-ui/code-editor': minor
+---
+
+Updated menu to require either refEl or trigger prop

packages/code-editor/src/ContextMenu/ContextMenu.tsx

@@ -47,6 +47,7 @@ export const ContextMenu = ({
   const [position, setPosition] = useState({ x: 0, y: 0 });
   const [selectedText, setSelectedText] = useState('');
   const containerRef = useRef<HTMLDivElement>(null);
+  const menuRef = useRef<HTMLDivElement>(null);
 
   /**
    * Handle showing and positioning custom menu onContextMenu
@@ -151,6 +152,8 @@ export const ContextMenu = ({
 
       {isOpen && (
         <div className={getMenuContainerStyles(position)}>
+          {/* This is a workaround to prevent the Menu from being rendered in the DOM when it's not open */}
+          <div ref={menuRef} />
           <Menu
             /** Force re-mount when position changes so Menu recalculates its positioning */
             key={`${position.x}-${position.y}`}
@@ -160,6 +163,7 @@ export const ContextMenu = ({
             variant={MenuVariant.Compact}
             data-lgid={lgIds.contextMenu}
             data-testid={lgIds.contextMenu}
+            refEl={menuRef}
           >
             {menuItems.map((item, index) => {
               if (item.isSeparator) {

packages/menu/src/Menu.spec.tsx

@@ -639,15 +639,102 @@ describe('packages/menu', () => {
     });
   });
 
-  // eslint-disable-next-line jest/no-disabled-tests
-  describe.skip('Types work as expected', () => {
-    test('Types work as expected', () => {
-      render(
-        <Menu data-testid="menu">
-          <MenuItem>Item</MenuItem>
-          <MenuItem>Item</MenuItem>
+  describe('refEl and trigger props', () => {
+    test('Menu works with only trigger prop', async () => {
+      const trigger = <button data-testid="trigger-btn">Open Menu</button>;
+      const { getByTestId, findByTestId } = render(
+        <Menu trigger={trigger}>
+          <MenuItem data-testid="menu-item">Item</MenuItem>
         </Menu>,
       );
+
+      const triggerBtn = getByTestId('trigger-btn');
+      expect(triggerBtn).toBeInTheDocument();
+
+      userEvent.click(triggerBtn);
+
+      const menu = await findByTestId(lgIds.root);
+      expect(menu).toBeInTheDocument();
+
+      const menuItem = getByTestId('menu-item');
+      expect(menuItem).toBeInTheDocument();
+    });
+
+    test('Menu works with only refEl prop', async () => {
+      const refEl = React.createRef<HTMLButtonElement>();
+      const { getByTestId, findByTestId } = render(
+        <>
+          <button ref={refEl} data-testid="ref-element">
+            Reference Element
+          </button>
+          <Menu refEl={refEl} open>
+            <MenuItem data-testid="menu-item">Item</MenuItem>
+          </Menu>
+        </>,
+      );
+
+      const refElement = getByTestId('ref-element');
+      expect(refElement).toBeInTheDocument();
+
+      const menu = await findByTestId(lgIds.root);
+      expect(menu).toBeInTheDocument();
+
+      const menuItem = getByTestId('menu-item');
+      expect(menuItem).toBeInTheDocument();
+    });
+
+    test('Menu works with both trigger and refEl props', async () => {
+      const refEl = React.createRef<HTMLDivElement>();
+      const trigger = <button data-testid="trigger-btn">Open Menu</button>;
+      const { getByTestId, findByTestId } = render(
+        <>
+          <div ref={refEl} data-testid="ref-element">
+            Reference Element
+          </div>
+          <Menu trigger={trigger} refEl={refEl}>
+            <MenuItem data-testid="menu-item">Item</MenuItem>
+          </Menu>
+        </>,
+      );
+
+      const triggerBtn = getByTestId('trigger-btn');
+      expect(triggerBtn).toBeInTheDocument();
+
+      userEvent.click(triggerBtn);
+
+      const menu = await findByTestId(lgIds.root);
+      expect(menu).toBeInTheDocument();
+
+      const menuItem = getByTestId('menu-item');
+      expect(menuItem).toBeInTheDocument();
+    });
+  });
+
+  // eslint-disable-next-line jest/no-disabled-tests
+  describe.skip('Types work as expected', () => {
+    test('Menu requires at least one of refEl or trigger', () => {
+      const refEl = React.createRef<HTMLDivElement>();
+      const trigger = <button>Open Menu</button>;
+
+      // @ts-expect-error - Missing both refEl and trigger
+      <Menu>
+        <MenuItem>Item</MenuItem>
+      </Menu>;
+
+      // Valid: Menu with only trigger
+      <Menu trigger={trigger}>
+        <MenuItem>Item</MenuItem>
+      </Menu>;
+
+      // Valid: Menu with only refEl
+      <Menu refEl={refEl} open>
+        <MenuItem>Item</MenuItem>
+      </Menu>;
+
+      // Valid: Menu with both trigger and refEl
+      <Menu trigger={trigger} refEl={refEl}>
+        <MenuItem>Item</MenuItem>
+      </Menu>;
     });
   });
 });

packages/menu/src/Menu/Menu.types.ts

@@ -1,6 +1,6 @@
 import { ReactElement, ReactNode } from 'react';
 
-import { LgIdProps } from '@leafygreen-ui/lib';
+import type { Either, LgIdProps } from '@leafygreen-ui/lib';
 import {
   InferredPolymorphicPropsWithRef,
   PolymorphicAs,
@@ -19,9 +19,9 @@ export const MenuVariant = {
 } as const;
 export type MenuVariant = (typeof MenuVariant)[keyof typeof MenuVariant];
 
-export interface MenuProps
+interface BaseMenuProps
   extends Omit<React.ComponentPropsWithoutRef<'ul'>, 'onClick'>,
-    Omit<PopoverProps, 'active' | 'dismissMode' | 'onToggle'>,
+    Omit<PopoverProps, 'active' | 'dismissMode' | 'onToggle' | 'refEl'>,
     LgIdProps {
   /**
    * The menu items, or submenus
@@ -34,6 +34,11 @@ export interface MenuProps
    */
   'data-testid'?: string;
 
+  /**
+   * A reference to the element against which the popover component will be positioned.
+   */
+  refEl?: React.RefObject<HTMLElement>;
+
   /**
    * A slot for the element used to trigger the Menu. Passing a trigger allows
    * Menu to control opening and closing itself internally.
@@ -107,3 +112,8 @@ export interface MenuProps
    */
   variant?: MenuVariant;
 }
+
+/**
+ * Menu props require at least one of `refEl` or `trigger` to be provided
+ */
+export type MenuProps = Either<BaseMenuProps, 'refEl' | 'trigger'>;