Render MenuItem without item or submenu (#989)

* Render MenuItem without item or submenu * Add test * Update changelog
iTwin · Sep 3, 2024 · 71f2d73 · 71f2d73
1 parent f249f16
commit 71f2d73
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 58 deletions.
diff --git a/common/changes/@itwin/appui-react/fix-menu-item-rendering_2024-08-29-06-11.json b/common/changes/@itwin/appui-react/fix-menu-item-rendering_2024-08-29-06-11.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/appui-react",
+      "comment": "Render `MenuItem` without `item` or `submenu` props.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/appui-react"
+}
diff --git a/docs/changehistory/NextVersion.md b/docs/changehistory/NextVersion.md
@@ -4,9 +4,14 @@ Table of contents:
 
 - [@itwin/appui-react](#itwinappui-react)
   - [Changes](#changes)
+  - [Fixes](#fixes)
 
 ## @itwin/appui-react
 
 ### Changes
 
 - Allow to set the available snap modes in `SnapModeField` component. [#974](https://github.com/iTwin/appui/pull/974)
+
+### Fixes
+
+- Render `MenuItem` without `item` or `submenu` props. [#989](https://github.com/iTwin/appui/pull/989)
diff --git a/test-apps/appui-test-app/appui-test-providers/src/ui/providers/ComponentExamplesProvider.tsx b/test-apps/appui-test-app/appui-test-providers/src/ui/providers/ComponentExamplesProvider.tsx
@@ -25,7 +25,8 @@ import {
   ContentLayout,
   ContentLayoutDef,
   CursorInformation,
-  CursorMenuData,
+  CursorMenuItemProps,
+  CursorMenuPayload,
   CursorPopupContent,
   CursorPopupManager,
   CustomItemDef,
@@ -40,7 +41,6 @@ import {
   ListPicker,
   ListPickerItem,
   MenuButton,
-  MenuItemProps,
   MessageCenterField,
   MessageManager,
   ModelessDialog,
@@ -525,18 +525,18 @@ export class ComponentExamplesProvider {
       CursorInformation.getRelativePositionFromCursorDirection(
         CursorInformation.cursorDirection
       );
-    const menuItems: MenuItemProps[] = [
-      { id: "menuItem1", item: { label: "Menu Item 1" } },
+    const menuItems: CursorMenuItemProps[] = [
+      { id: "menuItem1", label: "Menu Item 1", execute: () => {} },
       {
         id: "menuItem2",
         label: "Menu Item 2",
         submenu: [
-          { id: "submenuItem1", item: { label: "Submenu Item 1" } },
-          { id: "submenuItem2", item: { label: "Submenu Item 2" } },
+          { id: "submenuItem1", label: "Submenu Item 1", execute: () => {} },
+          { id: "submenuItem2", label: "Submenu Item 2", execute: () => {} },
         ],
       },
     ];
-    let menuData: CursorMenuData;
+    let menuPayload: CursorMenuPayload;
 
     // TODO: Figure out a way to change zIndex of cursor popup without changing styling in package. Without zIndex being set to at least 14000, CursorPopup, appears behind Component Examples frontstage modal
     function openCursorPopup() {
@@ -576,13 +576,13 @@ export class ComponentExamplesProvider {
           undefined,
           <Button
             onMouseDown={(e) => {
-              menuData = {
+              menuPayload = {
                 items: menuItems,
                 position: { x: e.clientX, y: e.clientY },
               };
             }}
             onClick={() => {
-              UiFramework.openCursorMenu(menuData);
+              UiFramework.openCursorMenu(menuPayload);
             }}
           >
             Open Cursor Popup Menu

diff --git a/ui/appui-react/src/appui-react/shared/MenuItem.tsx b/ui/appui-react/src/appui-react/shared/MenuItem.tsx
@@ -40,14 +40,14 @@ export interface CursorMenuItemProps extends CommonProps {
    */
   // eslint-disable-next-line deprecation/deprecation
   iconSpec?: IconSpec;
-  /** The item to execute when this item is invoked. Either 'item' or 'submenu' must be specified.
+  /** The item to execute when this item is invoked. Either 'item', 'submenu' or `execute` must be specified.
    * @deprecated in 4.15.0. Use properties of this object instead.
    */
   // eslint-disable-next-line deprecation/deprecation
   item?: CommandItemProps;
   /** Function to execute. */
   execute?: () => any;
-  /** Nested array of item props. Either 'item' or 'submenu' must be specified. */
+  /** Nested array of item props. Either 'item', 'submenu' or 'execute' must be specified. */
   submenu?: CursorMenuItemProps[];
   /** Icon to display on right side of the menu item.
    * Name of icon WebFont entry or if specifying an imported SVG symbol use "webSvg:" prefix  to imported symbol Id.
@@ -205,7 +205,6 @@ export class MenuItemHelpers {
     item: MenuItem,
     index: number
   ): React.ReactNode {
-    let node: React.ReactNode = null;
     const label = item.label;
     const iconSpec = item.iconSpec;
     const iconRightSpec = item.iconRightSpec;
@@ -215,40 +214,35 @@ export class MenuItemHelpers {
       item.isDisabled
     );
 
-    if (item.actionItem) {
-      const sel = () => item.itemPicked();
-      node = (
-        <ContextMenuItem
+    if (item.submenu && item.submenu.length > 0) {
+      const items = this.createMenuItemNodes(item.submenu);
+
+      return (
+        <ContextSubMenu
           key={index}
-          onSelect={sel}
           icon={iconSpec}
-          iconRight={iconRightSpec}
+          label={label}
           badgeType={badgeType}
           badgeKind={badgeKind}
           disabled={isDisabled}
         >
-          {label}
-        </ContextMenuItem>
+          {items}
+        </ContextSubMenu>
       );
-    } else {
-      if (item.submenu && item.submenu.length > 0) {
-        const items = this.createMenuItemNodes(item.submenu);
-
-        node = (
-          <ContextSubMenu
-            key={index}
-            icon={iconSpec}
-            label={label}
-            badgeType={badgeType}
-            badgeKind={badgeKind}
-            disabled={isDisabled}
-          >
-            {items}
-          </ContextSubMenu>
-        );
-      }
     }
 
-    return node;
+    return (
+      <ContextMenuItem
+        key={index}
+        onSelect={() => item.itemPicked()}
+        icon={iconSpec}
+        iconRight={iconRightSpec}
+        badgeType={badgeType}
+        badgeKind={badgeKind}
+        disabled={isDisabled}
+      >
+        {label}
+      </ContextMenuItem>
+    );
   }
 }
diff --git a/ui/appui-react/src/test/shared/MenuItem.test.tsx b/ui/appui-react/src/test/shared/MenuItem.test.tsx
@@ -3,7 +3,7 @@
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
 import * as React from "react";
-import { render, screen } from "@testing-library/react";
+import { render } from "@testing-library/react";
 import { BadgeType } from "@itwin/core-react";
 import type { CursorMenuItemProps } from "../../appui-react/shared/MenuItem";
 import { MenuItem, MenuItemHelpers } from "../../appui-react/shared/MenuItem";
@@ -82,11 +82,11 @@ describe("MenuItem", () => {
       new MenuItem({
         id: "test",
       });
-    }).to.throw(Error);
+    }).toThrow(Error);
   });
 
   it("createMenuItems should create a valid MenuItem", () => {
-    const CursorMenuItemProps: CursorMenuItemProps[] = [
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
       {
         id: "test",
         item: {
@@ -97,7 +97,7 @@ describe("MenuItem", () => {
       },
     ];
 
-    const menuItems = MenuItemHelpers.createMenuItems(CursorMenuItemProps);
+    const menuItems = MenuItemHelpers.createMenuItems(cursorMenuItemProps);
 
     expect(menuItems.length).toEqual(1);
 
@@ -110,7 +110,7 @@ describe("MenuItem", () => {
   });
 
   it("createMenuItems should create a valid submenu", () => {
-    const CursorMenuItemProps: CursorMenuItemProps[] = [
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
       {
         id: "test",
         label: "test label",
@@ -136,7 +136,7 @@ describe("MenuItem", () => {
       },
     ];
 
-    const menuItems = MenuItemHelpers.createMenuItems(CursorMenuItemProps);
+    const menuItems = MenuItemHelpers.createMenuItems(cursorMenuItemProps);
 
     expect(menuItems.length).toEqual(1);
 
@@ -148,7 +148,7 @@ describe("MenuItem", () => {
   });
 
   it("createMenuItemNodes should create a valid MenuItem", () => {
-    const CursorMenuItemProps: CursorMenuItemProps[] = [
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
       {
         id: "test",
         badgeType: BadgeType.New,
@@ -162,15 +162,15 @@ describe("MenuItem", () => {
       },
     ];
 
-    const menuItems = MenuItemHelpers.createMenuItems(CursorMenuItemProps);
+    const menuItems = MenuItemHelpers.createMenuItems(cursorMenuItemProps);
     expect(menuItems.length).toEqual(1);
 
     const menuItemNodes = MenuItemHelpers.createMenuItemNodes(menuItems);
     expect(menuItemNodes.length).toEqual(1);
 
-    render(<div>{menuItemNodes}</div>);
+    const component = render(<div>{menuItemNodes}</div>);
 
-    expect(screen.getByRole("menuitem"))
+    expect(component.getByRole("menuitem"))
       .satisfy(
         selectorMatches(".core-context-menu-item.core-context-menu-disabled")
       )
@@ -183,8 +183,42 @@ describe("MenuItem", () => {
       );
   });
 
+  it("createMenuItemNodes should create a valid MenuItem without item prop", () => {
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
+      {
+        id: "test",
+        badgeKind: "technical-preview",
+        isDisabled: true,
+        label: "test label",
+        icon: "icon-placeholder",
+        execute: () => {},
+        iconRight: "icon-checkmark",
+      },
+    ];
+
+    const menuItems = MenuItemHelpers.createMenuItems(cursorMenuItemProps);
+    expect(menuItems.length).toEqual(1);
+
+    const menuItemNodes = MenuItemHelpers.createMenuItemNodes(menuItems);
+    expect(menuItemNodes.length).toEqual(1);
+
+    const component = render(<div>{menuItemNodes}</div>);
+
+    expect(component.getByRole("menuitem"))
+      .satisfy(
+        selectorMatches(".core-context-menu-item.core-context-menu-disabled")
+      )
+      .satisfy(
+        childStructure([
+          ".core-context-menu-icon-right > .icon.icon-checkmark",
+          ".core-context-menu-item > .core-context-menu-icon > .icon.icon-placeholder",
+          ".core-context-menu-badge .core-badge-technicalPreviewBadge",
+        ])
+      );
+  });
+
   it("createMenuItemNodes abstract disabled item should create a disabled MenuItem", () => {
-    const CursorMenuItemProps: CursorMenuItemProps[] = [
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
       {
         id: "test",
         badgeType: BadgeType.New,
@@ -198,15 +232,15 @@ describe("MenuItem", () => {
       },
     ];
 
-    const menuItems = MenuItemHelpers.createMenuItems(CursorMenuItemProps);
+    const menuItems = MenuItemHelpers.createMenuItems(cursorMenuItemProps);
     expect(menuItems.length).toEqual(1);
 
     const menuItemNodes = MenuItemHelpers.createMenuItemNodes(menuItems);
     expect(menuItemNodes.length).toEqual(1);
 
-    render(<div>{menuItemNodes}</div>);
+    const component = render(<div>{menuItemNodes}</div>);
 
-    expect(screen.getByRole("menuitem")).satisfy(
+    expect(component.getByRole("menuitem")).satisfy(
       selectorMatches(".core-context-menu-item.core-context-menu-disabled")
     );
   });
@@ -215,7 +249,7 @@ describe("MenuItem", () => {
     const handleSelect = vi.fn();
     const handleSelect2 = vi.fn();
 
-    const CursorMenuItemProps: CursorMenuItemProps[] = [
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
       {
         id: "test",
         item: {
@@ -228,7 +262,7 @@ describe("MenuItem", () => {
     ];
 
     const menuItems = MenuItemHelpers.createMenuItems(
-      CursorMenuItemProps,
+      cursorMenuItemProps,
       handleSelect2
     );
     expect(menuItems.length).toEqual(1);
@@ -249,7 +283,7 @@ describe("MenuItem", () => {
   });
 
   it("createMenuItemNodes should create a valid submenu", () => {
-    const CursorMenuItemProps: CursorMenuItemProps[] = [
+    const cursorMenuItemProps: CursorMenuItemProps[] = [
       {
         id: "test",
         label: "test label",
@@ -275,15 +309,15 @@ describe("MenuItem", () => {
       },
     ];
 
-    const menuItems = MenuItemHelpers.createMenuItems(CursorMenuItemProps);
+    const menuItems = MenuItemHelpers.createMenuItems(cursorMenuItemProps);
     expect(menuItems.length).toEqual(1);
 
     const menuItemNodes = MenuItemHelpers.createMenuItemNodes(menuItems);
     expect(menuItemNodes.length).toEqual(1);
 
-    render(<div>{menuItemNodes}</div>);
+    const component = render(<div>{menuItemNodes}</div>);
 
-    expect(screen.getByText("Mode 2")).to.satisfy(
+    expect(component.getByText("Mode 2")).toSatisfy(
       selectorMatches(
         ".core-context-submenu .core-context-submenu-popup .core-context-menu-container .core-context-menu-content"
       )