fix(menu, menu-item): ensure that menu-items all remain the same heig…

…ht if any wrap to new lines Ensures that `MenuItem`s flex so that they all have the same height if any wrap their content to new lines at smaller screen resolutions. Ensures no additional padding is set on `MenuItem` children of `MenuFullscreen`. BREAKING CHANGE: `Menu` no longer supports `height`, `minHeight`, `maxHeight`, `size`, `overflowY` and `display` props. `MenuItem` no longer supports `height`, `minHeight`, `maxHeight`, `size`, `verticalAlign`, `overflow`, `overflowY`, `overflowX` and `display` props. fix #6934, fix #7000
Sage · Oct 16, 2024 · 00c842b · 00c842b
1 parent 96d41f2
commit 00c842b
Show file tree

Hide file tree

Showing 18 changed files with 240 additions and 214 deletions.
diff --git a/src/components/global-header/component.test-pw.tsx b/src/components/global-header/component.test-pw.tsx
@@ -7,7 +7,7 @@ import NavigationBar from "../navigation-bar";
 export const FullMenuExample = () => (
   <>
     <GlobalHeader>
-      <Menu menuType="black" display="flex" flex="1">
+      <Menu menuType="black" flex="1">
         <MenuItem flex="1" submenu="Product Switcher">
           <MenuItem href="#">Product A</MenuItem>
         </MenuItem>
@@ -22,7 +22,7 @@ export const FullMenuExample = () => (
       </Menu>
     </GlobalHeader>
     <NavigationBar position="fixed" orientation="top" offset="40px">
-      <Menu display="flex" flex="1">
+      <Menu flex="1">
         <MenuItem flex="1">Menu Item One</MenuItem>
         <MenuItem flex="0 0 auto" href="#">
           Menu Item Two

diff --git a/src/components/global-header/global-header-test.stories.tsx b/src/components/global-header/global-header-test.stories.tsx
@@ -27,7 +27,7 @@ export const MenuWithIconOnlyButtonsStory: StoryFn<
   return (
     <GlobalHeader logo={<img height={28} src={carbonLogo} alt="Carbon logo" />}>
       <VerticalDivider h="100%" pt={1} pb={1} pr={0} pl={2} tint={100} />
-      <Menu menuType="black" display="flex" flex="1">
+      <Menu menuType="black" flex="1">
         <MenuItem flex="1" submenu="Product Switcher">
           <MenuItem href="#">Product A</MenuItem>
         </MenuItem>

diff --git a/src/components/global-header/global-header.stories.tsx b/src/components/global-header/global-header.stories.tsx
@@ -55,7 +55,7 @@ export const BasicMenu: Story = () => {
 
   return (
     <GlobalHeader logo={<Logo />}>
-      <Menu menuType="black" display="flex" flex="1">
+      <Menu menuType="black" flex="1">
         <MenuItem flex="1" submenu="Product Switcher">
           <MenuItem>Product A</MenuItem>
         </MenuItem>
@@ -112,7 +112,7 @@ export const ResponsiveMenu: Story = () => {
 
   return (
     <GlobalHeader logo={<Logo />}>
-      <Menu menuType="black" display="flex" flex="1">
+      <Menu menuType="black" flex="1">
         {fullscreenViewBreakPoint ? (
           <>
             <MenuItem
@@ -145,7 +145,7 @@ export const GlobalLocalNavBarLayout: Story = () => {
   return (
     <>
       <GlobalHeader logo={<Logo />}>
-        <Menu menuType="black" display="flex" flex="1">
+        <Menu menuType="black" flex="1">
           <MenuItem flex="1" submenu="Product Switcher">
             <MenuItem href="#">Product A</MenuItem>
           </MenuItem>
@@ -160,7 +160,7 @@ export const GlobalLocalNavBarLayout: Story = () => {
         </Menu>
       </GlobalHeader>
       <NavigationBar position="fixed" orientation="top" offset="40px">
-        <Menu display="flex" flex="1">
+        <Menu flex="1">
           <MenuItem href="#" flex="1">
             Menu Item One
           </MenuItem>

diff --git a/src/components/menu/__internal__/submenu/submenu.style.ts b/src/components/menu/__internal__/submenu/submenu.style.ts
@@ -34,15 +34,16 @@ const StyledSubmenuWrapper = styled.div<StyledSubmenuWrapperProps>`
   position: relative;
   width: fit-content;
   max-width: inherit;
+  height: inherit;
 
   ${({ isSubmenuOpen, theme }) =>
     isSubmenuOpen &&
     css`
       z-index: ${theme.zIndex.popover};
     `}
 
-  ${({ inFullscreenView, menuType, asPassiveItem }) =>
-    inFullscreenView &&
+  ${({ inFullscreenView, menuType, asPassiveItem }) => css`
+    ${inFullscreenView &&
     css`
       width: 100%;
 
@@ -55,6 +56,11 @@ const StyledSubmenuWrapper = styled.div<StyledSubmenuWrapperProps>`
         }
       `}
     `}
+    ${!inFullscreenView &&
+    css`
+      display: flex;
+    `}
+  `}
 `;
 
 const StyledSubmenu = styled.ul<StyledSubmenuProps>`
@@ -73,6 +79,7 @@ const StyledSubmenu = styled.ul<StyledSubmenuProps>`
     css`
       box-shadow: var(--boxShadow100);
       position: absolute;
+      top: 100%;
       background-color: ${variant === "default"
         ? menuConfigVariants[menuType].submenuItemBackground
         : menuConfigVariants[menuType].background};
@@ -212,10 +219,6 @@ const StyledSubmenu = styled.ul<StyledSubmenuProps>`
         > a,
         > button {
           padding: 11px 16px 12px;
-
-          :has([data-component="icon"]) {
-            padding: 9px 16px 7px;
-          }
         }
       `}
 

diff --git a/src/components/menu/component.test-pw.tsx b/src/components/menu/component.test-pw.tsx
@@ -33,7 +33,7 @@ export const MenuComponent = (props: Partial<MenuProps> & MenuDividerProps) => {
           <Typography variant="h4" textTransform="capitalize" my={2}>
             {menuType}
           </Typography>
-          <Menu menuType={menuType} display="flex" {...props}>
+          <Menu menuType={menuType} {...props}>
             <MenuItem href="#">Menu Item One</MenuItem>
             <MenuItem href="#">Menu Item Two</MenuItem>
             <MenuItem submenu="Menu Item Three">
@@ -422,7 +422,7 @@ export const MenuComponentItems = (
   return (
     <Box mb={150}>
       <Typography textTransform="capitalize" my={2} />
-      <Menu menuType="white" display="flex">
+      <Menu menuType="white">
         <MenuItem submenu="Menu Item One" submenuDirection="right" {...props}>
           <MenuItem href="#">Item Submenu One</MenuItem>
           <MenuItem href="#">Item Submenu Two</MenuItem>
@@ -622,7 +622,7 @@ export const MenuSegmentTitleComponent = (props: Partial<MenuTitleProps>) => {
           <Typography variant="h4" textTransform="capitalize" my={2}>
             {menuType}
           </Typography>
-          <Menu menuType={menuType} display="flex">
+          <Menu menuType={menuType}>
             <MenuItem href="#">Menu Item One</MenuItem>
             <MenuItem href="#">Menu Item Two</MenuItem>
             <MenuItem submenu="Menu Item Three">
@@ -655,7 +655,7 @@ export const MenuSegmentTitleComponentWithAdditionalMenuItem = (
           <Typography variant="h4" textTransform="capitalize" my={2}>
             {menuType}
           </Typography>
-          <Menu menuType={menuType} display="flex">
+          <Menu menuType={menuType}>
             <MenuItem href="#">Menu Item One</MenuItem>
             <MenuItem href="#">Menu Item Two</MenuItem>
             <MenuItem submenu="Menu Item Three">
@@ -705,7 +705,7 @@ export const MenuDividerComponent = (props: MenuDividerProps) => {
           <Typography variant="h4" textTransform="capitalize" my={2}>
             {menuType}
           </Typography>
-          <Menu menuType={menuType} display="flex">
+          <Menu menuType={menuType}>
             <MenuItem href="#">Menu Item One</MenuItem>
             <MenuItem href="#">Menu Item Two</MenuItem>
             <MenuItem submenu="Menu Item Three">

diff --git a/src/components/menu/menu-item/menu-item.component.tsx b/src/components/menu/menu-item/menu-item.component.tsx
@@ -23,7 +23,7 @@ export type VariantType = "default" | "alternate";
 
 interface MenuItemBaseProps
   extends Omit<TagProps, "data-component">,
-    LayoutProps,
+    Pick<LayoutProps, "width" | "maxWidth" | "minWidth">,
     FlexboxProps,
     PaddingProps {
   /** Custom className */
@@ -330,6 +330,7 @@ export const MenuItem = ({
         {...paddingProps}
         asDiv={hasInput || as === "div"}
         hasInput={hasInput}
+        inSubmenu={!!handleSubmenuKeyDown}
       >
         {children}
       </StyledMenuItemWrapper>

diff --git a/src/components/menu/menu-item/menu-item.style.ts b/src/components/menu/menu-item/menu-item.style.ts
@@ -33,6 +33,7 @@ interface StyledMenuItemWrapperProps
   asDiv?: boolean;
   hasInput?: boolean;
   menuItemVariant?: Pick<MenuWithChildren, "variant">["variant"];
+  inSubmenu?: boolean;
 }
 
 const BASE_SPACING = 16;
@@ -98,6 +99,7 @@ const StyledMenuItemWrapper = styled.a.attrs({
     asPassiveItem,
     asDiv,
     hasInput,
+    inSubmenu,
   }) => css`
     display: flex;
     align-items: center;
@@ -130,7 +132,7 @@ const StyledMenuItemWrapper = styled.a.attrs({
         button:has([data-component="icon"]):not(:has(button))
         ${StyledContent} {
         position: relative;
-        top: -2px;
+        top: -1px;
       }
     `}
 
@@ -155,11 +157,16 @@ const StyledMenuItemWrapper = styled.a.attrs({
     ${!inFullscreenView &&
     css`
       max-width: inherit;
+      width: inherit;
+      height: inherit;
 
       > a,
       > button {
         display: flex;
         align-items: center;
+        ${!inSubmenu ? "justify-content: center;" : ""}
+        width: inherit;
+        max-width: inherit;
       }
 
       && {
@@ -212,7 +219,7 @@ const StyledMenuItemWrapper = styled.a.attrs({
           !inFullscreenView &&
           `
           > a:not(:has(button)) {
-            padding: 11px 16px 12px;
+            padding: 11px 16px;
           }
 
           > a ${StyledButton}:not(.search-button) {
@@ -237,29 +244,48 @@ const StyledMenuItemWrapper = styled.a.attrs({
         }
       `
       : `
-        a,
-        ${StyledLink} a,
-        button,
-        ${StyledLink} button {
-       
-          padding: ${inFullscreenView ? "0px 16px" : "11px 16px 12px"};
-
-          :has([data-component="icon"]) {
-            padding: 9px 16px 7px;
-          }
+        ${
+          hasSubmenu
+            ? `
+              a,
+              ${StyledLink} a,
+              button,
+              ${StyledLink} button {
+                height: 100%;
+                padding: 0px 16px;
+              }
+            `
+            : `
+              button,
+              ${StyledLink} button {
+                height: 100%;
+                padding: ${!inFullscreenView ? "11px" : "0px"} 16px;
+              }
+
+              a,
+              ${StyledLink} a {
+                height: 100%;
+                padding: ${maxWidth && !inFullscreenView ? "10px" : "0px"} 16px;
+              }
+            `
         }
       `}
 
     button,
-    ${StyledLink} button {
-      height: 40px;
+    ${StyledLink} button,
+    a,
+    ${StyledLink} a {
       margin: 0px;
       text-align: left;
 
       ${inFullscreenView &&
       css`
         height: auto;
         white-space: normal;
+
+        ${StyledIcon} {
+          top: -2px;
+        }
       `}
     }
 
@@ -279,6 +305,8 @@ const StyledMenuItemWrapper = styled.a.attrs({
       css`
         a > ${StyledIcon}, button > ${StyledIcon} {
           display: inline-block;
+          height: 18px;
+          top: -2px;
         }
       `}
     }