From 685e5d911cde4e472b9baf23d6b7846f491ea533 Mon Sep 17 00:00:00 2001
From: thyhjwb6 <56078793+thyhjwb6@users.noreply.github.com>
Date: Mon, 16 Dec 2024 16:16:00 +0000
Subject: [PATCH 1/4] test(FloatingBox): Align with patterns

---
 .../FloatingBox/FloatingBox.test.tsx          | 194 ++++++------------
 1 file changed, 66 insertions(+), 128 deletions(-)

diff --git a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
index dbdff52cb0..f3c5f04e9a 100644
--- a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
+++ b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
@@ -1,141 +1,79 @@
 import React from 'react'
 
 import { renderToStaticMarkup } from 'react-dom/server'
-import { render, RenderResult } from '@testing-library/react'
+import { render, screen } from '@testing-library/react'
 
 import { FloatingBox } from '.'
 import { useStatefulRef } from '../../hooks/useStatefulRef'
 
-describe('FloatingBox', () => {
-  let wrapper: RenderResult
-  let children: React.ReactElement
+it('renders the floating box when provided with a `renderTarget` and arbitrary JSX content', () => {
+  render(
+    <FloatingBox
+      isVisible
+      renderTarget={<div>Hello, World!</div>}
+      role="dialog"
+      aria-modal
+    >
+      <pre>This is some arbitrary JSX</pre>
+    </FloatingBox>
+  )
+
+  expect(screen.getByTestId('floating-box')).toHaveAttribute('role', 'dialog')
+  expect(screen.getByTestId('floating-box')).toHaveAttribute('aria-modal')
+  expect(screen.getByText('Hello, World!')).toBeInTheDocument()
+  expect(screen.getByTestId('floating-box-content').innerHTML).toContain(
+    renderToStaticMarkup(<pre>This is some arbitrary JSX</pre>)
+  )
+  expect(screen.getByTestId('floating-box-styled-target')).toBeInTheDocument()
+})
 
-  describe('when provided a `renderTarget` and arbitrary JSX content', () => {
-    beforeEach(() => {
-      children = <pre>This is some arbitrary JSX</pre>
+it('renders the floating box when provided with a `renderTargetElement` and arbitrary JSX content', () => {
+  const TestComponent = () => {
+    const [element, setElement] = useStatefulRef()
 
-      wrapper = render(
-        <FloatingBox
-          isVisible
-          renderTarget={<div>Hello, World!</div>}
-          role="dialog"
-          aria-modal
-        >
-          {children}
+    return (
+      <>
+        <div ref={setElement}>Hello, World!</div>
+        <FloatingBox isVisible targetElement={element} role="dialog" aria-modal>
+          <pre>This is some arbitrary JSX</pre>
         </FloatingBox>
-      )
-    })
-
-    it('applies the supplied `role`', () => {
-      expect(wrapper.queryByTestId('floating-box')).toHaveAttribute(
-        'role',
-        'dialog'
-      )
-    })
-
-    it('applies additional arbitrary props to wrapper', () => {
-      expect(wrapper.queryByTestId('floating-box')).toHaveAttribute(
-        'aria-modal'
-      )
-    })
-
-    it('applies the `role` attribute', () => {
-      expect(wrapper.getByTestId('floating-box')).toHaveAttribute(
-        'role',
-        'dialog'
-      )
-    })
-
-    it('renders an embedded target', () => {
-      expect(
-        wrapper.getByTestId('floating-box-styled-target')
-      ).toBeInTheDocument()
-    })
-
-    it('renders the box', () => {
-      expect(wrapper.getByTestId('floating-box')).toBeInTheDocument()
-    })
-
-    it('renders the provided renderTarget JSX', () => {
-      expect(wrapper.getByText('Hello, World!')).toBeInTheDocument()
-    })
-
-    it('renders the provided arbitrary JSX', () => {
-      expect(wrapper.getByTestId('floating-box-content').innerHTML).toContain(
-        renderToStaticMarkup(children)
-      )
-    })
-  })
-
-  describe('when provided a `renderTargetElement` and arbitrary JSX content', () => {
-    beforeEach(() => {
-      children = <pre>This is some arbitrary JSX</pre>
-
-      const TestComponent = () => {
-        const [element, setElement] = useStatefulRef()
-
-        return (
-          <>
-            <div ref={setElement}>Hello, World!</div>
-            <FloatingBox
-              isVisible
-              targetElement={element}
-              role="dialog"
-              aria-modal
-            >
-              {children}
-            </FloatingBox>
-          </>
-        )
-      }
-      wrapper = render(<TestComponent />)
-    })
-
-    it('renders the box', () => {
-      expect(wrapper.queryByTestId('floating-box')).toBeInTheDocument()
-    })
-
-    it('does not render an embedded target', () => {
-      expect(
-        wrapper.queryByTestId('floating-box-styled-target')
-      ).not.toBeInTheDocument()
-    })
-
-    it('renders the provided arbitrary JSX', () => {
-      expect(wrapper.getByTestId('floating-box-content').innerHTML).toContain(
-        renderToStaticMarkup(children)
-      )
-    })
-  })
-
-  describe('when the content changes', () => {
-    const ExampleFloatingBox = ({
-      children: content,
-    }: {
-      children: string
-    }) => (
-      <FloatingBox
-        isVisible
-        renderTarget={<div>Hello, World!</div>}
-        role="dialog"
-        aria-modal
-      >
-        <>{content}</>
-      </FloatingBox>
+      </>
     )
-    let initialId: string
-
-    beforeEach(() => {
-      wrapper = render(<ExampleFloatingBox>Initial content</ExampleFloatingBox>)
-      initialId = wrapper.getByTestId('floating-box-content').id
-      wrapper.rerender(<ExampleFloatingBox>Updated content</ExampleFloatingBox>)
-    })
+  }
+
+  render(<TestComponent />)
+
+  expect(screen.getByTestId('floating-box')).toHaveAttribute('role', 'dialog')
+  expect(screen.getByTestId('floating-box')).toHaveAttribute('aria-modal')
+  expect(screen.getByText('Hello, World!')).toBeInTheDocument()
+  expect(screen.getByTestId('floating-box-content').innerHTML).toContain(
+    renderToStaticMarkup(<pre>This is some arbitrary JSX</pre>)
+  )
+  expect(
+    screen.queryByTestId('floating-box-styled-target')
+  ).not.toBeInTheDocument()
+})
 
-    it('does not generate a new content `id`', () => {
-      expect(wrapper.getByTestId('floating-box-content')).toHaveAttribute(
-        'id',
-        initialId
-      )
-    })
-  })
+it('does not generate a new content `id` when the content changes', () => {
+  const ExampleFloatingBox = ({ children: content }: { children: string }) => (
+    <FloatingBox
+      isVisible
+      renderTarget={<div>Hello, World!</div>}
+      role="dialog"
+      aria-modal
+    >
+      <>{content}</>
+    </FloatingBox>
+  )
+
+  const { rerender } = render(
+    <ExampleFloatingBox>Initial content</ExampleFloatingBox>
+  )
+  const initialId = screen.getByTestId('floating-box-content').id
+  rerender(<ExampleFloatingBox>Updated content</ExampleFloatingBox>)
+
+  expect(screen.getByTestId('floating-box-content')).toHaveAttribute(
+    'id',
+    initialId
+  )
 })

From 475106a8d2ae14fa22cad59a23763e6c53887d3b Mon Sep 17 00:00:00 2001
From: thyhjwb6 <56078793+thyhjwb6@users.noreply.github.com>
Date: Tue, 17 Dec 2024 16:21:16 +0000
Subject: [PATCH 2/4] test(FloatingBox): Test box position

---
 .../FloatingBox/FloatingBox.test.tsx          | 41 ++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
index f3c5f04e9a..67e96c2be0 100644
--- a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
+++ b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
@@ -1,7 +1,7 @@
 import React from 'react'
 
 import { renderToStaticMarkup } from 'react-dom/server'
-import { render, screen } from '@testing-library/react'
+import { render, screen, waitFor } from '@testing-library/react'
 
 import { FloatingBox } from '.'
 import { useStatefulRef } from '../../hooks/useStatefulRef'
@@ -77,3 +77,42 @@ it('does not generate a new content `id` when the content changes', () => {
     initialId
   )
 })
+
+it('sets the position of the floating box', async () => {
+  const offsetHeight = 100
+
+  jest.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
+    top: 0,
+    left: 0,
+    right: 100,
+    bottom: 100,
+    width: 100,
+    height: 100,
+    x: 0,
+    y: 0,
+    toJSON() {
+      return this
+    },
+  })
+
+  Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
+    value: offsetHeight,
+  })
+
+  render(
+    <FloatingBox
+      isVisible
+      renderTarget={<div>Hello, World!</div>}
+      placement="bottom"
+      aria-modal
+    >
+      <pre>This is some arbitrary JSX</pre>
+    </FloatingBox>
+  )
+
+  await waitFor(() => {
+    expect(screen.getByTestId('floating-box')).toHaveStyle({
+      transform: `translate(0px, ${offsetHeight}px)`,
+    })
+  })
+})

From a0933553bb673e2d6926c6a336e43db125a82217 Mon Sep 17 00:00:00 2001
From: thyhjwb6 <56078793+thyhjwb6@users.noreply.github.com>
Date: Thu, 19 Dec 2024 10:40:27 +0000
Subject: [PATCH 3/4] fix(FloatingBox): Move setting of refs

When using React 19, ref is a prop and was being overwritten with {...rest}.
---
 .../FloatingBox/FloatingBox.test.tsx          |  40 +++++
 .../primitives/FloatingBox/FloatingBox.tsx    | 159 ++++++++++--------
 2 files changed, 129 insertions(+), 70 deletions(-)

diff --git a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
index 67e96c2be0..b6dfdf9abe 100644
--- a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
+++ b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
@@ -116,3 +116,43 @@ it('sets the position of the floating box', async () => {
     })
   })
 })
+
+it('sets the position of the floating box when the ref is null', async () => {
+  const offsetHeight = 100
+
+  jest.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
+    top: 0,
+    left: 0,
+    right: 100,
+    bottom: 100,
+    width: 100,
+    height: 100,
+    x: 0,
+    y: 0,
+    toJSON() {
+      return this
+    },
+  })
+
+  Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
+    value: offsetHeight,
+  })
+
+  render(
+    <FloatingBox
+      isVisible
+      renderTarget={<div>Hello, World!</div>}
+      placement="bottom"
+      aria-modal
+      ref={null}
+    >
+      <pre>This is some arbitrary JSX</pre>
+    </FloatingBox>
+  )
+
+  await waitFor(() => {
+    expect(screen.getByTestId('floating-box')).toHaveStyle({
+      transform: `translate(0px, ${offsetHeight}px)`,
+    })
+  })
+})
diff --git a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.tsx b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.tsx
index a24f20d18a..91ab75abad 100644
--- a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.tsx
+++ b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.tsx
@@ -1,4 +1,4 @@
-import React, { useRef } from 'react'
+import React, { forwardRef, useRef } from 'react'
 import { BasePlacement, Placement, VariationPlacement } from '@popperjs/core'
 import { Transition } from 'react-transition-group'
 import mergeRefs from 'react-merge-refs'
@@ -52,79 +52,98 @@ export type FloatingBoxProps =
   | FloatingBoxWithExternalTargetProps
   | FloatingBoxWithEmbeddedTargetProps
 
-export const FloatingBox = ({
-  contentId: externalContentId,
-  scheme = FLOATING_BOX_SCHEME.LIGHT,
-  onMouseEnter,
-  onMouseLeave,
-  children,
-  renderTarget,
-  targetElement,
-  isVisible,
-  placement = 'auto',
-  role = 'dialog',
-  ...rest
-}: FloatingBoxProps) => {
-  const nodeRef = useRef<HTMLDivElement>(null)
-  const contentId = useExternalId('floating-box', externalContentId)
-  const {
-    targetElementRef,
-    floatingElementRef,
-    arrowElementRef,
-    styles,
-    attributes,
-  } = useFloatingElement(placement, undefined, targetElement)
+// forwardRef` is being used to replicate an issue when using React 19, because
+// `ref` is a prop. When upgrading, `forwardRef` can be removed.
+// https://react.dev/blog/2024/12/05/react-19#ref-as-a-prop
+// https://github.com/Royal-Navy/design-system/issues/3969
+export const FloatingBox = forwardRef<HTMLDivElement, FloatingBoxProps>(
+  (
+    {
+      contentId: externalContentId,
+      scheme = FLOATING_BOX_SCHEME.LIGHT,
+      onMouseEnter,
+      onMouseLeave,
+      children,
+      renderTarget,
+      targetElement,
+      isVisible,
+      placement = 'auto',
+      role = 'dialog',
+      ...rest
+    },
+    ref
+  ) => {
+    const nodeRef = useRef<HTMLDivElement>(null)
+    const contentId = useExternalId('floating-box', externalContentId)
+    const {
+      targetElementRef,
+      floatingElementRef,
+      arrowElementRef,
+      styles,
+      attributes,
+    } = useFloatingElement(placement, undefined, targetElement)
 
-  const calculatedPlacement = attributes?.popper?.['data-popper-placement'] as
-    | BasePlacement
-    | VariationPlacement
-    | undefined
+    const calculatedPlacement = attributes?.popper?.[
+      'data-popper-placement'
+    ] as BasePlacement | VariationPlacement | undefined
 
-  const basePlacement = calculatedPlacement?.split('-', 1)?.[0] as
-    | BasePlacement
-    | undefined
+    const basePlacement = calculatedPlacement?.split('-', 1)?.[0] as
+      | BasePlacement
+      | undefined
 
-  return (
-    <>
-      {renderTarget && (
-        <StyledTarget
-          ref={targetElementRef}
-          data-testid="floating-box-styled-target"
-        >
-          {renderTarget}
-        </StyledTarget>
-      )}
-      <Transition nodeRef={nodeRef} in={isVisible} timeout={0} unmountOnExit>
-        {(state) => (
-          <StyledFloatingBox
-            $transitionStatus={state}
-            style={styles.popper}
-            ref={mergeRefs([nodeRef, floatingElementRef])}
-            onMouseEnter={onMouseEnter}
-            onMouseLeave={onMouseLeave}
-            role={role}
-            data-testid="floating-box"
-            {...attributes.popper}
-            {...rest}
+    // Set `floatingBoxRest` to replicate what is happening in React 19 because
+    // `ref` is a prop leading to the merged refs being overwritten with
+    // `{...rest}`. When upgrading to React 19, `forwardRef` will be removed, and
+    // we can revert to using `{...rest}` rather than setting `floatingBoxRest`.
+    // https://react.dev/blog/2024/12/05/react-19#ref-as-a-prop
+    // https://github.com/Royal-Navy/design-system/issues/3969
+    const floatingBoxRest = {
+      ...rest,
+      ref,
+    }
+
+    return (
+      <>
+        {renderTarget && (
+          <StyledTarget
+            ref={targetElementRef}
+            data-testid="floating-box-styled-target"
           >
-            <FloatingBoxContent
-              contentId={contentId}
-              scheme={scheme}
-              data-testid="floating-box-content"
-            >
-              <StyledArrow
-                $placement={basePlacement}
-                ref={arrowElementRef}
-                style={styles.arrow}
-                {...attributes.arrow}
-              />
-              {children}
-            </FloatingBoxContent>
-          </StyledFloatingBox>
+            {renderTarget}
+          </StyledTarget>
         )}
-      </Transition>
-    </>
-  )
-}
+        <Transition nodeRef={nodeRef} in={isVisible} timeout={0} unmountOnExit>
+          {(state) => (
+            <StyledFloatingBox
+              $transitionStatus={state}
+              style={styles.popper}
+              onMouseEnter={onMouseEnter}
+              onMouseLeave={onMouseLeave}
+              role={role}
+              data-testid="floating-box"
+              {...attributes.popper}
+              {...floatingBoxRest}
+              ref={mergeRefs([nodeRef, floatingElementRef, ref])}
+            >
+              <FloatingBoxContent
+                contentId={contentId}
+                scheme={scheme}
+                data-testid="floating-box-content"
+              >
+                <StyledArrow
+                  $placement={basePlacement}
+                  ref={arrowElementRef}
+                  style={styles.arrow}
+                  {...attributes.arrow}
+                />
+                {children}
+              </FloatingBoxContent>
+            </StyledFloatingBox>
+          )}
+        </Transition>
+      </>
+    )
+  }
+)
 
 FloatingBox.displayName = 'FloatingBox'

From 00fe246c58f76af034f2e7d84be61598d2801e9d Mon Sep 17 00:00:00 2001
From: thyhjwb6 <56078793+thyhjwb6@users.noreply.github.com>
Date: Thu, 19 Dec 2024 11:02:52 +0000
Subject: [PATCH 4/4] test(FloatingBox): Reuse common setup

Setup functions can be safely abstracted and reused across tests.
---
 .../FloatingBox/FloatingBox.test.tsx          | 88 +++++++------------
 1 file changed, 30 insertions(+), 58 deletions(-)

diff --git a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
index b6dfdf9abe..e0aa57d2a3 100644
--- a/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
+++ b/packages/react-component-library/src/primitives/FloatingBox/FloatingBox.test.tsx
@@ -6,10 +6,31 @@ import { render, screen, waitFor } from '@testing-library/react'
 import { FloatingBox } from '.'
 import { useStatefulRef } from '../../hooks/useStatefulRef'
 
-it('renders the floating box when provided with a `renderTarget` and arbitrary JSX content', () => {
+function setupOffsetHeightMock(offsetHeight: number) {
+  jest.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
+    top: 0,
+    left: 0,
+    right: 100,
+    bottom: 100,
+    width: 100,
+    height: 100,
+    x: 0,
+    y: 0,
+    toJSON() {
+      return this
+    },
+  })
+
+  Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
+    value: offsetHeight,
+  })
+}
+
+function setup() {
   render(
     <FloatingBox
       isVisible
+      placement="bottom"
       renderTarget={<div>Hello, World!</div>}
       role="dialog"
       aria-modal
@@ -17,6 +38,10 @@ it('renders the floating box when provided with a `renderTarget` and arbitrary J
       <pre>This is some arbitrary JSX</pre>
     </FloatingBox>
   )
+}
+
+it('renders the floating box when provided with a `renderTarget` and arbitrary JSX content', () => {
+  setup()
 
   expect(screen.getByTestId('floating-box')).toHaveAttribute('role', 'dialog')
   expect(screen.getByTestId('floating-box')).toHaveAttribute('aria-modal')
@@ -81,34 +106,8 @@ it('does not generate a new content `id` when the content changes', () => {
 it('sets the position of the floating box', async () => {
   const offsetHeight = 100
 
-  jest.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
-    top: 0,
-    left: 0,
-    right: 100,
-    bottom: 100,
-    width: 100,
-    height: 100,
-    x: 0,
-    y: 0,
-    toJSON() {
-      return this
-    },
-  })
-
-  Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
-    value: offsetHeight,
-  })
-
-  render(
-    <FloatingBox
-      isVisible
-      renderTarget={<div>Hello, World!</div>}
-      placement="bottom"
-      aria-modal
-    >
-      <pre>This is some arbitrary JSX</pre>
-    </FloatingBox>
-  )
+  setupOffsetHeightMock(offsetHeight)
+  setup()
 
   await waitFor(() => {
     expect(screen.getByTestId('floating-box')).toHaveStyle({
@@ -120,35 +119,8 @@ it('sets the position of the floating box', async () => {
 it('sets the position of the floating box when the ref is null', async () => {
   const offsetHeight = 100
 
-  jest.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({
-    top: 0,
-    left: 0,
-    right: 100,
-    bottom: 100,
-    width: 100,
-    height: 100,
-    x: 0,
-    y: 0,
-    toJSON() {
-      return this
-    },
-  })
-
-  Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
-    value: offsetHeight,
-  })
-
-  render(
-    <FloatingBox
-      isVisible
-      renderTarget={<div>Hello, World!</div>}
-      placement="bottom"
-      aria-modal
-      ref={null}
-    >
-      <pre>This is some arbitrary JSX</pre>
-    </FloatingBox>
-  )
+  setupOffsetHeightMock(offsetHeight)
+  setup()
 
   await waitFor(() => {
     expect(screen.getByTestId('floating-box')).toHaveStyle({