Merge pull request #6382 from Sage/FE-6187-resize-observer-warning

fix: remove resize observer error
Sage · Nov 1, 2023 · 75b6413 · 75b6413
2 parents 1db9c98 + d80db16
commit 75b6413
Show file tree

Hide file tree

Showing 11 changed files with 178 additions and 50 deletions.
diff --git a/cypress/components/global-header/global-header.cy.tsx b/cypress/components/global-header/global-header.cy.tsx
@@ -1,14 +1,23 @@
 /* eslint-disable jest/valid-expect, jest/valid-expect-in-promise */
 import React from "react";
 import GlobalHeader from "../../../src/components/global-header";
-import { FullMenuExample } from "../../../src/components/global-header/global-header-test.stories";
+import {
+  FullMenuExample,
+  GlobalHeaderWithErrorHandler,
+} from "../../../src/components/global-header/global-header-test.stories";
 import CypressMountWithProviders from "../../support/component-helper/cypress-mount";
 
 import carbonLogo from "../../../logo/carbon-logo.png";
 import navigationBar from "../../locators/navigation-bar";
 import { globalHeader, globalHeaderLogo } from "../../locators/global-header";
 
 context("Testing Global Header component", () => {
+  it("should not cause a ResizeObserver-related error to occur", () => {
+    CypressMountWithProviders(<GlobalHeaderWithErrorHandler />);
+    cy.wait(500);
+    cy.get("#error-div").should("have.text", "");
+  });
+
   it("should check that z-index of component is greater than that of NavigationBar", () => {
     CypressMountWithProviders(<FullMenuExample />);
     globalHeader().invoke("css", "zIndex").as("globalHeaderZIndex");

diff --git a/cypress/components/menu/menu.cy.tsx b/cypress/components/menu/menu.cy.tsx
@@ -63,6 +63,7 @@ import {
   MenuDividerComponent,
   InGlobalHeaderStory,
 } from "../../../src/components/menu/menu-test.stories";
+import { NavigationBarWithSubmenuAndChangingHeight } from "../../../src/components/navigation-bar/navigation-bar-test.stories";
 
 const span = "span";
 const div = "div";
@@ -1960,7 +1961,7 @@ context("Testing Menu component", () => {
     });
   });
 
-  describe("when inside a GlobalHeader", () => {
+  describe("when inside a Navigation Bar", () => {
     it("all the content of a long submenu can be accessed with the keyboard while remaining visible", () => {
       CypressMountWithProviders(<InGlobalHeaderStory />);
 
@@ -1978,5 +1979,32 @@ context("Testing Menu component", () => {
         '[data-component="submenu-wrapper"] ul > li:nth-child(20)'
       );
     });
+
+    it("all the content of a long submenu can be accessed with the keyboard while remaining visible if the navbar height changes", () => {
+      CypressMountWithProviders(<NavigationBarWithSubmenuAndChangingHeight />);
+
+      cy.viewport(1000, 500);
+
+      menuComponent(1).trigger("keydown", keyCode("downarrow"));
+      submenuItem(1).should("have.length", 21);
+
+      // navigate to "change height" item and press it
+      for (let i = 0; i < 3; i++) {
+        cy.focused().trigger("keydown", keyCode("downarrow"));
+      }
+      cy.focused().trigger("keydown", keyCode("Enter"));
+
+      // reopen menu and scroll to bottom with keyboard
+      cy.wait(100);
+      menuComponent(1).trigger("keydown", keyCode("downarrow"));
+
+      for (let i = 0; i < 21; i++) {
+        cy.focused().trigger("keydown", keyCode("downarrow"));
+      }
+
+      cy.checkInViewport(
+        '[data-component="submenu-wrapper"] ul > li:nth-child(21)'
+      );
+    });
   });
 });
diff --git a/src/components/global-header/global-header-test.stories.tsx b/src/components/global-header/global-header-test.stories.tsx
@@ -1,7 +1,7 @@
-import React from "react";
+import React, { useState, useEffect } from "react";
 import { ComponentMeta, ComponentStory } from "@storybook/react";
 
-import GlobalHeader from "./global-header.component";
+import GlobalHeader, { GlobalHeaderProps } from "./global-header.component";
 import { Menu, MenuItem, MenuDivider } from "../menu";
 import VerticalDivider from "../vertical-divider";
 import NavigationBar from "../navigation-bar";
@@ -96,3 +96,23 @@ export const FullMenuExample = () => (
     </NavigationBar>
   </>
 );
+
+export const GlobalHeaderWithErrorHandler = ({
+  ...props
+}: GlobalHeaderProps) => {
+  const [error, setError] = useState("");
+  useEffect(() => {
+    const handleError = (e: ErrorEvent) => {
+      setError(e.message);
+    };
+    window.addEventListener("error", handleError);
+
+    return () => window.removeEventListener("error", handleError);
+  });
+  return (
+    <>
+      <GlobalHeader {...props} />
+      <div id="error-div">{error}</div>
+    </>
+  );
+};
diff --git a/src/components/global-header/global-header.spec.tsx b/src/components/global-header/global-header.spec.tsx
@@ -1,24 +1,12 @@
 import { render, screen } from "@testing-library/react";
 import React from "react";
 import GlobalHeader, { GlobalHeaderProps } from "./global-header.component";
-import Logger from "../../__internal__/utils/logger";
-
-// mock Logger.deprecate so that no console warnings occur while running the tests
-const loggerSpy = jest.spyOn(Logger, "deprecate");
 
 function renderer(props?: GlobalHeaderProps) {
   return render(<GlobalHeader {...props}>foobar</GlobalHeader>);
 }
 
 describe("Global Header", () => {
-  beforeAll(() => {
-    loggerSpy.mockImplementation(() => {});
-  });
-
-  afterAll(() => {
-    loggerSpy.mockRestore();
-  });
-
   it("should be visible with correct accessible name", () => {
     renderer();
     expect(screen.getByRole("navigation")).toHaveAccessibleName(

diff --git a/src/components/navigation-bar/components.test-pw.tsx b/src/components/navigation-bar/components.test-pw.tsx
@@ -1,4 +1,4 @@
-import React from "react";
+import React, { useState, useEffect } from "react";
 import { ComponentStory } from "@storybook/react";
 import NavigationBar, { NavigationBarProps } from ".";
 import { Menu, MenuDivider, MenuItem } from "../menu";
@@ -218,3 +218,23 @@ Fixed.parameters = {
   docs: { inlineStories: false, iframeHeight: 200 },
   themeProvider: { chromatic: { theme: "sage" } },
 };
+
+export const NavigationBarWithErrorHandler = ({
+  ...props
+}: NavigationBarProps) => {
+  const [error, setError] = useState("");
+  useEffect(() => {
+    const handleError = (e: ErrorEvent) => {
+      setError(e.message);
+    };
+    window.addEventListener("error", handleError);
+
+    return () => window.removeEventListener("error", handleError);
+  });
+  return (
+    <>
+      <NavigationBar {...props} />
+      <div id="error-div">{error}</div>
+    </>
+  );
+};
diff --git a/src/components/navigation-bar/fixed-navigation-bar-context.spec.tsx b/src/components/navigation-bar/fixed-navigation-bar-context.spec.tsx
@@ -14,15 +14,13 @@ const ConsumerComponent = () => {
 };
 
 const mockNavbarElement = { offsetHeight: 40 } as HTMLElement;
+const navbarRef = { current: mockNavbarElement };
 
 const MockComponent = (
-  props: Omit<FixedNavigationBarContextProviderProps, "navbarElement">
+  props: Omit<FixedNavigationBarContextProviderProps, "navbarRef">
 ) => {
   return (
-    <FixedNavigationBarContextProvider
-      navbarElement={mockNavbarElement}
-      {...props}
-    >
+    <FixedNavigationBarContextProvider navbarRef={navbarRef} {...props}>
       <ConsumerComponent />
     </FixedNavigationBarContextProvider>
   );

diff --git a/src/components/navigation-bar/fixed-navigation-bar.context.tsx b/src/components/navigation-bar/fixed-navigation-bar.context.tsx
@@ -1,4 +1,4 @@
-import React, { createContext, useState, useCallback } from "react";
+import React, { createContext, useState, useCallback, useEffect } from "react";
 import useResizeObserver from "../../hooks/__internal__/useResizeObserver/useResizeObserver";
 import { NavigationBarProps } from ".";
 
@@ -15,24 +15,30 @@ export interface FixedNavigationBarContextProviderProps
     NavigationBarProps,
     "position" | "orientation" | "offset" | "children"
   > {
-  navbarElement: HTMLElement | null;
+  navbarRef: React.RefObject<HTMLElement>;
 }
 
 export const FixedNavigationBarContextProvider = ({
   position,
   orientation,
   offset,
   children,
-  navbarElement,
+  navbarRef,
 }: FixedNavigationBarContextProviderProps) => {
-  const [navbarHeight, setNavbarHeight] = useState(navbarElement?.offsetHeight);
+  const [navbarHeight, setNavbarHeight] = useState(
+    navbarRef.current?.offsetHeight
+  );
 
   const updateHeight = useCallback(
-    () => setNavbarHeight(navbarElement?.offsetHeight),
-    [navbarElement]
+    () => setNavbarHeight(navbarRef.current?.offsetHeight),
+    [navbarRef]
   );
 
-  useResizeObserver({ current: navbarElement }, updateHeight);
+  useEffect(() => {
+    updateHeight();
+  }, [updateHeight]);
+
+  useResizeObserver(navbarRef, updateHeight);
 
   let submenuMaxHeight;
 

diff --git a/src/components/navigation-bar/navigation-bar-test.stories.tsx b/src/components/navigation-bar/navigation-bar-test.stories.tsx
@@ -1,9 +1,10 @@
-import React from "react";
+import React, { useRef } from "react";
 import NavigationBar, { NavigationBarProps } from ".";
+import { Menu, MenuItem } from "../menu";
 
 export default {
   title: "Navigation Bar/Test",
-  includeStories: ["DefaultStory"],
+  includeStories: ["DefaultStory", "NavigationBarWithSubmenuAndChangingHeight"],
   parameters: {
     info: { disable: true },
     chromatic: {
@@ -25,3 +26,45 @@ DefaultStory.args = {
   position: undefined,
   offset: "0",
 };
+
+export const NavigationBarWithSubmenuAndChangingHeight = () => {
+  const wrapperRef = useRef<HTMLDivElement>(null);
+  const toggleHeight = () => {
+    const navbarElement = wrapperRef.current?.querySelector("nav");
+    if (navbarElement) {
+      navbarElement.style.height =
+        navbarElement.style.height === "100px" ? "40px" : "100px";
+    }
+  };
+  return (
+    <div ref={wrapperRef}>
+      <NavigationBar position="fixed" orientation="top">
+        <Menu menuType="dark">
+          <MenuItem submenu="I'm long" clickToOpen>
+            <MenuItem onClick={() => {}}>Foo 1</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 2</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 3</MenuItem>
+            <MenuItem onClick={toggleHeight}>Change Height!</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 4</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 5</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 6</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 7</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 8</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 9</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 10</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 11</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 12</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 13</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 14</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 15</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 16</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 17</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 18</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 19</MenuItem>
+            <MenuItem onClick={() => {}}>Foo 20</MenuItem>
+          </MenuItem>
+        </Menu>
+      </NavigationBar>
+    </div>
+  );
+};
diff --git a/src/components/navigation-bar/navigation-bar.component.tsx b/src/components/navigation-bar/navigation-bar.component.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from "react";
+import React, { useRef } from "react";
 import { PaddingProps, FlexboxProps } from "styled-system";
 import StyledNavigationBar from "./navigation-bar.style";
 import { FixedNavigationBarContextProvider } from "./fixed-navigation-bar.context";
@@ -32,35 +32,35 @@ export const NavigationBar = ({
   children,
   ariaLabel,
   position,
-  offset = "0",
+  offset = "0px",
   orientation,
   isGlobal,
   ...props
 }: NavigationBarProps): JSX.Element => {
-  const [navbarElement, setNavbarElement] = useState<HTMLElement | null>(null);
+  const navbarRef = useRef(null);
 
   return (
-    <StyledNavigationBar
-      role="navigation"
-      data-component={isGlobal ? "global-header" : "navigation-bar"}
-      aria-label={isGlobal ? "Global Header" : ariaLabel}
-      navigationType={isGlobal ? "black" : navigationType}
+    <FixedNavigationBarContextProvider
       orientation={isGlobal ? "top" : orientation}
       offset={isGlobal ? "0px" : offset}
       position={isGlobal ? "fixed" : position}
-      {...props}
-      isGlobal={isGlobal}
-      ref={setNavbarElement}
+      navbarRef={navbarRef}
     >
-      <FixedNavigationBarContextProvider
+      <StyledNavigationBar
+        role="navigation"
+        data-component={isGlobal ? "global-header" : "navigation-bar"}
+        aria-label={isGlobal ? "Global Header" : ariaLabel}
+        navigationType={isGlobal ? "black" : navigationType}
         orientation={isGlobal ? "top" : orientation}
         offset={isGlobal ? "0px" : offset}
         position={isGlobal ? "fixed" : position}
-        navbarElement={navbarElement}
+        {...props}
+        isGlobal={isGlobal}
+        ref={navbarRef}
       >
         {!isLoading && children}
-      </FixedNavigationBarContextProvider>
-    </StyledNavigationBar>
+      </StyledNavigationBar>
+    </FixedNavigationBarContextProvider>
   );
 };
 

diff --git a/src/components/navigation-bar/navigation-bar.pw.tsx b/src/components/navigation-bar/navigation-bar.pw.tsx
@@ -15,6 +15,7 @@ import {
   ContentMaxWidthBox,
   Sticky,
   Fixed,
+  NavigationBarWithErrorHandler,
 } from "./components.test-pw";
 
 import navigationBar from "../../../playwright/components/navigation-bar";
@@ -32,6 +33,15 @@ const variants = [
 const offsetVal = [25, 100, -100];
 
 test.describe("Test props for NavigationBar component", () => {
+  test("should not cause a ResizeObserver-related error to occur", async ({
+    mount,
+    page,
+  }) => {
+    await mount(<NavigationBarWithErrorHandler />);
+
+    await expect(page.locator("#error-div")).toContainText("");
+  });
+
   specialCharacters.forEach((childrenValue) => {
     test(`should render with ${childrenValue} as a children`, async ({
       mount,