fix(tabs): memoize tabRef array so it regenerate when children change

Updates implementation of `tabRefs` to `useMemo`, this means they will regenerate if the children
change at all

fix #4498

src/components/tabs/tabs.component.js

@@ -3,8 +3,12 @@ import React, {
   useContext,
   useEffect,
   useLayoutEffect,
+  useMemo,
   useRef,
   useState,
+  createRef,
+  cloneElement,
+  Children,
 } from "react";
 import PropTypes from "prop-types";
 import styledSystemPropTypes from "@styled-system/prop-types";
@@ -37,7 +41,19 @@ const Tabs = ({
   headerWidth,
   ...rest
 }) => {
-  const tabRefs = useRef([]);
+  /** The children nodes converted into an Array */
+  const filteredChildren = useMemo(
+    () => Children.toArray(children).filter((child) => child),
+    [children]
+  );
+
+  /** Array of refs to the TabTitle nodes */
+  const tabRefs = useMemo(
+    () =>
+      Array.from({ length: filteredChildren.length }).map(() => createRef()),
+    [filteredChildren.length]
+  );
+
   const previousSelectedTabId = useRef(selectedTabId);
   const [selectedTabIdState, setSelectedTabIdState] = useState();
   const { isInSidebar } = useContext(DrawerSidebarContext);
@@ -47,7 +63,7 @@ const Tabs = ({
 
   useLayoutEffect(() => {
     const selectedTab =
-      selectedTabId || React.Children.toArray(children)[0].props.tabId;
+      selectedTabId || Children.toArray(children)[0].props.tabId;
 
     setSelectedTabIdState(selectedTab);
     // eslint-disable-next-line react-hooks/exhaustive-deps
@@ -106,13 +122,7 @@ const Tabs = ({
   };
 
   /** Focuses the tab for the reference specified */
-  const focusTab = (ref) => ref.focus();
-
-  /** The children nodes converted into an Array */
-  const filteredChildren = React.useMemo(
-    () => React.Children.toArray(children).filter((child) => child),
-    [children]
-  );
+  const focusTab = (ref) => ref.current.focus();
 
   /** Array of the tabIds for the child nodes */
   const tabIds = () => {
@@ -132,7 +142,7 @@ const Tabs = ({
       newIndex = 0;
     }
     const nextTabId = ids[newIndex];
-    const nextRef = tabRefs.current[newIndex];
+    const nextRef = tabRefs[newIndex];
     updateVisibleTab(nextTabId);
     focusTab(nextRef);
   };
@@ -156,12 +166,6 @@ const Tabs = ({
   /** Returns true/false for if the given tab id is selected. */
   const isTabSelected = (tabId) => tabId === selectedTabIdState;
 
-  const addRef = (ref) => {
-    if (ref && !tabRefs.current.includes(ref)) {
-      tabRefs.current.push(ref);
-    }
-  };
-
   /** Build the headers for the tab component */
   const renderTabHeaders = () => {
     const tabTitles = filteredChildren.map((child, index) => {
@@ -218,7 +222,7 @@ const Tabs = ({
           key={tabId}
           onClick={handleTabClick}
           onKeyDown={handleKeyDown(index)}
-          ref={(node) => addRef(node)}
+          ref={tabRefs[index]}
           tabIndex={isTabSelected(tabId) ? "0" : "-1"}
           title={title}
           href={href}
@@ -270,7 +274,7 @@ const Tabs = ({
     });
 
     return tab
-      ? React.cloneElement(tab, {
+      ? cloneElement(tab, {
           isTabSelected: isTabSelected(tab.props.tabId),
         })
       : null;
@@ -285,7 +289,7 @@ const Tabs = ({
     }
 
     const tabs = filteredChildren.map((child) => {
-      return React.cloneElement(child, {
+      return cloneElement(child, {
         ...child.props,
         role: "tabpanel",
         position,

src/components/tabs/tabs.spec.js

@@ -9,6 +9,7 @@ import { TabContext } from "./tab/index";
 import { rootTagTest } from "../../__internal__/utils/helpers/tags/tags-specs";
 import StyledTabs from "./tabs.style";
 import StyledTab from "./tab/tab.style";
+import { StyledTabTitle } from "./__internal__/tab-title/tab-title.style";
 import {
   assertStyleMatch,
   simulate,
@@ -734,6 +735,96 @@ describe("Tabs", () => {
         expect(tabTitle.at(1).props().info).toEqual(false);
       });
     });
+
+    describe("Keyboard behaviour", () => {
+      let container;
+      let wrapper;
+      const tabTitles = ["tab-1", "tab-2", "tab-3"];
+
+      const ConditionalChildrenMock = () => {
+        const [showAllTabs, setShowAllTabs] = React.useState(true);
+
+        const generateTab = (tabTitle) => (
+          <Tab title={tabTitle} tabId={tabTitle} key={tabTitle}>
+            {tabTitle}
+          </Tab>
+        );
+
+        return (
+          <>
+            <button
+              id="foo"
+              type="button"
+              onClick={() => setShowAllTabs((prev) => !prev)}
+            >
+              Toggle children
+            </button>
+            <Tabs>
+              {!showAllTabs && generateTab(tabTitles[0])}
+              {showAllTabs &&
+                tabTitles.map((tabTitle) => generateTab(tabTitle))}
+            </Tabs>
+          </>
+        );
+      };
+
+      beforeEach(() => {
+        container = document.createElement("div");
+        container.id = "enzymeContainer";
+        document.body.appendChild(container);
+
+        wrapper = mount(<ConditionalChildrenMock />, {
+          attachTo: document.getElementById("enzymeContainer"),
+        });
+      });
+
+      afterEach(() => {
+        if (container?.parentNode) {
+          container.parentNode.removeChild(container);
+        }
+
+        container = null;
+      });
+
+      const runFocusExpectations = (keyDown, array) =>
+        array.forEach((index) => {
+          const child = wrapper.update().find(StyledTabTitle).at(index);
+          expect(child.getDOMNode()).toBeFocused();
+          simulate.keydown[keyDown](child);
+        });
+
+      const toggleChildren = () => {
+        act(() => {
+          wrapper.find("#foo").prop("onClick")();
+        });
+
+        expect(wrapper.update().find(StyledTabTitle).length).toEqual(1);
+
+        act(() => {
+          wrapper.find("#foo").prop("onClick")();
+        });
+
+        expect(wrapper.update().find(StyledTabTitle).length).toEqual(3);
+      };
+
+      it("is consistent when navigating with the arrow keys and the composition of the children changes", () => {
+        wrapper.find(StyledTabTitle).first().getDOMNode().focus();
+
+        runFocusExpectations("pressLeftArrow", [0, 2, 1, 0, 2]);
+
+        toggleChildren();
+
+        wrapper.find(StyledTabTitle).first().getDOMNode().focus();
+
+        runFocusExpectations("pressLeftArrow", [0, 2, 1, 0, 2]);
+
+        toggleChildren();
+
+        wrapper.find(StyledTabTitle).first().getDOMNode().focus();
+
+        runFocusExpectations("pressRightArrow", [0, 1, 2, 0]);
+      });
+    });
   });
 
   describe("tags", () => {