From 4141a1e19b02267b74b9d0358c1682e8f80ff981 Mon Sep 17 00:00:00 2001
From: Felix Habib <fhabib@seek.com.au>
Date: Wed, 18 Dec 2024 15:38:55 +1100
Subject: [PATCH] Ensure left slot spacing is consistent

---
 .../components/MenuItem/useMenuItem.css.ts    |  4 +
 .../lib/components/MenuItem/useMenuItem.tsx   | 75 +++++++++++++------
 .../MenuItemCheckbox/MenuItemCheckbox.tsx     | 41 +++++-----
 3 files changed, 74 insertions(+), 46 deletions(-)

diff --git a/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.css.ts b/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.css.ts
index b9d84156ee4..bebbe9e06ea 100644
--- a/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.css.ts
+++ b/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.css.ts
@@ -7,3 +7,7 @@ export const menuItem = style({
     },
   },
 });
+
+export const menuItemLeftSlot = style({
+  height: '0px',
+});
diff --git a/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.tsx b/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.tsx
index 320c6309fc2..ca99079a7d8 100644
--- a/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.tsx
+++ b/packages/braid-design-system/src/lib/components/MenuItem/useMenuItem.tsx
@@ -160,7 +160,7 @@ export function useMenuItem<MenuItemElement extends HTMLElement>({
           display: 'flex',
           alignItems: 'center',
           width: 'full',
-          paddingX: size === 'standard' ? 'small' : 'small', // todo - with 'small' size - small or xsmall?
+          paddingX: size === 'standard' ? 'small' : 'small',
           paddingY: size === 'standard' ? undefined : 'xsmall',
           height: size === 'standard' ? 'touchable' : undefined,
           cursor: 'pointer',
@@ -173,6 +173,46 @@ export function useMenuItem<MenuItemElement extends HTMLElement>({
   } as const;
 }
 
+export function MenuItemLeftSlot({ children }: { children?: ReactNode }) {
+  const menuRendererContext = useContext(MenuRendererContext);
+
+  assert(
+    menuRendererContext !== null,
+    `MenuItem must be rendered as an immediate child of a menu. See the documentation for correct usage: https://seek-oss.github.io/braid-design-system/components/MenuItem`,
+  );
+
+  const { size } = menuRendererContext;
+  const iconSpace = useBraidTheme().legacy ? 'small' : iconSlotSpace;
+  return (
+    <Box
+      component="span"
+      paddingRight={iconSpace}
+      flexShrink={0}
+      minWidth={0}
+      position="relative"
+      display="flex"
+      alignItems="center"
+      className={styles.menuItemLeftSlot}
+    >
+      <Box component="span" display="block" className={iconSize({ size })}>
+        &nbsp;
+      </Box>
+      {children ? (
+        <Box
+          position="absolute"
+          left={0}
+          top={0}
+          bottom={0}
+          display="flex"
+          alignItems="center"
+        >
+          {children}
+        </Box>
+      ) : null}
+    </Box>
+  );
+}
+
 export interface MenuItemChildrenProps {
   children: ReactNode;
   tone: MenuItemTone;
@@ -188,9 +228,7 @@ function MenuItemChildren({
   formElement = false,
 }: MenuItemChildrenProps) {
   const menuRendererContext = useContext(MenuRendererContext);
-  const legacy = useBraidTheme().legacy;
-  const iconSpace = legacy ? 'small' : iconSlotSpace;
-  const badgeSpace = legacy ? 'small' : badgeSlotSpace;
+  const badgeSpace = useBraidTheme().legacy ? 'small' : badgeSlotSpace;
 
   assert(
     menuRendererContext !== null,
@@ -205,28 +243,17 @@ function MenuItemChildren({
 
   const { size, reserveIconSpace } = menuRendererContext;
 
-  const leftSlot =
-    !formElement && (icon || reserveIconSpace) ? (
-      <Text size={size} tone={tone === 'critical' ? tone : undefined}>
-        {icon || (
-          <Box component="span" display="block" className={iconSize({ size })}>
-            &nbsp;
-          </Box>
-        )}
-      </Text>
-    ) : null;
-
   return (
     <Box component="span" display="flex" alignItems="center" minWidth={0}>
-      {leftSlot ? (
-        <Box
-          component="span"
-          paddingRight={iconSpace}
-          flexShrink={0}
-          minWidth={0}
-        >
-          {leftSlot}
-        </Box>
+      {!formElement && (icon || reserveIconSpace) ? (
+        <MenuItemLeftSlot>
+          <Text
+            size={size}
+            tone={tone && tone === 'critical' ? tone : undefined}
+          >
+            {icon}
+          </Text>
+        </MenuItemLeftSlot>
       ) : null}
       <Box component="span" minWidth={0}>
         <Text
diff --git a/packages/braid-design-system/src/lib/components/MenuItemCheckbox/MenuItemCheckbox.tsx b/packages/braid-design-system/src/lib/components/MenuItemCheckbox/MenuItemCheckbox.tsx
index a21e1597c7f..b0059d0318a 100644
--- a/packages/braid-design-system/src/lib/components/MenuItemCheckbox/MenuItemCheckbox.tsx
+++ b/packages/braid-design-system/src/lib/components/MenuItemCheckbox/MenuItemCheckbox.tsx
@@ -2,9 +2,7 @@ import React, { type ReactNode } from 'react';
 import { Box } from '../Box/Box';
 import { IconTick } from '../icons/IconTick/IconTick';
 import type { MenuItemProps } from '../MenuItem/MenuItem';
-import { useMenuItem } from '../MenuItem/useMenuItem';
-import { iconSlotSpace } from '../private/iconSlotSpace';
-import { useBraidTheme } from '../BraidProvider/BraidThemeContext';
+import { MenuItemLeftSlot, useMenuItem } from '../MenuItem/useMenuItem';
 
 import * as styles from './MenuItemCheckbox.css';
 
@@ -28,8 +26,6 @@ export const MenuItemCheckbox = ({
     data,
     id,
   });
-  const legacy = useBraidTheme().legacy;
-  const iconSpace = legacy ? 'xsmall' : iconSlotSpace;
 
   return (
     <Box
@@ -41,28 +37,29 @@ export const MenuItemCheckbox = ({
       display="flex"
       alignItems="center"
     >
-      <Box
-        component="span"
-        borderRadius="standard"
-        boxShadow="borderField"
-        position="relative"
-        background={{ lightMode: 'surface' }}
-        marginRight={iconSpace}
-        flexShrink={0}
-        className={styles.checkboxSize}
-      >
+      <MenuItemLeftSlot>
         <Box
           component="span"
-          position="absolute"
-          inset={0}
-          background="formAccent"
           borderRadius="standard"
-          transition="fast"
-          opacity={checked ? undefined : 0}
+          boxShadow="borderField"
+          position="relative"
+          background={{ lightMode: 'surface' }}
+          flexShrink={0}
+          className={styles.checkboxSize}
         >
-          <IconTick size="fill" />
+          <Box
+            component="span"
+            position="absolute"
+            inset={0}
+            background="formAccent"
+            borderRadius="standard"
+            transition="fast"
+            opacity={checked ? undefined : 0}
+          >
+            <IconTick size="fill" />
+          </Box>
         </Box>
-      </Box>
+      </MenuItemLeftSlot>
       <MenuItemChildren
         tone={undefined}
         icon={undefined}