From c30bac946148a3205770be47c467552eb9bff049 Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Thu, 2 May 2024 17:58:34 +0300
Subject: [PATCH 1/9] refactor(MenuItem): separate SubMenuIcon logic from
 MenuItem

---
 .../Menu/MenuItem/MenuItem.module.scss        | 37 +--------------
 .../src/components/Menu/MenuItem/MenuItem.tsx | 46 ++++---------------
 .../MenuItemSubMenuIcon.module.scss           | 32 +++++++++++++
 .../MenuItemSubMenuIcon.tsx                   | 45 ++++++++++++++++++
 .../MenuItemSubMenuIcon.types.ts              |  6 +++
 5 files changed, 94 insertions(+), 72 deletions(-)
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.module.scss
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts

diff --git a/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss b/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss
index e6735007c1..def371db39 100644
--- a/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss
+++ b/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss
@@ -17,45 +17,11 @@
 
 .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) {
+  .icon {
     color: var(--icon-color);
   }
 }
@@ -69,7 +35,6 @@
   color: var(--disabled-text-color);
 }
 
-.item.disabled .subMenuIcon,
 .item.disabled .icon {
   cursor: not-allowed;
   color: var(--disabled-text-color);
diff --git a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
index 4600e158be..f8f9249831 100644
--- a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
+++ b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
@@ -30,11 +30,8 @@ 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 MenuItemSubMenuIcon from "./components/MenuItemSubMenuIcon/MenuItemSubMenuIcon";
 
 export interface MenuItemProps extends VibeComponentProps {
   title?: string;
@@ -239,37 +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
@@ -364,7 +330,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
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.module.scss b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.module.scss
new file mode 100644
index 0000000000..97f5b1a945
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.module.scss
@@ -0,0 +1,32 @@
+.subMenuIconWrapper {
+  margin-left: var(--spacing-xs);
+
+  .divider {
+    height: 19px;
+  }
+
+  .splitSubMenuIcon {
+    width: 18px;
+    height: 18px;
+
+    &.disabled {
+      cursor: not-allowed;
+      color: var(--disabled-text-color);
+    }
+  }
+
+  .subMenuIcon,
+  .splitSubMenuIcon {
+    color: var(--icon-color);
+  }
+
+  .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);
+  }
+}
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
new file mode 100644
index 0000000000..3e75acc8c6
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
@@ -0,0 +1,45 @@
+import React, { forwardRef } from "react";
+import cx from "classnames";
+import Divider from "../../../../Divider/Divider";
+import Icon from "../../../../Icon/Icon";
+import Flex from "../../../../Flex/Flex";
+import IconButton from "../../../../IconButton/IconButton";
+import { DropdownChevronRight } from "../../../../Icon/Icons";
+import styles from "./MenuItemSubMenuIcon.module.scss";
+import { MenuItemSubMenuIconProps } from "./MenuItemSubMenuIcon.types";
+
+const MenuItemSubMenuIcon = forwardRef(
+  ({ isSplit, label, active, disabled }: MenuItemSubMenuIconProps, ref: React.ForwardedRef<HTMLDivElement>) => (
+    <Flex justify={Flex.justify.CENTER} className={styles.subMenuIconWrapper}>
+      {isSplit ? (
+        <>
+          <Divider direction={Divider.directions.VERTICAL} className={styles.divider} />
+          <IconButton
+            icon={DropdownChevronRight}
+            className={styles.splitMenuItemIconButton}
+            kind={IconButton.kinds.TERTIARY}
+            size={null} // Customizing size via className
+            iconClassName={cx(styles.splitSubMenuIcon, { [styles.disabled]: disabled })}
+            tabIndex={-1}
+            ref={ref}
+            active={active}
+            disabled={disabled}
+          />
+        </>
+      ) : (
+        <>
+          <Icon
+            clickable={false}
+            icon={DropdownChevronRight}
+            iconLabel={label}
+            className={styles.subMenuIcon}
+            ignoreFocusStyle
+            iconSize={18}
+          />
+        </>
+      )}
+    </Flex>
+  )
+);
+
+export default MenuItemSubMenuIcon;
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts
new file mode 100644
index 0000000000..95bcbbbbac
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts
@@ -0,0 +1,6 @@
+export interface MenuItemSubMenuIconProps {
+  isSplit?: boolean;
+  label?: string;
+  active?: boolean;
+  disabled?: boolean;
+}

From a09e44e065987bf56237662804762798ad63a2b7 Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Thu, 2 May 2024 18:23:27 +0300
Subject: [PATCH 2/9] docs(MenuItem): allow LiveEdit

---
 .../Menu/MenuItem/__stories__/MenuItem.mdx    |  2 +-
 .../MenuItem/__stories__/MenuItem.stories.tsx | 24 +++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.mdx b/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.mdx
index 1f1456d15d..9f982bcf71 100644
--- a/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.mdx
+++ b/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";
 
diff --git a/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx b/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx
index d9017462a8..ea323c9225 100644
--- a/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx
+++ b/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx
@@ -32,6 +32,13 @@ export const Overview = {
 
   args: {
     title: "Menu item"
+  },
+  parameters: {
+    docs: {
+      liveEdit: {
+        isEnabled: false
+      }
+    }
   }
 };
 
@@ -53,7 +60,13 @@ export const Icons = {
       <MenuItem title="Font icon" icon="fa fa-star" iconType={MenuItem.iconType.ICON_FONT} />
     </Menu>
   ),
-  name: "Icons"
+  parameters: {
+    docs: {
+      liveEdit: {
+        scope: { Activity }
+      }
+    }
+  }
 };
 
 export const Label = {
@@ -130,5 +143,12 @@ export const TooltipStory = {
       />
     </Menu>
   ),
-  name: "Tooltip"
+  name: "Tooltip",
+  parameters: {
+    docs: {
+      liveEdit: {
+        scope: { Activity }
+      }
+    }
+  }
 };

From d99ba77fb9cc78d139be3fcba5d3a50406d6dd39 Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Thu, 2 May 2024 18:23:58 +0300
Subject: [PATCH 3/9] docs(MenuItem): align story's TS

---
 .../MenuItem/__stories__/MenuItem.stories.tsx | 31 +++++++++----------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx b/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx
index ea323c9225..ae2c1f5203 100644
--- a/packages/core/src/components/Menu/MenuItem/__stories__/MenuItem.stories.tsx
+++ b/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,7 +21,7 @@ export default {
   component: MenuItem,
   argTypes: metaSettings.argTypes,
   decorators: metaSettings.decorators
-};
+} satisfies Meta<typeof MenuItem>;
 
 const menuItemTemplate = (args: MenuItemProps) => (
   <Menu>
@@ -26,10 +29,8 @@ const menuItemTemplate = (args: MenuItemProps) => (
   </Menu>
 );
 
-export const Overview = {
+export const Overview: Story = {
   render: menuItemTemplate.bind({}),
-  name: "Overview",
-
   args: {
     title: "Menu item"
   },
@@ -42,18 +43,17 @@ export const Overview = {
   }
 };
 
-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} />
@@ -69,14 +69,12 @@ export const Icons = {
   }
 };
 
-export const Label = {
+export const Label: Story = {
   render: () => (
     <Menu>
       <MenuItem title="Menu item" label="New" />
     </Menu>
   ),
-  name: "Label",
-
   parameters: {
     chromatic: {
       pauseAnimationAtEnd: true
@@ -84,7 +82,7 @@ export const Label = {
   }
 };
 
-export const SubMenu = {
+export const SubMenu: Story = {
   render: () => (
     <Menu>
       <MenuItem title="Opens on item hover">
@@ -94,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")} />
@@ -106,7 +104,7 @@ export const SubMenu = {
   name: "Sub menu"
 };
 
-export const Overflow = {
+export const Overflow: Story = {
   render: () => (
     <Menu>
       <MenuItem title="short text" />
@@ -119,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" />

From 6552ad0e876d558eabce59bc0d1828619e978f15 Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Thu, 2 May 2024 18:24:47 +0300
Subject: [PATCH 4/9] refactor(MenuItem): no need for fragments

---
 .../MenuItemSubMenuIcon.tsx                    | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
index 3e75acc8c6..20ab68ef18 100644
--- a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
@@ -27,16 +27,14 @@ const MenuItemSubMenuIcon = forwardRef(
           />
         </>
       ) : (
-        <>
-          <Icon
-            clickable={false}
-            icon={DropdownChevronRight}
-            iconLabel={label}
-            className={styles.subMenuIcon}
-            ignoreFocusStyle
-            iconSize={18}
-          />
-        </>
+        <Icon
+          clickable={false}
+          icon={DropdownChevronRight}
+          iconLabel={label}
+          className={styles.subMenuIcon}
+          ignoreFocusStyle
+          iconSize={18}
+        />
       )}
     </Flex>
   )

From 688ba886129c83463a9730254e344efe366cae0a Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Sun, 5 May 2024 11:13:29 +0300
Subject: [PATCH 5/9] test(MenuItemSubMenuIcon): add tests

---
 .../__tests__/MenuItemSubMenuIcon.jest.tsx    | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/__tests__/MenuItemSubMenuIcon.jest.tsx

diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/__tests__/MenuItemSubMenuIcon.jest.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/__tests__/MenuItemSubMenuIcon.jest.tsx
new file mode 100644
index 0000000000..8a29b21deb
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/__tests__/MenuItemSubMenuIcon.jest.tsx
@@ -0,0 +1,37 @@
+import React from "react";
+import { render } from "@testing-library/react";
+import "@testing-library/jest-dom";
+import MenuItemSubMenuIcon from "../MenuItemSubMenuIcon";
+import { MenuItemSubMenuIconProps } from "../MenuItemSubMenuIcon.types";
+
+function renderMenuItemSubMenuIcon(props: Partial<MenuItemSubMenuIconProps> & { ref?: React.Ref<HTMLDivElement> }) {
+  return render(<MenuItemSubMenuIcon {...props} />);
+}
+
+describe("MenuItemSubMenuIcon", () => {
+  it("should render the split icon button when isSplit is true", () => {
+    const { getByRole } = renderMenuItemSubMenuIcon({ isSplit: true });
+    expect(getByRole("button")).toBeInTheDocument();
+  });
+
+  it("should render a simple icon when isSplit is false", () => {
+    const { queryByRole, getByTestId, getByLabelText } = renderMenuItemSubMenuIcon({
+      isSplit: false,
+      label: "SubMenu"
+    });
+    expect(queryByRole("button")).not.toBeInTheDocument();
+    expect(getByTestId("icon")).toBeInTheDocument();
+    expect(getByLabelText("SubMenu")).toBeInTheDocument();
+  });
+
+  it("should apply disabled styles when disabled is true", () => {
+    const { getByRole } = renderMenuItemSubMenuIcon({ isSplit: true, disabled: true });
+    expect(getByRole("button")).toHaveAttribute("aria-disabled", "true");
+  });
+
+  it("should forward ref to the button when isSplit is true", () => {
+    const ref = React.createRef<HTMLDivElement>();
+    renderMenuItemSubMenuIcon({ isSplit: true, ref });
+    expect(ref.current.tagName).toBe("BUTTON");
+  });
+});

From d7a01b29af6c440b10a73d534af383f17e1baccf Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Sun, 5 May 2024 12:00:28 +0300
Subject: [PATCH 6/9] refactor(MenuItemSubMenuIcon): allow to use props
 relative to isSplit is true or false

---
 .../MenuItemSubMenuIcon.tsx                   | 56 +++++++++----------
 .../MenuItemSubMenuIcon.types.ts              | 25 ++++++++-
 2 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
index 20ab68ef18..8a980b0ba7 100644
--- a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.tsx
@@ -8,36 +8,34 @@ import { DropdownChevronRight } from "../../../../Icon/Icons";
 import styles from "./MenuItemSubMenuIcon.module.scss";
 import { MenuItemSubMenuIconProps } from "./MenuItemSubMenuIcon.types";
 
-const MenuItemSubMenuIcon = forwardRef(
-  ({ isSplit, label, active, disabled }: MenuItemSubMenuIconProps, ref: React.ForwardedRef<HTMLDivElement>) => (
-    <Flex justify={Flex.justify.CENTER} className={styles.subMenuIconWrapper}>
-      {isSplit ? (
-        <>
-          <Divider direction={Divider.directions.VERTICAL} className={styles.divider} />
-          <IconButton
-            icon={DropdownChevronRight}
-            className={styles.splitMenuItemIconButton}
-            kind={IconButton.kinds.TERTIARY}
-            size={null} // Customizing size via className
-            iconClassName={cx(styles.splitSubMenuIcon, { [styles.disabled]: disabled })}
-            tabIndex={-1}
-            ref={ref}
-            active={active}
-            disabled={disabled}
-          />
-        </>
-      ) : (
-        <Icon
-          clickable={false}
+const MenuItemSubMenuIcon = forwardRef((props: MenuItemSubMenuIconProps, ref: React.ForwardedRef<HTMLDivElement>) => (
+  <Flex justify={Flex.justify.CENTER} className={styles.subMenuIconWrapper}>
+    {props.isSplit === true ? (
+      <>
+        <Divider direction={Divider.directions.VERTICAL} className={styles.divider} />
+        <IconButton
           icon={DropdownChevronRight}
-          iconLabel={label}
-          className={styles.subMenuIcon}
-          ignoreFocusStyle
-          iconSize={18}
+          className={styles.splitMenuItemIconButton}
+          kind={IconButton.kinds.TERTIARY}
+          size={null} // Customizing size via className
+          iconClassName={cx(styles.splitSubMenuIcon, { [styles.disabled]: props.disabled })}
+          tabIndex={-1}
+          ref={ref}
+          active={props.active}
+          disabled={props.disabled}
         />
-      )}
-    </Flex>
-  )
-);
+      </>
+    ) : (
+      <Icon
+        clickable={false}
+        icon={DropdownChevronRight}
+        iconLabel={props.label}
+        className={styles.subMenuIcon}
+        ignoreFocusStyle
+        iconSize={18}
+      />
+    )}
+  </Flex>
+));
 
 export default MenuItemSubMenuIcon;
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts
index 95bcbbbbac..9eb4399f13 100644
--- a/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemSubMenuIcon/MenuItemSubMenuIcon.types.ts
@@ -1,6 +1,25 @@
-export interface MenuItemSubMenuIconProps {
-  isSplit?: boolean;
-  label?: string;
+export interface SplitMenuItemSubMenuIconProps {
+  /**
+   * Determines whether the submenu icon is split from the main menu item.
+   * When true, clicking the main menu item and the submenu icon trigger different actions.
+   */
+  isSplit?: true;
+  /**
+   * Indicates if the split submenu is currently active.
+   */
   active?: boolean;
+  /**
+   * Whether the split submenu icon is disabled.
+   */
   disabled?: boolean;
 }
+
+export interface SimpleMenuItemSubMenuIconProps {
+  isSplit?: false;
+  /**
+   * Label for the submenu icon, used for accessibility.
+   */
+  label?: string;
+}
+
+export type MenuItemSubMenuIconProps = SimpleMenuItemSubMenuIconProps | SplitMenuItemSubMenuIconProps;

From 75ba1c339cf0e92f367e175acd9b3c42a0e9fa36 Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Sun, 5 May 2024 12:25:42 +0300
Subject: [PATCH 7/9] refactor(MenuItem): separate MenuIcon logic from MenuItem

---
 .../Menu/MenuItem/MenuItem.module.scss        | 20 --------
 .../src/components/Menu/MenuItem/MenuItem.tsx | 51 +++++--------------
 .../MenuItemIcon/MenuItemIcon.module.scss     | 28 ++++++++++
 .../components/MenuItemIcon/MenuItemIcon.tsx  | 41 +++++++++++++++
 .../MenuItemIcon/MenuItemIcon.types.ts        | 33 ++++++++++++
 .../__tests__/MenuItemIcon.jest.tsx           | 42 +++++++++++++++
 6 files changed, 156 insertions(+), 59 deletions(-)
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.module.scss
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.types.ts
 create mode 100644 packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/__tests__/MenuItemIcon.jest.tsx

diff --git a/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss b/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss
index def371db39..417d86e18c 100644
--- a/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss
+++ b/packages/core/src/components/Menu/MenuItem/MenuItem.module.scss
@@ -15,17 +15,6 @@
   outline: none;
 }
 
-.iconWrapper {
-  margin-right: var(--spacing-small);
-  display: flex;
-  align-items: center;
-  justify-content: center;
-
-  .icon {
-    color: var(--icon-color);
-  }
-}
-
 .item.splitMenuItem {
   padding: 0 0 0 var(--spacing-small);
 }
@@ -35,11 +24,6 @@
   color: var(--disabled-text-color);
 }
 
-.item.disabled .icon {
-  cursor: not-allowed;
-  color: var(--disabled-text-color);
-}
-
 .item:not(.disabled).focused {
   background-color: var(--primary-background-hover-color);
 }
@@ -57,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;
diff --git a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
index f8f9249831..fc38bd10ae 100644
--- a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
+++ b/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";
@@ -31,6 +30,7 @@ import { CloseMenuOption, MenuChild } from "../Menu/MenuConstants";
 import Label from "../../Label/Label";
 import styles from "./MenuItem.module.scss";
 import useIsMouseEnter from "../../../hooks/useIsMouseEnter";
+import MenuItemIcon from "./components/MenuItemIcon/MenuItemIcon";
 import MenuItemSubMenuIcon from "./components/MenuItemSubMenuIcon/MenuItemSubMenuIcon";
 
 export interface MenuItemProps extends VibeComponentProps {
@@ -237,43 +237,6 @@ const MenuItem: VibeComponent<MenuItemProps | MenuItemTitleComponentProps> & {
     // if "title" is a component ariaLabel is mandatory
     const iconLabel = ariaLabel ?? (title as string);
 
-    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 {};
       return {
@@ -314,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}
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.module.scss b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.module.scss
new file mode 100644
index 0000000000..49481744a7
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.module.scss
@@ -0,0 +1,28 @@
+.iconWrapper {
+  margin-right: var(--spacing-small);
+
+  &.withBackgroundColor {
+    border-radius: var(--spacing-xs);
+
+    &.disabled {
+      opacity: 0.4;
+    }
+
+    .icon {
+      color: var(--text-color-on-primary);
+    }
+  }
+
+  &.disabled .icon {
+    cursor: not-allowed;
+    color: var(--disabled-text-color);
+  }
+
+  .icon {
+    color: var(--icon-color);
+
+    &.selected {
+      color: var(--primary-color);
+    }
+  }
+}
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx
new file mode 100644
index 0000000000..68debd21fa
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx
@@ -0,0 +1,41 @@
+import React from "react";
+import Icon from "../../../../Icon/Icon";
+import Flex from "../../../../Flex/Flex";
+import cx from "classnames";
+import styles from "./MenuItemIcon.module.scss";
+import { MenuItemIconProps } from "./MenuItemIcon.types";
+
+const MenuItemIcon = ({
+  icon,
+  type,
+  label,
+  disabled,
+  selected,
+  backgroundColor,
+  wrapperClassName
+}: MenuItemIconProps) => (
+  <Flex
+    justify={Flex.justify.CENTER}
+    className={cx(
+      styles.iconWrapper,
+      {
+        [styles.disabled]: disabled,
+        [styles.withBackgroundColor]: !!backgroundColor
+      },
+      wrapperClassName
+    )}
+    style={{ backgroundColor }}
+  >
+    <Icon
+      iconType={type || (typeof icon === "function" ? Icon.type.SVG : Icon.type.ICON_FONT)}
+      clickable={false}
+      icon={icon}
+      iconLabel={label}
+      className={cx(styles.icon, { [styles.selected]: !disabled && selected })}
+      ignoreFocusStyle
+      iconSize={18}
+    />
+  </Flex>
+);
+
+export default MenuItemIcon;
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.types.ts b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.types.ts
new file mode 100644
index 0000000000..c48fdb0768
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.types.ts
@@ -0,0 +1,33 @@
+import { SubIcon } from "../../../../../types";
+import { IconType } from "../../../../Icon/IconConstants";
+
+export interface MenuItemIconProps {
+  /**
+   * Icon to be displayed. Can be a string or an icon component.
+   */
+  icon?: SubIcon;
+  /**
+   * Icon type to be used.
+   */
+  type?: IconType;
+  /**
+   * Label for the icon, used for accessibility.
+   */
+  label?: string;
+  /**
+   * Indicates whether the icon is disabled. Disabled icons appear faded and do not respond to user interactions.
+   */
+  disabled?: boolean;
+  /**
+   * Indicates whether the icon is selected. Selected icons have a different visual style.
+   */
+  selected?: boolean;
+  /**
+   * Background color for the icon wrapper.
+   */
+  backgroundColor?: string;
+  /**
+   * Additional class name for the icon wrapper.
+   */
+  wrapperClassName?: string;
+}
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/__tests__/MenuItemIcon.jest.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/__tests__/MenuItemIcon.jest.tsx
new file mode 100644
index 0000000000..bf78dde516
--- /dev/null
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/__tests__/MenuItemIcon.jest.tsx
@@ -0,0 +1,42 @@
+import { render, screen } from "@testing-library/react";
+import "@testing-library/jest-dom";
+import MenuItemIcon from "../MenuItemIcon";
+import { MenuItemIconProps } from "../MenuItemIcon.types";
+
+function renderMenuItemIcon(props: Partial<MenuItemIconProps> = {}) {
+  return render(<MenuItemIcon icon="test-icon" label="Test Icon" {...props} />);
+}
+
+describe("MenuItemIcon", () => {
+  it("should render the icon correctly", () => {
+    const { getByLabelText } = renderMenuItemIcon();
+    expect(getByLabelText("Test Icon")).toBeInTheDocument();
+  });
+
+  it("should apply the disabled style if the disabled prop is true", () => {
+    const { getByLabelText } = renderMenuItemIcon({ disabled: true });
+    const iconWrapper = getByLabelText("Test Icon").parentNode;
+    expect(iconWrapper).toHaveClass("disabled");
+  });
+
+  it("should apply the selected style if the selected prop is true and not disabled", () => {
+    const { getByLabelText } = renderMenuItemIcon({ selected: true });
+    screen.logTestingPlaygroundURL();
+    expect(getByLabelText("Test Icon")).toHaveClass("selected");
+  });
+
+  it("should not apply the selected style if the icon is disabled", () => {
+    const { getByLabelText } = renderMenuItemIcon({
+      selected: true,
+      disabled: true
+    });
+    expect(getByLabelText("Test Icon")).not.toHaveClass("selected");
+  });
+
+  it("should set the background color if backgroundColor prop is provided", () => {
+    const { getByLabelText } = renderMenuItemIcon({ backgroundColor: "red" });
+    const iconWrapper = getByLabelText("Test Icon").parentNode;
+    expect(iconWrapper).toHaveClass("withBackgroundColor");
+    expect(iconWrapper).toHaveStyle("background-color: red");
+  });
+});

From f3166699258ae7765868726ab62f9b88e8c8dd0b Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Sun, 5 May 2024 14:04:03 +0300
Subject: [PATCH 8/9] fix(MenuItem): do not render icon if empty string, do not
 add background color if not specified

---
 .../core/src/components/Menu/MenuItem/MenuItem.tsx |  2 +-
 .../__snapshots__/menuItem-snapshots.jest.js.snap  | 14 ++++++++++++--
 .../components/MenuItemIcon/MenuItemIcon.tsx       |  2 +-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
index fc38bd10ae..eb04b0b34f 100644
--- a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
+++ b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
@@ -277,7 +277,7 @@ const MenuItem: VibeComponent<MenuItemProps | MenuItemTitleComponentProps> & {
         onMouseEnter={onMouseEnter}
         tabIndex={TAB_INDEX_FOCUS_WITH_JS_ONLY}
       >
-        {icon && (
+        {!!icon && (
           <MenuItemIcon
             icon={icon}
             type={iconType}
diff --git a/packages/core/src/components/Menu/MenuItem/__tests__/__snapshots__/menuItem-snapshots.jest.js.snap b/packages/core/src/components/Menu/MenuItem/__tests__/__snapshots__/menuItem-snapshots.jest.js.snap
index 145b179d77..470d990913 100644
--- a/packages/core/src/components/Menu/MenuItem/__tests__/__snapshots__/menuItem-snapshots.jest.js.snap
+++ b/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}
diff --git a/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx
index 68debd21fa..1393fb80b7 100644
--- a/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx
+++ b/packages/core/src/components/Menu/MenuItem/components/MenuItemIcon/MenuItemIcon.tsx
@@ -24,7 +24,7 @@ const MenuItemIcon = ({
       },
       wrapperClassName
     )}
-    style={{ backgroundColor }}
+    style={{ ...(backgroundColor && { backgroundColor }) }}
   >
     <Icon
       iconType={type || (typeof icon === "function" ? Icon.type.SVG : Icon.type.ICON_FONT)}

From 87c1d871691ced78e1d85902492fdddc49436942 Mon Sep 17 00:00:00 2001
From: Yossi Saadi <yossisa@monday.com>
Date: Sun, 5 May 2024 15:40:46 +0300
Subject: [PATCH 9/9] refactor(MenuItem): pr comments

---
 packages/core/src/components/Menu/MenuItem/MenuItem.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
index eb04b0b34f..dc45d6486e 100644
--- a/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
+++ b/packages/core/src/components/Menu/MenuItem/MenuItem.tsx
@@ -277,7 +277,7 @@ const MenuItem: VibeComponent<MenuItemProps | MenuItemTitleComponentProps> & {
         onMouseEnter={onMouseEnter}
         tabIndex={TAB_INDEX_FOCUS_WITH_JS_ONLY}
       >
-        {!!icon && (
+        {Boolean(icon) && (
           <MenuItemIcon
             icon={icon}
             type={iconType}