From a265b1cdc18d01c35c1ecf88299f9dc4fc76208c Mon Sep 17 00:00:00 2001
From: Robin Zigmond <robin.zigmond@sage.com>
Date: Tue, 17 Sep 2024 09:23:56 +0100
Subject: [PATCH] fix(navigation-bar): move context provider to not wrap parent

The FixedNavigationBarContextProvider, which uses a ResizeObserver to respond to size changes on the
navbar element, currently wraps that same element. This can result in unexpected ResizeObserver
errors on initial render, when a media query is being used to determine layout. The context provider
only needs to wrap the navbar's children, and by moving it down the React tree to only wrap the
children and not the parent, the error goes away.
---
 .../navigation-bar/components.test-pw.tsx     |  5 +++-
 .../navigation-bar-test.stories.tsx           | 24 ++++++++++++++++++-
 .../navigation-bar.component.tsx              | 24 +++++++++----------
 .../navigation-bar/navigation-bar.pw.tsx      |  2 +-
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/src/components/navigation-bar/components.test-pw.tsx b/src/components/navigation-bar/components.test-pw.tsx
index 99db345610..5f2e90632b 100644
--- a/src/components/navigation-bar/components.test-pw.tsx
+++ b/src/components/navigation-bar/components.test-pw.tsx
@@ -2,6 +2,7 @@ import React, { useState, useEffect } from "react";
 import NavigationBar, { NavigationBarProps } from ".";
 import { Menu, MenuDivider, MenuItem } from "../menu";
 import Box from "../box";
+import useMediaQuery from "../../hooks/useMediaQuery";
 
 export const NavigationBarComponent = ({
   children,
@@ -214,6 +215,8 @@ Fixed.parameters = {
 export const NavigationBarWithErrorHandler = ({
   ...props
 }: NavigationBarProps) => {
+  const isLargeScreen = useMediaQuery("(min-width: 600px)");
+  const rightPadding = isLargeScreen ? 2 : 1;
   const [error, setError] = useState("");
   useEffect(() => {
     const handleError = (e: ErrorEvent) => {
@@ -225,7 +228,7 @@ export const NavigationBarWithErrorHandler = ({
   });
   return (
     <>
-      <NavigationBar {...props} />
+      <NavigationBar pr={rightPadding} {...props} />
       <div id="error-div">{error}</div>
     </>
   );
diff --git a/src/components/navigation-bar/navigation-bar-test.stories.tsx b/src/components/navigation-bar/navigation-bar-test.stories.tsx
index 0063f0da35..d3b6b5f0c0 100644
--- a/src/components/navigation-bar/navigation-bar-test.stories.tsx
+++ b/src/components/navigation-bar/navigation-bar-test.stories.tsx
@@ -1,12 +1,14 @@
-import React, { useRef } from "react";
+import React, { useRef, useState, useEffect } from "react";
 import NavigationBar, { NavigationBarProps } from ".";
 import { Menu, MenuItem } from "../menu";
+import useMediaQuery from "../../hooks/useMediaQuery";
 
 export default {
   title: "Navigation Bar/Test",
   includeStories: [
     "DefaultStory",
     "NavigationBarWithSubmenuAndChangingHeight",
+    "WithMediaQuery",
     "ResponsivePadding",
   ],
   parameters: {
@@ -74,6 +76,26 @@ export const NavigationBarWithSubmenuAndChangingHeight = () => {
   );
 };
 
+export const WithMediaQuery = () => {
+  const isLargeScreen = useMediaQuery("(min-width: 600px)");
+  const rightPadding = isLargeScreen ? 2 : 1;
+  const [error, setError] = useState("");
+  useEffect(() => {
+    const handleError = (e: ErrorEvent) => {
+      setError(e.message);
+    };
+    window.addEventListener("error", handleError);
+
+    return () => window.removeEventListener("error", handleError);
+  }, []);
+  return (
+    <>
+      <NavigationBar pr={rightPadding} />
+      <div id="error-div">{error}</div>
+    </>
+  );
+};
+
 export const ResponsivePadding = () => {
   return <NavigationBar>Example content</NavigationBar>;
 };
diff --git a/src/components/navigation-bar/navigation-bar.component.tsx b/src/components/navigation-bar/navigation-bar.component.tsx
index ab7022558f..c391c3c1a9 100644
--- a/src/components/navigation-bar/navigation-bar.component.tsx
+++ b/src/components/navigation-bar/navigation-bar.component.tsx
@@ -40,27 +40,27 @@ export const NavigationBar = ({
   const navbarRef = useRef(null);
 
   return (
-    <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}
-      navbarRef={navbarRef}
+      {...props}
+      isGlobal={isGlobal}
+      ref={navbarRef}
     >
-      <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={navbarRef}
+        navbarRef={navbarRef}
       >
         {!isLoading && children}
-      </StyledNavigationBar>
-    </FixedNavigationBarContextProvider>
+      </FixedNavigationBarContextProvider>
+    </StyledNavigationBar>
   );
 };
 
diff --git a/src/components/navigation-bar/navigation-bar.pw.tsx b/src/components/navigation-bar/navigation-bar.pw.tsx
index 7724f16a68..cbab856c44 100644
--- a/src/components/navigation-bar/navigation-bar.pw.tsx
+++ b/src/components/navigation-bar/navigation-bar.pw.tsx
@@ -39,7 +39,7 @@ test.describe("Test props for NavigationBar component", () => {
   }) => {
     await mount(<NavigationBarWithErrorHandler />);
 
-    await expect(page.locator("#error-div")).toContainText("");
+    await expect(page.locator("#error-div")).toHaveText("");
   });
 
   specialCharacters.forEach((childrenValue) => {