fix(menu-item): prevent default padding on button/anchor inside MenuI…

…tem if no onClick/href set No longer sets the padding styling on the internal anchor or button elements rendered internally by the `MenuItem` if both `onClick` and `href` props are not passed fix #5640
Sage · Dec 1, 2022 · 5d67b4d · 5d67b4d
1 parent 279e13d
commit 5d67b4d
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 10 deletions.
diff --git a/src/components/menu/menu-item/menu-item.component.js b/src/components/menu/menu-item/menu-item.component.js
@@ -133,10 +133,9 @@ const MenuItem = ({
     maxWidth && typeof title === "string" ? title : "";
 
   const itemMaxWidth = !inFullscreenView ? maxWidth : undefined;
+  const asPassiveItem = !(onClick || href);
 
   if (submenu) {
-    const asPassiveItem = !(onClick || href);
-
     return (
       <StyledMenuItem
         data-component="menu-item"
@@ -187,6 +186,7 @@ const MenuItem = ({
         ariaLabel={ariaLabel}
         maxWidth={maxWidth}
         inFullscreenView={inFullscreenView}
+        asPassiveItem={asPassiveItem}
       >
         {clonedChildren}
       </StyledMenuItemWrapper>

diff --git a/src/components/menu/menu-item/menu-item.spec.js b/src/components/menu/menu-item/menu-item.spec.js
@@ -18,6 +18,8 @@ import StyledIcon from "../../icon/icon.style";
 import Icon from "../../icon/icon.component";
 import { StyledMenuItem } from "../menu.style";
 import menuConfigVariants from "../menu.config";
+import IconButton from "../../icon-button";
+import StyledIconButton from "../../icon-button/icon-button.style";
 
 const events = {
   enter: {
@@ -144,6 +146,53 @@ describe("MenuItem", () => {
         );
       });
 
+      it.each([
+        ["button", "onClick"],
+        ["a", "href"],
+      ])(
+        "should not set padding on the %s element if no %s passed",
+        (element) => {
+          wrapper = mount(
+            <MenuContext.Provider value={{ menuType }}>
+              <MenuItem>Item one</MenuItem>
+            </MenuContext.Provider>
+          );
+
+          expect(
+            wrapper.find(StyledMenuItemWrapper)
+          ).not.toHaveStyleRule("padding", "0 16px", { modifier: element });
+        }
+      );
+
+      it("applies the expected styling overrides when an IconButton is rendered as a child", () => {
+        wrapper = mount(
+          <MenuContext.Provider value={{ menuType }}>
+            <MenuItem>
+              <IconButton>
+                <Icon type="home" />
+              </IconButton>
+            </MenuItem>
+          </MenuContext.Provider>
+        );
+
+        assertStyleMatch(
+          {
+            display: "inline-flex",
+            marginRight: "0",
+          },
+          wrapper.find(StyledMenuItemWrapper),
+          { modifier: `${StyledIconButton} > span` }
+        );
+
+        assertStyleMatch(
+          {
+            outline: "none",
+          },
+          wrapper.find(StyledMenuItemWrapper),
+          { modifier: `${StyledIconButton}:focus` }
+        );
+      });
+
       describe.each([
         ["button", "focus"],
         ["a", "focus"],

diff --git a/src/components/menu/menu-item/menu-item.style.js b/src/components/menu/menu-item/menu-item.style.js
@@ -1,6 +1,7 @@
 import styled, { css } from "styled-components";
 import { StyledLink } from "../../link/link.style";
-import IconStyle from "../../icon/icon.style";
+import StyledIcon from "../../icon/icon.style";
+import StyledIconButton from "../../icon-button/icon-button.style";
 import menuConfigVariants from "../menu.config";
 
 const StyledMenuItemWrapper = styled.a`
@@ -18,6 +19,7 @@ const StyledMenuItemWrapper = styled.a`
     inFullscreenView,
     overrideColor,
     as,
+    asPassiveItem,
   }) => css`
     display: inline-block;
     font-size: 14px;
@@ -85,11 +87,27 @@ const StyledMenuItemWrapper = styled.a`
       `
     }
 
-    a,
-    ${StyledLink} a,
-    button,
-    ${StyledLink} button {
-      padding: 0 16px;
+    ${
+      !asPassiveItem &&
+      css`
+        a,
+        ${StyledLink} a,
+        button,
+        ${StyledLink} button {
+          padding: 0 16px;
+        }
+      `
+    }
+
+    ${StyledIconButton} {
+      > span {
+        display: inline-flex;
+        margin-right: 0;
+      }
+
+      :focus {
+        outline: none;
+      }
     }
 
     button,
@@ -112,7 +130,7 @@ const StyledMenuItemWrapper = styled.a`
         color: ${menuConfigVariants[menuType].color};
       }
 
-      ${IconStyle} {
+      ${StyledIcon} {
         bottom: 1px;
       }
     }
@@ -226,7 +244,7 @@ const StyledMenuItemWrapper = styled.a`
         ${showDropdownArrow &&
         css`
           > a,
-          > button {
+          > button:not(${StyledIconButton}) {
             padding-right: 32px;
           }