fix(scrollable-block): ensure border-styling is applied when appropri…

…ate to do so border-radius styling was being set incorrectly
Sage · May 17, 2024 · 14b0b21 · 14b0b21
1 parent 0300a5e
commit 14b0b21
Show file tree

Hide file tree

Showing 5 changed files with 140 additions and 5 deletions.
diff --git a/src/components/menu/__internal__/submenu/submenu.component.tsx b/src/components/menu/__internal__/submenu/submenu.component.tsx
@@ -114,6 +114,10 @@ const Submenu = React.forwardRef<
     const [submenuItemIds, setSubmenuItemIds] = useState<(string | null)[]>([]);
     const [characterString, setCharacterString] = useState("");
     const [applyFocusRadius, setApplyFocusRadius] = useState<boolean>(false);
+    const [
+      applyFocusRadiusToLastItem,
+      setApplyFocusRadiusToLastItem,
+    ] = useState<boolean>(false);
     const shiftTabPressed = useRef(false);
     const focusFirstMenuItemOnOpen = useRef(false);
 
@@ -192,6 +196,12 @@ const Submenu = React.forwardRef<
         // Get the last segment block
         const lastSegmentBlock = ulElements.pop();
 
+        // Check if the last segment block is a scrollable block
+        const isLastSegmentBlockScrollableBlock = Boolean(
+          lastSegmentBlock?.parentElement?.dataset.component ===
+            SCROLLABLE_BLOCK
+        );
+
         // Get all the menu items from the last segment block
         const segmentBlockMenuItems = Array.from(
           lastSegmentBlock?.querySelectorAll("[data-component='menu-item']") ||
@@ -200,18 +210,36 @@ const Submenu = React.forwardRef<
 
         // Get the last menu item in the last segment block
         const lastMenuItemInSegmentBlock = segmentBlockMenuItems.pop();
+
+        // Check to see if the last menu item in the last segment block is visible
+        let isLastMenuItemInSegmentBlockVisible = false;
+        if (lastMenuItemInSegmentBlock && lastSegmentBlock) {
+          isLastMenuItemInSegmentBlockVisible =
+            lastMenuItemInSegmentBlock.getBoundingClientRect().bottom <
+            lastSegmentBlock.getBoundingClientRect().bottom;
+        }
+
         // Check if the last item in the segment block is the same as the last MenuItem in the submenu
         const menuItemsMatch =
           !!lastMenuItemInSegmentBlock &&
           lastMenuItemInSegmentBlock === lastMenuItem;
 
+        // Applies the focus radius to the StyledBox of the StyledScrollableBlock
         setApplyFocusRadius(menuItemsMatch);
+
+        // Applies border radius to the last item in the segment block
+        setApplyFocusRadiusToLastItem(
+          (menuItemsMatch && !isLastSegmentBlockScrollableBlock) ||
+            (menuItemsMatch &&
+              isLastSegmentBlockScrollableBlock &&
+              !isLastMenuItemInSegmentBlockVisible)
+        );
       };
 
       if (submenuOpen && submenuRef) {
         handleBorderRadiusStyling();
       }
-    }, [submenuOpen, submenuRef]);
+    }, [submenuOpen, submenuRef, numberOfChildren]);
 
     useEffect(() => {
       if (submenuOpen && onSubmenuOpen) {
@@ -470,6 +498,7 @@ const Submenu = React.forwardRef<
             inFullscreenView={inFullscreenView}
             ref={setSubmenuRef}
             applyFocusRadiusStyling={false}
+            applyFocusRadiusStylingToLastItem={applyFocusRadiusToLastItem}
           >
             <SubmenuContext.Provider
               value={{
@@ -524,6 +553,7 @@ const Submenu = React.forwardRef<
             role={blockIndex === 0 ? "presentation" : "list"}
             maxHeight={submenuMaxHeight}
             applyFocusRadiusStyling={applyFocusRadius}
+            applyFocusRadiusStylingToLastItem={applyFocusRadiusToLastItem}
           >
             <SubmenuContext.Provider
               value={{

diff --git a/src/components/menu/__internal__/submenu/submenu.spec.tsx b/src/components/menu/__internal__/submenu/submenu.spec.tsx
@@ -645,6 +645,7 @@ describe("Submenu component", () => {
             menuType="light"
             variant="default"
             applyFocusRadiusStyling={false}
+            applyFocusRadiusStylingToLastItem={false}
           >
             <MenuItem>Apple</MenuItem>
             <MenuItem>Banana</MenuItem>

diff --git a/src/components/menu/__internal__/submenu/submenu.style.ts b/src/components/menu/__internal__/submenu/submenu.style.ts
@@ -27,6 +27,7 @@ interface StyledSubmenuProps
   submenuDirection?: string;
   maxHeight?: string;
   applyFocusRadiusStyling: boolean;
+  applyFocusRadiusStylingToLastItem: boolean;
 }
 
 const StyledSubmenuWrapper = styled.div<StyledSubmenuWrapperProps>`
@@ -64,6 +65,7 @@ const StyledSubmenu = styled.ul<StyledSubmenuProps>`
     inFullscreenView,
     maxHeight,
     applyFocusRadiusStyling,
+    applyFocusRadiusStylingToLastItem,
   }) => css`
     ${!inFullscreenView &&
     menuType &&
@@ -111,10 +113,10 @@ const StyledSubmenu = styled.ul<StyledSubmenuProps>`
         border-bottom-left-radius: var(--borderRadius000);
 
         :focus {
-          border-bottom-right-radius: ${applyFocusRadiusStyling
+          border-bottom-right-radius: ${applyFocusRadiusStylingToLastItem
             ? "var(--borderRadius100)"
             : "var(--borderRadius000)"};
-          border-bottom-left-radius: ${applyFocusRadiusStyling
+          border-bottom-left-radius: ${applyFocusRadiusStylingToLastItem
             ? "var(--borderRadius100)"
             : "var(--borderRadius000)"};
         }
@@ -130,7 +132,7 @@ const StyledSubmenu = styled.ul<StyledSubmenuProps>`
           ${StyledMenuItem}:last-child ${StyledLink}, ${StyledMenuItem}:last-child a,
           ${StyledMenuItem}:last-child button {
             border-bottom-right-radius: var(--borderRadius000);
-            border-bottom-left-radius: ${applyFocusRadiusStyling
+            border-bottom-left-radius: ${applyFocusRadiusStylingToLastItem
               ? "var(--borderRadius100)"
               : "var(--borderRadius000)"};
           }

diff --git a/src/components/menu/component.test-pw.tsx b/src/components/menu/component.test-pw.tsx
@@ -14,7 +14,7 @@ import {
   ScrollableBlockProps,
 } from ".";
 import { MenuType } from "./menu.context";
-import Search from "../search";
+import Search, { SearchEvent } from "../search";
 import GlobalHeader from "../global-header";
 import Box from "../box/box.component";
 import Typography from "../typography/typography.component";
@@ -106,6 +106,60 @@ export const MenuComponentScrollable = (
   );
 };
 
+export const MenuComponentScrollableWithSearch = () => {
+  const items = [
+    "apple",
+    "banana",
+    "carrot",
+    "grapefruit",
+    "melon",
+    "orange",
+    "pear",
+    "strawberry",
+  ];
+  const [itemSearch, setItemSearch] = React.useState(items);
+  const [searchString, setSearchString] = React.useState("");
+  const handleTextChange = (e: SearchEvent) => {
+    const searchStr = e.target.value;
+    setSearchString(searchStr);
+    let found;
+    if (searchStr.length > 0) {
+      found = items.filter((item) => item.includes(searchStr));
+    } else {
+      found = items;
+    }
+    setItemSearch(found);
+  };
+  return (
+    <Box mb={300}>
+      <Menu>
+        <MenuItem onClick={() => {}}>Menu Item One</MenuItem>
+        <MenuItem href="#">Menu Item Two</MenuItem>
+        <MenuItem submenu="Menu Item Three">
+          <MenuItem href="#">Item Submenu One</MenuItem>
+          <ScrollableBlock
+            variant="alternate"
+            height="200px"
+            parent={
+              <Search
+                placeholder="search"
+                value={searchString}
+                onChange={handleTextChange}
+              />
+            }
+          >
+            {itemSearch.map((item) => (
+              <MenuItem key={item} href="#">
+                {item}
+              </MenuItem>
+            ))}
+          </ScrollableBlock>
+        </MenuItem>
+      </Menu>
+    </Box>
+  );
+};
+
 export const MenuComponentSearch = () => {
   return (
     <Box mb={150}>

diff --git a/src/components/menu/menu.pw.tsx b/src/components/menu/menu.pw.tsx
@@ -64,6 +64,7 @@ import {
   MenuDividerComponent,
   InGlobalHeaderStory,
   SubMenuWithVeryLongLabel,
+  MenuComponentScrollableWithSearch,
   MenuSegmentTitleComponentWithAdditionalMenuItem,
 } from "./component.test-pw";
 import { NavigationBarWithSubmenuAndChangingHeight } from "../navigation-bar/navigation-bar-test.stories";
@@ -2393,6 +2394,53 @@ test.describe(
       );
     });
 
+    test(`renders last MenuItem in a scrollable block without rounded corner, if there is no overflow in the block`, async ({
+      mount,
+      page,
+    }) => {
+      await mount(<MenuComponentScrollableWithSearch />);
+
+      const subMenu = submenu(page).first();
+      await subMenu.hover();
+      const searchInput = searchDefaultInput(page);
+      await searchInput.fill("app");
+      const scrollableBlock = scrollBlock(page);
+
+      await expect(scrollableBlock).toHaveCSS(
+        "border-radius",
+        "0px 0px 0px 8px"
+      );
+
+      const scrollableItem = scrollBlock(page).locator("a").last();
+
+      await expect(scrollableItem).toHaveCSS("border-radius", "0px");
+    });
+
+    test(`renders last MenuItem in a scrollable block with rounded corner, if there is overflow within the block`, async ({
+      mount,
+      page,
+    }) => {
+      await mount(<MenuComponentScrollableWithSearch />);
+
+      const subMenu = submenu(page).first();
+      await subMenu.hover();
+      const searchInput = searchDefaultInput(page);
+      await searchInput.fill("r");
+      const scrollableBlock = scrollBlock(page);
+
+      await expect(scrollableBlock).toHaveCSS(
+        "border-radius",
+        "0px 0px 0px 8px"
+      );
+
+      const scrollableItem = scrollBlock(page).locator("a").last();
+
+      await expect(scrollableItem).toHaveCSS(
+        "border-radius",
+        "0px 0px 0px 8px"
+      );
+    });
+
     test(`should verify that tabbing forward through the menu and back to the start should not make the background scroll to the bottom`, async ({
       mount,
       page,