Repository: Sage/carbon (ts)

Issue:
Menu component - Re-sizes menu item on initial render
### Description

I am using `<Menu>` and `<MenuItem>` in a navigation bar.
One of the Menu items in the navigation bar imports an external component which displays a Carbon `<Icon>`.
Existing behaviour is that these Menu items render at a consistent size, with consistent padding.
Since upgrading to React 18 via Carbon v148.0.0 I have noticed that on page load the external icon is rendered with additional padding.
The moment any element in the navigation bar is interacted with `onClick` or `onHover`, the element size shrinks back down to the expected height.

### Reproduction

https://stackblitz.com/edit/parsium-carbon-starter-padhopq4

### Steps to reproduce

_No response_

### JIRA ticket numbers (Sage only)

_No response_

### Suggested solution

_No response_

### Carbon version

148.0.0

### React version

v18

### Design tokens version

4.29.0

### Relevant browsers

Chrome

### Relevant OSs

MacOS

### Additional context

_No response_

### Confidentiality

- [x] I confirm there is no confidential or commercially sensitive information included.

Pull request description:
fix(menu-item): ensure that items with a focusable child are styled with no additional padding
fixes #7263

### Proposed behaviour

<!--
A clear and concise description of what changes this PR makes. If applicable, include any UI screenshots to help explain your request. If you are a Sage contributor, please DO NOT share any commercially sensitive information, such as UI screenshots or code from Sage products.
-->

Refactors internal ref and the ref for storing the `firstFocusableChild` to use state, this means that when they are set/updated they will force a re-render and the correct styling will be applied

### Current behaviour

<!--
A clear and concise description of the behaviour before this change. If applicable, include any UI screenshots to help explain your request. If you are a Sage contributor, please DO NOT share any commercially sensitive information, such as UI screenshots or code from Sage products.
-->

Refs used internally rely on `useRef` that has issues when updating and forcing a re-render.

### Checklist

<!-- Each PR should include the following -->

- [x] Commits follow our style guide
- [x] Related issues linked in commit messages if required
- [x] Unit tests added or updated if required

#### QA

- [ ] Tested in provided StackBlitz sandbox/Storybook
- [ ] Add new Playwright test coverage if required
- [ ] Carbon implementation matches Design System/designs
- [ ] UI Tests GitHub check reviewed if required

### Additional context

<!-- Add any other context or links about the pull request here. -->

This behaviour is only visible when using two versions of carbon so I wasn't able to unit or playwright test it.
I am happy to push up a separate branch with an adequate story and whoever is reviewing can follow the testing instructions below etc

### Testing instructions

<!--
How can a reviewer test this PR?

If this PR addresses a pre-existing bug, please include a link to a sandbox that reproduces the original bug. A starter template has been provided to help you do this:
<https://stackblitz.com/fork/github/Parsium/carbon-starter>
-->
You will need to test this with two versions of Carbon as it only displays the incorrect padding when importing Menu and MenuItem from different ones etc.

I have created [test-menu-item-changes](https://github.com/Sage/carbon/tree/test-menu-item-changes) branch with a demo story for testing:
- checkout to this branch
- run the `precompile` script to generate the new `lib` and `esm` directories, the `TestSpacingWhenLoadingFromOtherCarbon` story I have added imports one from the `src` folder and one from the `lib`
- in this branch you will see it fixed if you try the above instructions in master you will see the incorrect spacing you see kn the stackblitz provided in the linked issue

Patch:
diff --git a/src/components/link/link.component.tsx b/src/components/link/link.component.tsx
index a9611c53c..d8fe68047 100644
--- a/src/components/link/link.component.tsx
+++ b/src/components/link/link.component.tsx
@@ -94,6 +94,17 @@ export const Link = React.forwardRef<
     const { batchSelectionDisabled } = useContext(BatchSelectionContext);
     const isDisabled = disabled || batchSelectionDisabled;
 
+    const setRefs = React.useCallback(
+      (reference: HTMLAnchorElement) => {
+        if (!ref) return;
+        if (typeof ref === "object") ref.current = reference;
+        if (typeof ref === "function") {
+          ref(reference);
+        }
+      },
+      [ref],
+    );
+
     const renderLinkIcon = (currentAlignment = "left") => {
       const hasProperAlignment = icon && iconAlign === currentAlignment;
 
@@ -125,7 +136,7 @@ export const Link = React.forwardRef<
       onClick,
       disabled: isDisabled,
       target,
-      ref,
+      ref: setRefs,
       href,
       rel,
       "aria-label": ariaLabel,
diff --git a/src/components/menu/menu-item/menu-item.component.tsx b/src/components/menu/menu-item/menu-item.component.tsx
index 95f836410..f96cac81b 100644
--- a/src/components/menu/menu-item/menu-item.component.tsx
+++ b/src/components/menu/menu-item/menu-item.component.tsx
@@ -1,4 +1,10 @@
-import React, { useRef, useEffect, useContext } from "react";
+import React, {
+  useRef,
+  useEffect,
+  useContext,
+  useState,
+  useLayoutEffect,
+} from "react";
 import {
   FlexboxProps,
   LayoutProps,
@@ -180,9 +186,17 @@ export const MenuItem = ({
     ? submenuFocusId === menuItemId.current
     : undefined;
 
-  const ref = useRef<HTMLAnchorElement>(null);
-  const firstFocusableChild =
-    ref.current?.querySelector<HTMLElement>(focusableSelectors) ?? null;
+  const [ref, setRef] = useState<HTMLAnchorElement | null>(null);
+  const [firstFocusableChild, setFirstFocusableChild] =
+    useState<HTMLElement | null>(null);
+
+  useLayoutEffect(() => {
+    const firstFocusable =
+      ref?.querySelector<HTMLElement>(focusableSelectors) ?? null;
+    if (firstFocusable !== firstFocusableChild) {
+      setFirstFocusableChild(firstFocusable);
+    }
+  }, [ref]);
 
   useEffect(() => {
     const id = menuItemId.current;
@@ -207,7 +221,7 @@ export const MenuItem = ({
         return;
       }
 
-      ref.current?.focus();
+      ref?.focus();
     }
   }, [firstFocusableChild, focusFromMenu, focusFromSubmenu]);
 
@@ -230,7 +244,7 @@ export const MenuItem = ({
     onKeyDown?.(event);
 
     if (Events.isEscKey(event)) {
-      ref.current?.focus();
+      ref?.focus();
     }
 
     handleSubmenuKeyDown?.(event);
@@ -247,7 +261,7 @@ export const MenuItem = ({
     selected,
     onKeyDown: !inFullscreenView ? handleKeyDown : undefined,
     overrideColor,
-    ref,
+    ref: setRef,
   };
 
   if (
@@ -303,9 +317,7 @@ export const MenuItem = ({
   }
 
   const paddingProps = filterStyledSystemPaddingProps(rest);
-  const hasInput = !!ref.current?.querySelector<HTMLElement>(
-    "[data-element='input']",
-  );
+  const hasInput = !!ref?.querySelector<HTMLElement>("[data-element='input']");
 
   return (
     <StyledMenuItem


Test patch:
diff --git a/src/components/link/link.test.tsx b/src/components/link/link.test.tsx
index 96a472e3f..57ba2e39e 100644
--- a/src/components/link/link.test.tsx
+++ b/src/components/link/link.test.tsx
@@ -553,3 +553,30 @@ describe("link display styling", () => {
     expect(linkElement).not.toHaveStyle(`display: inline-block`);
   });
 });
+
+test("accepts ref as a ref object", () => {
+  const mockRef = { current: null };
+  render(<Link href="#" ref={mockRef} />);
+
+  const link = screen.getByRole("link");
+
+  expect(mockRef.current).toBe(link);
+});
+
+test("accepts ref as a ref callback", () => {
+  const mockRef = jest.fn();
+  render(<Link href="#" ref={mockRef} />);
+
+  const link = screen.getByRole("link");
+
+  expect(mockRef).toHaveBeenCalledWith(link);
+});
+
+test("sets ref to empty after unmount", () => {
+  const mockRef = { current: null };
+  const { unmount } = render(<Link />);
+
+  unmount();
+
+  expect(mockRef.current).toBe(null);
+});