refactor(MenuItem): separate MenuIcon logic from MenuItem

packages/core/src/components/Menu/MenuItem/MenuItem.module.scss

@@ -15,51 +15,6 @@
   outline: none;
 }
 
-.iconWrapper {
-  margin-right: var(--spacing-small);
-}
-
-.subMenuIconWrapper {
-  margin-left: var(--spacing-xs);
-}
-
-.subMenuIconWrapper,
-.iconWrapper {
-  display: flex;
-  align-items: center;
-  justify-content: center;
-}
-
-.subMenuIconWrapper .subMenuIcon,
-.subMenuIconWrapper .icon,
-.iconWrapper .subMenuIcon,
-.iconWrapper .icon {
-  color: var(--icon-color);
-}
-
-.subMenuIconWrapper .divider {
-  height: 19px;
-}
-
-.subMenuIconWrapper .splitMenuItemIconButton {
-  width: 34px;
-  height: 30px;
-
-  // TODO Temp override for Tal changes - remove once token is changed
-  --primary-background-hover-color: rgba(103, 104, 121, 0.1);
-  // Override for IconButton active color
-  --primary-selected-color: rgba(103, 104, 121, 0.1);
-}
-
-.iconButton {
-  width: 18px;
-  height: 18px;
-
-  &:not(.disabled) {
-    color: var(--icon-color);
-  }
-}
-
 .item.splitMenuItem {
   padding: 0 0 0 var(--spacing-small);
 }
@@ -69,12 +24,6 @@
   color: var(--disabled-text-color);
 }
 
-.item.disabled .subMenuIcon,
-.item.disabled .icon {
-  cursor: not-allowed;
-  color: var(--disabled-text-color);
-}
-
 .item:not(.disabled).focused {
   background-color: var(--primary-background-hover-color);
 }
@@ -92,10 +41,6 @@
   border-radius: var(--border-radius-small);
 }
 
-.item:not(.disabled).selected .iconWrapper .icon {
-  color: var(--primary-color);
-}
-
 .title {
   white-space: nowrap;
   overflow: hidden;

packages/core/src/components/Menu/MenuItem/MenuItem.tsx

@@ -11,7 +11,6 @@ import React, {
   useRef
 } from "react";
 import cx from "classnames";
-import { isFunction } from "lodash-es";
 import { ComponentDefaultTestId, getTestId } from "../../../tests/test-ids-utils";
 import { DialogPosition } from "../../../constants/positions";
 import Text from "../../Text/Text";
@@ -30,11 +29,9 @@ import { TAB_INDEX_FOCUS_WITH_JS_ONLY, TooltipPosition } from "./MenuItemConstan
 import { CloseMenuOption, MenuChild } from "../Menu/MenuConstants";
 import Label from "../../Label/Label";
 import styles from "./MenuItem.module.scss";
-import { DropdownChevronRight } from "../../Icon/Icons";
-import IconButton from "../../IconButton/IconButton";
-import Divider from "../../Divider/Divider";
-import { DirectionType } from "../../Divider/DividerConstants";
 import useIsMouseEnter from "../../../hooks/useIsMouseEnter";
+import MenuItemIcon from "./components/MenuItemIcon/MenuItemIcon";
+import MenuItemSubMenuIcon from "./components/MenuItemSubMenuIcon/MenuItemSubMenuIcon";
 
 export interface MenuItemProps extends VibeComponentProps {
   title?: string;
@@ -239,74 +236,6 @@ const MenuItem: VibeComponent<MenuItemProps | MenuItemTitleComponentProps> & {
 
     // if "title" is a component ariaLabel is mandatory
     const iconLabel = ariaLabel ?? (title as string);
-    const renderSubMenuIconIfNeeded = () => {
-      if (!hasChildren) return null;
-
-      return splitMenuItem ? (
-        <div className={styles.subMenuIconWrapper}>
-          <Divider direction={DirectionType.VERTICAL} className={styles.divider} />
-          <IconButton
-            icon={DropdownChevronRight}
-            className={styles.splitMenuItemIconButton}
-            kind={IconButton.kinds.TERTIARY}
-            size={null} // Customizing size via className
-            iconClassName={cx(styles.iconButton, { [styles.disabled]: disabled })}
-            tabIndex={-1}
-            ref={iconButtonElementRef}
-            active={shouldShowSubMenu}
-            disabled={disabled}
-          />
-        </div>
-      ) : (
-        <div className={styles.subMenuIconWrapper}>
-          <Icon
-            clickable={false}
-            icon={DropdownChevronRight}
-            iconLabel={iconLabel}
-            className={styles.subMenuIcon}
-            ignoreFocusStyle
-            iconSize={18}
-          />
-        </div>
-      );
-    };
-
-    const [iconWrapperStyle, iconStyle] = useMemo(() => {
-      return iconBackgroundColor
-        ? [
-            {
-              backgroundColor: iconBackgroundColor,
-              borderRadius: "4px",
-              opacity: disabled ? 0.4 : 1
-            },
-            { color: "var(--text-color-on-primary)" }
-          ]
-        : [];
-    }, [iconBackgroundColor, disabled]);
-
-    const renderMenuItemIconIfNeeded = () => {
-      if (!icon) return null;
-
-      let finalIconType = iconType;
-      if (!finalIconType) {
-        finalIconType = isFunction(icon) ? Icon.type.SVG : Icon.type.ICON_FONT;
-      }
-
-      return (
-        <div className={cx(styles.iconWrapper, iconWrapperClassName)} style={iconWrapperStyle}>
-          <Icon
-            iconType={finalIconType}
-            clickable={false}
-            icon={icon}
-            iconLabel={iconLabel}
-            className={styles.icon}
-            ignoreFocusStyle
-            style={iconStyle}
-            iconSize={18}
-          />
-        </div>
-      );
-    };
 
     const a11yProps = useMemo(() => {
       if (!children) return {};
@@ -348,7 +277,17 @@ const MenuItem: VibeComponent<MenuItemProps | MenuItemTitleComponentProps> & {
         onMouseEnter={onMouseEnter}
         tabIndex={TAB_INDEX_FOCUS_WITH_JS_ONLY}
       >
-        {renderMenuItemIconIfNeeded()}
+        {!!icon && (
+          <MenuItemIcon
+            icon={icon}
+            type={iconType}
+            label={iconLabel}
+            disabled={disabled}
+            selected={selected}
+            backgroundColor={iconBackgroundColor}
+            wrapperClassName={iconWrapperClassName}
+          />
+        )}
         <Tooltip
           content={shouldShowTooltip ? finalTooltipContent : null}
           position={tooltipPosition}
@@ -364,7 +303,15 @@ const MenuItem: VibeComponent<MenuItemProps | MenuItemTitleComponentProps> & {
           </div>
         </Tooltip>
         {label && <Label kind={Label.kinds.LINE} text={label} />}
-        {renderSubMenuIconIfNeeded()}
+        {hasChildren && (
+          <MenuItemSubMenuIcon
+            ref={iconButtonElementRef}
+            isSplit={splitMenuItem}
+            label={iconLabel}
+            active={shouldShowSubMenu}
+            disabled={disabled}
+          />
+        )}
         <div
           style={{ ...popoverStyles.popper, visibility: shouldShowSubMenu ? "visible" : "hidden" }}
           // eslint-disable-next-line react/jsx-props-no-spreading

packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.mdx

@@ -1,4 +1,4 @@
-import { Canvas, Meta } from "@storybook/blocks";
+import { Meta } from "@storybook/blocks";
 import { ComponentRuleWithLabelDo, ComponentRuleWithLabelDont } from "./MenuItem.stories.helpers";
 import * as MenuItemStories from "./MenuItem.stories";
 

packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx

@@ -6,6 +6,9 @@ import { Activity } from "../../../Icon/Icons";
 import Icon from "../../../Icon/Icon";
 import Tooltip from "../../../Tooltip/Tooltip";
 import { createStoryMetaSettingsDecorator } from "../../../../storybook/functions/createStoryMetaSettingsDecorator";
+import { Meta, StoryObj } from "@storybook/react";
+
+type Story = StoryObj<MenuItemProps>;
 
 const metaSettings = createStoryMetaSettingsDecorator({
   component: MenuItem,
@@ -18,60 +21,68 @@ export default {
   component: MenuItem,
   argTypes: metaSettings.argTypes,
   decorators: metaSettings.decorators
-};
+} satisfies Meta<typeof MenuItem>;
 
 const menuItemTemplate = (args: MenuItemProps) => (
   <Menu>
     <MenuItem {...args} />
   </Menu>
 );
 
-export const Overview = {
+export const Overview: Story = {
   render: menuItemTemplate.bind({}),
-  name: "Overview",
-
   args: {
     title: "Menu item"
+  },
+  parameters: {
+    docs: {
+      liveEdit: {
+        isEnabled: false
+      }
+    }
   }
 };
 
-export const States = {
+export const States: Story = {
   render: () => (
     <Menu>
       <MenuItem title="Regular menu item" />
       <MenuItem title="Selected menu item" selected />
       <MenuItem title="Disabled menu item" disabled />
     </Menu>
-  ),
-  name: "States"
+  )
 };
 
-export const Icons = {
+export const Icons: Story = {
   render: () => (
     <Menu>
       <MenuItem title="SVG icon" icon={Activity} />
       <MenuItem title="Font icon" icon="fa fa-star" iconType={MenuItem.iconType.ICON_FONT} />
     </Menu>
   ),
-  name: "Icons"
+  parameters: {
+    docs: {
+      liveEdit: {
+        scope: { Activity }
+      }
+    }
+  }
 };
 
-export const Label = {
+export const Label: Story = {
   render: () => (
     <Menu>
       <MenuItem title="Menu item" label="New" />
     </Menu>
   ),
-  name: "Label",
-
   parameters: {
     chromatic: {
       pauseAnimationAtEnd: true
     }
   }
 };
 
-export const SubMenu = {
+export const SubMenu: Story = {
   render: () => (
     <Menu>
       <MenuItem title="Opens on item hover">
@@ -81,7 +92,7 @@ export const SubMenu = {
           <MenuItem title="Sub menu item 3" onClick={() => alert("clicked on sub menu item 3")} />
         </Menu>
       </MenuItem>
-      <MenuItem title="Opens on icon hover" splitMenuItem={true} onClick={() => alert("clicked on menu item")}>
+      <MenuItem title="Opens on icon hover" splitMenuItem onClick={() => alert("clicked on menu item")}>
         <Menu tabIndex={0} id="sub-menu">
           <MenuItem title="Sub menu item 1" onClick={() => alert("clicked on sub menu item 1")} />
           <MenuItem title="Sub menu item 2" onClick={() => alert("clicked on sub menu item 2")} />
@@ -93,7 +104,7 @@ export const SubMenu = {
   name: "Sub menu"
 };
 
-export const Overflow = {
+export const Overflow: Story = {
   render: () => (
     <Menu>
       <MenuItem title="short text" />
@@ -106,11 +117,10 @@ export const Overflow = {
         </Menu>
       </MenuItem>
     </Menu>
-  ),
-  name: "Overflow"
+  )
 };
 
-export const TooltipStory = {
+export const TooltipStory: Story = {
   render: () => (
     <Menu>
       <MenuItem title="Menu item with tooltip" tooltipContent="I am tooltip" />
@@ -130,5 +140,12 @@ export const TooltipStory = {
       />
     </Menu>
   ),
-  name: "Tooltip"
+  name: "Tooltip",
+  parameters: {
+    docs: {
+      liveEdit: {
+        scope: { Activity }
+      }
+    }
+  }
 };

packages/core/src/components/Menu/MenuItem/__tests__/__snapshots__/menuItem-snapshots.jest.js.snap

@@ -203,7 +203,12 @@ exports[`Snapshots renders correctly with title and SVG icon 1`] = `
   tabIndex={-1}
 >
   <div
-    className="iconWrapper"
+    className="container directionRow justifyCenter alignCenter iconWrapper"
+    style={
+      Object {
+        "gap": "0px",
+      }
+    }
   >
     <svg
       aria-hidden={true}
@@ -260,7 +265,12 @@ exports[`Snapshots renders correctly with title and font icon 1`] = `
   tabIndex={-1}
 >
   <div
-    className="iconWrapper"
+    className="container directionRow justifyCenter alignCenter iconWrapper"
+    style={
+      Object {
+        "gap": "0px",
+      }
+    }
   >
     <span
       aria-hidden={true}