feat(menu): deprecate props.mode / always support full capabilities + icons (#18153)

* feat(menu): deprecate props.mode

* test(menu): remove mode-specific tests

* docs(menu-button): add nested demo

* fix(menu-button): prevent clipping of submenus

---------

Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
Co-authored-by: Taylor Jones <taylor.jones826@gmail.com>
Co-authored-by: Preeti Bansal <146315451+preetibansalui@users.noreply.github.com>

Author: 3 people

packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap

@@ -4658,15 +4658,7 @@ Map {
       "menuAlignment": Object {
         "type": "string",
       },
-      "mode": Object {
-        "args": Array [
-          Array [
-            "full",
-            "basic",
-          ],
-        ],
-        "type": "oneOf",
-      },
+      "mode": [Function],
       "onClose": Object {
         "type": "func",
       },

packages/react/src/components/ComboButton/ComboButton-test.js

@@ -186,56 +186,6 @@ describe('ComboButton', () => {
       ).toHaveTextContent(/^Additional action$/);
     });
 
-    it('warns when MenuItemSelectable is used in children', async () => {
-      const spy = jest.spyOn(console, 'warn').mockImplementation(() => {});
-
-      render(
-        <ComboButton label="Primary action">
-          <MenuItemSelectable label="Option" />
-        </ComboButton>
-      );
-
-      await userEvent.click(screen.getAllByRole('button')[1]);
-
-      expect(spy).toHaveBeenCalled();
-      spy.mockRestore();
-    });
-
-    it('warns when MenuItemRadioGroup is used in children', async () => {
-      const spy = jest.spyOn(console, 'warn').mockImplementation(() => {});
-
-      render(
-        <ComboButton label="Primary action">
-          <MenuItemRadioGroup
-            label="Options"
-            items={['Option 1', 'Option 2']}
-          />
-        </ComboButton>
-      );
-
-      await userEvent.click(screen.getAllByRole('button')[1]);
-
-      expect(spy).toHaveBeenCalled();
-      spy.mockRestore();
-    });
-
-    it('warns when a nested Menu is used in children', async () => {
-      const spy = jest.spyOn(console, 'warn').mockImplementation(() => {});
-
-      render(
-        <ComboButton label="Primary action">
-          <MenuItem label="Submenu">
-            <MenuItem label="Action" />
-          </MenuItem>
-        </ComboButton>
-      );
-
-      await userEvent.click(screen.getAllByRole('button')[1]);
-
-      expect(spy).toHaveBeenCalled();
-      spy.mockRestore();
-    });
-
     it('supports ellipsis in ComboButton by checking the className', async () => {
       render(
         <ComboButton label="Primary action super long text to enable ellipsis">

packages/react/src/components/ComboButton/index.tsx

@@ -235,7 +235,6 @@ const ComboButton = React.forwardRef<HTMLDivElement, ComboButtonProps>(
           ref={refs.setFloating}
           id={id}
           label={t('carbon.combo-button.additional-actions')}
-          mode="basic"
           size={size}
           open={open}
           onClose={handleClose}>

packages/react/src/components/ContextMenu/useContextMenu.tsx

@@ -14,7 +14,6 @@ export interface ContextMenuProps {
   x: number;
   y: number;
   onClose: () => void;
-  mode: string;
 }
 
 /**
@@ -62,7 +61,6 @@ function useContextMenu(trigger: TriggerType = document): ContextMenuProps {
     x: position[0],
     y: position[1],
     onClose,
-    mode: 'full',
   };
 }
 

packages/react/src/components/Menu/Menu-test.js

@@ -83,21 +83,6 @@ describe('Menu', () => {
       expect(document.querySelector('.custom-class')).toBeInTheDocument();
       document.body.removeChild(el);
     });
-
-    it('warns about nested menus in basic mode', async () => {
-      const spy = jest.spyOn(console, 'warn').mockImplementation(() => {});
-
-      render(
-        <Menu open mode="basic">
-          <MenuItem label="Submenu">
-            <MenuItem label="Item" />
-          </MenuItem>
-        </Menu>
-      );
-      await waitForPosition();
-      expect(spy).toHaveBeenCalled();
-      spy.mockRestore();
-    });
   });
 
   describe('Submenu behavior', () => {
@@ -224,35 +209,6 @@ describe('MenuItem', () => {
 
       expect(screen.getByText('item')).toBeInTheDocument();
     });
-
-    it('warns about MenuItemSelectable in basic mode', () => {
-      const spy = jest.spyOn(console, 'warn').mockImplementation(() => {});
-
-      render(
-        <Menu open mode="basic">
-          <MenuItemSelectable label="Option" />
-        </Menu>
-      );
-
-      expect(spy).toHaveBeenCalled();
-      spy.mockRestore();
-    });
-
-    it('warns about MenuItemRadioGroup in basic mode', () => {
-      const spy = jest.spyOn(console, 'warn').mockImplementation(() => {});
-
-      render(
-        <Menu open mode="basic">
-          <MenuItemRadioGroup
-            label="Options"
-            items={['Option 1', 'Option 2']}
-          />
-        </Menu>
-      );
-
-      expect(spy).toHaveBeenCalled();
-      spy.mockRestore();
-    });
   });
 
   it('should call MenuItemRadioGroup onChange once', async () => {

packages/react/src/components/Menu/Menu.stories.js

@@ -8,6 +8,16 @@
 import React from 'react';
 import { action } from '@storybook/addon-actions';
 
+import {
+  Copy,
+  Cut,
+  FolderShared,
+  Paste,
+  TextBold,
+  TextItalic,
+  TrashCan,
+} from '@carbon/icons-react';
+
 import {
   Menu,
   MenuItem,
@@ -49,7 +59,7 @@ export const Default = (args) => {
 
   return (
     <Menu {...args} target={target} x={document?.dir === 'rtl' ? 250 : 0}>
-      <MenuItem label="Share with">
+      <MenuItem label="Share with" renderIcon={FolderShared}>
         <MenuItemRadioGroup
           label="Share with"
           items={['None', 'Product team', 'Organization', 'Company']}
@@ -58,17 +68,40 @@ export const Default = (args) => {
         />
       </MenuItem>
       <MenuItemDivider />
-      <MenuItem label="Cut" shortcut="⌘X" onClick={itemOnClick} />
-      <MenuItem label="Copy" shortcut="⌘C" onClick={itemOnClick} />
-      <MenuItem label="Paste" shortcut="⌘V" disabled onClick={itemOnClick} />
+      <MenuItem
+        label="Cut"
+        shortcut="⌘X"
+        onClick={itemOnClick}
+        renderIcon={Cut}
+      />
+      <MenuItem
+        label="Copy"
+        shortcut="⌘C"
+        onClick={itemOnClick}
+        renderIcon={Copy}
+      />
+      <MenuItem
+        label="Paste"
+        shortcut="⌘V"
+        disabled
+        onClick={itemOnClick}
+        renderIcon={Paste}
+      />
       <MenuItemDivider />
       <MenuItemGroup label="Font style">
         <MenuItemSelectable
           label="Bold"
+          shortcut="⌘B"
           defaultSelected
           onChange={selectableOnChange}
+          renderIcon={TextBold}
+        />
+        <MenuItemSelectable
+          label="Italic"
+          shortcut="⌘I"
+          onChange={selectableOnChange}
+          renderIcon={TextItalic}
         />
-        <MenuItemSelectable label="Italic" onChange={selectableOnChange} />
       </MenuItemGroup>
       <MenuItemDivider />
       <MenuItemRadioGroup
@@ -83,6 +116,7 @@ export const Default = (args) => {
         shortcut="⌫"
         kind="danger"
         onClick={itemOnClick}
+        renderIcon={TrashCan}
       />
     </Menu>
   );

packages/react/src/components/Menu/Menu.tsx

@@ -23,7 +23,7 @@ import { createPortal } from 'react-dom';
 import { keys, match } from '../../internal/keyboard';
 import { useMergedRefs } from '../../internal/useMergedRefs';
 import { usePrefix } from '../../internal/usePrefix';
-import { warning } from '../../internal/warning.js';
+import deprecate from '../../prop-types/deprecate';
 
 import { MenuContext, menuReducer } from './MenuContext';
 import { useLayoutDirection } from '../LayoutDirection';
@@ -57,6 +57,7 @@ export interface MenuProps extends React.HTMLAttributes<HTMLUListElement> {
   menuAlignment?: string;
 
   /**
+   * @deprecated Menus now always support both icons as well as selectable items and nesting.
    * The mode of this menu. Defaults to full.
    * `full` supports nesting and selectable menu items, but no icons.
    * `basic` supports icons but no nesting or selectable menu items.
@@ -110,7 +111,7 @@ const Menu = forwardRef<HTMLUListElement, MenuProps>(function Menu(
     containerRef,
     label,
     menuAlignment,
-    mode = 'full',
+    mode,
     onClose,
     onOpen,
     open,
@@ -132,19 +133,11 @@ const Menu = forwardRef<HTMLUListElement, MenuProps>(function Menu(
 
   const isRoot = context.state.isRoot;
 
-  if (context.state.mode === 'basic' && !isRoot) {
-    warning(
-      false,
-      'Nested menus are not supported when the menu is in "basic" mode.'
-    );
-  }
-
   const menuSize = isRoot ? size : context.state.size;
 
   const [childState, childDispatch] = useReducer(menuReducer, {
     ...context.state,
     isRoot: false,
-    mode,
     size,
     requestCloseRoot: isRoot ? handleClose : context.state.requestCloseRoot,
   });
@@ -424,6 +417,8 @@ const Menu = forwardRef<HTMLUListElement, MenuProps>(function Menu(
       [`${prefix}--menu--shown`]:
         (open && !legacyAutoalign) || (position[0] >= 0 && position[1] >= 0),
       [`${prefix}--menu--with-icons`]: childContext.state.hasIcons,
+      [`${prefix}--menu--with-selectable-items`]:
+        childContext.state.hasSelectableItems,
       [`${prefix}--autoalign`]: !legacyAutoalign,
     }
   );
@@ -474,13 +469,17 @@ Menu.propTypes = {
   menuAlignment: PropTypes.string,
 
   /**
+   * **Deprecated**: Menus now always support both icons as well as selectable items and nesting.
    * The mode of this menu. Defaults to full.
    * `full` supports nesting and selectable menu items, but no icons.
    * `basic` supports icons but no nesting or selectable menu items.
    *
    * **This prop is not intended for use and will be set by the respective implementation (like useContextMenu, MenuButton, and ComboButton).**
    */
-  mode: PropTypes.oneOf(['full', 'basic']),
+  mode: deprecate(
+    PropTypes.oneOf(['full', 'basic']),
+    'Menus now always support both icons as well as selectable items and nesting.'
+  ),
 
   /**
    * Provide an optional function to be called when the Menu should be closed.

packages/react/src/components/Menu/MenuContext.ts

@@ -8,23 +8,23 @@
 import { createContext, KeyboardEvent, RefObject } from 'react';
 
 type ActionType = {
-  type: 'enableIcons' | 'registerItem';
+  type: 'enableIcons' | 'enableSelectableItems' | 'registerItem';
   payload: any;
 };
 
 type StateType = {
   isRoot: boolean;
-  mode: 'full' | 'basic';
   hasIcons: boolean;
+  hasSelectableItems: boolean;
   size: 'xs' | 'sm' | 'md' | 'lg' | null;
   items: any[];
   requestCloseRoot: (e: Pick<KeyboardEvent<HTMLUListElement>, 'type'>) => void;
 };
 
 const menuDefaultState: StateType = {
   isRoot: true,
-  mode: 'full',
   hasIcons: false,
+  hasSelectableItems: false,
   size: null,
   items: [],
   requestCloseRoot: () => {},
@@ -37,6 +37,11 @@ function menuReducer(state: StateType, action: ActionType) {
         ...state,
         hasIcons: true,
       };
+    case 'enableSelectableItems':
+      return {
+        ...state,
+        hasSelectableItems: true,
+      };
     case 'registerItem':
       return {
         ...state,
@@ -48,7 +53,7 @@ function menuReducer(state: StateType, action: ActionType) {
 }
 
 type DispatchFuncProps = {
-  type: 'registerItem' | 'enableIcons';
+  type: ActionType['type'];
   payload: {
     ref: RefObject<HTMLLIElement>;
     disabled: boolean;