fix(focus-trap): update trap to use ref from content container

Removes div wrapper from `FocusTrap` and replaces with fragement, now uses ref for the content container via `wrapperRef` prop fix #3770
Sage · Mar 4, 2021 · e59b5f4 · e59b5f4
1 parent 076c234
commit e59b5f4
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 106 deletions.
diff --git a/src/__internal__/focus-trap/focus-trap.component.js b/src/__internal__/focus-trap/focus-trap.component.js
@@ -1,4 +1,4 @@
-import React, {
+import {
   useCallback,
   useEffect,
   useLayoutEffect,
@@ -19,8 +19,8 @@ const FocusTrap = ({
   autoFocus = true,
   focusFirstElement,
   bespokeTrap,
+  wrapperRef,
 }) => {
-  const ref = useRef();
   const firstOpen = useRef(true);
   const [focusableElements, setFocusableElements] = useState();
   const [firstElement, setFirstElement] = useState();
@@ -38,16 +38,20 @@ const FocusTrap = ({
   );
 
   useLayoutEffect(() => {
-    const elements = Array.from(
-      ref.current.querySelectorAll(defaultFocusableSelectors)
-    ).filter((el) => Number(el.tabIndex) !== -1);
-
-    if (hasNewInputs(elements)) {
-      setFocusableElements(Array.from(elements));
-      setFirstElement(elements[0]);
-      setLastElement(elements[elements.length - 1]);
+    if (wrapperRef) {
+      const ref = wrapperRef.current || wrapperRef;
+
+      const elements = Array.from(
+        ref.querySelectorAll(defaultFocusableSelectors)
+      ).filter((el) => Number(el.tabIndex) !== -1);
+
+      if (hasNewInputs(elements)) {
+        setFocusableElements(Array.from(elements));
+        setFirstElement(elements[0]);
+        setLastElement(elements[elements.length - 1]);
+      }
     }
-  }, [children, hasNewInputs]);
+  }, [children, hasNewInputs, wrapperRef]);
 
   useEffect(() => {
     if (autoFocus && firstOpen.current && (focusFirstElement || firstElement)) {
@@ -101,7 +105,7 @@ const FocusTrap = ({
     };
   }, [firstElement, lastElement, focusableElements, bespokeTrap]);
 
-  return <div ref={ref}>{children}</div>;
+  return children;
 };
 
 FocusTrap.propTypes = {
@@ -115,6 +119,8 @@ FocusTrap.propTypes = {
   ]),
   /** a custom callback that will override the default focus trap behaviour */
   bespokeTrap: PropTypes.func,
+  /** a ref to the container wrapping the focusable elements */
+  wrapperRef: PropTypes.shape({ current: PropTypes.any }),
 };
 
 export default FocusTrap;
diff --git a/src/__internal__/focus-trap/focus-trap.spec.js b/src/__internal__/focus-trap/focus-trap.spec.js
@@ -1,4 +1,4 @@
-import React from "react";
+import React, { useRef } from "react";
 import { mount } from "enzyme";
 
 import FocusTrap from "./focus-trap.component";
@@ -9,6 +9,19 @@ import {
 
 jest.useFakeTimers();
 
+// eslint-disable-next-line
+const MockComponent = ({ children, ...rest }) => {
+  const ref = useRef();
+
+  return (
+    <FocusTrap wrapperRef={ref} {...rest}>
+      <div ref={ref} id="myComponent">
+        {children}
+      </div>
+    </FocusTrap>
+  );
+};
+
 describe("FocusTrap", () => {
   const element = document.createElement("div");
   const htmlElement = document.body.appendChild(element);
@@ -25,12 +38,10 @@ describe("FocusTrap", () => {
 
     beforeEach(() => {
       wrapper = mount(
-        <FocusTrap autoFocus={false}>
-          <div id="myComponent">
-            <button type="button">Test button One</button>
-            <input type="text" />
-          </div>
-        </FocusTrap>,
+        <MockComponent autoFocus={false}>
+          <button type="button">Test button One</button>
+          <input type="text" />
+        </MockComponent>,
         { attachTo: htmlElement }
       );
     });
@@ -56,12 +67,10 @@ describe("FocusTrap", () => {
           document.querySelectorAll("button")[0].focus()
         );
       wrapper = mount(
-        <FocusTrap focusFirstElement={onFocus}>
-          <div id="myComponent">
-            <button type="button">Test button One</button>
-            <button type="button">Test button Two</button>
-          </div>
-        </FocusTrap>,
+        <MockComponent focusFirstElement={onFocus}>
+          <button type="button">Test button One</button>
+          <button type="button">Test button Two</button>
+        </MockComponent>,
         { attachTo: htmlElement }
       );
     });
@@ -116,12 +125,10 @@ describe("FocusTrap", () => {
     beforeEach(() => {
       bespokeFn = jest.fn();
       mount(
-        <FocusTrap bespokeTrap={bespokeFn}>
-          <div id="myComponent">
-            <button type="button">Test button One</button>
-            <button type="button">Test button Two</button>
-          </div>
-        </FocusTrap>,
+        <MockComponent bespokeTrap={bespokeFn}>
+          <button type="button">Test button One</button>
+          <button type="button">Test button Two</button>
+        </MockComponent>,
         { attachTo: htmlElement }
       );
     });
@@ -151,12 +158,10 @@ describe("FocusTrap", () => {
 
       beforeEach(() => {
         wrapper = mount(
-          <FocusTrap>
-            <div id="myComponent">
-              <button type="button">Test button One</button>
-              <button type="button">Test button Two</button>
-            </div>
-          </FocusTrap>,
+          <MockComponent>
+            <button type="button">Test button One</button>
+            <button type="button">Test button Two</button>
+          </MockComponent>,
           { attachTo: htmlElement }
         );
       });
@@ -221,11 +226,9 @@ describe("FocusTrap", () => {
 
       beforeEach(() => {
         wrapper = mount(
-          <FocusTrap>
-            <div id="myComponent">
-              <p>Test content</p>
-            </div>
-          </FocusTrap>,
+          <MockComponent>
+            <p>Test content</p>
+          </MockComponent>,
           { attachTo: htmlElement }
         );
       });
@@ -248,18 +251,16 @@ describe("FocusTrap", () => {
 
       beforeEach(() => {
         wrapper = mount(
-          <FocusTrap>
-            <div id="myComponent">
-              <button type="button">Test button One</button>
-              <button type="button" disabled>
-                Disabled button One
-              </button>
-              <button type="button">Test button Two</button>
-              <button type="button" disabled>
-                Disabled button two
-              </button>
-            </div>
-          </FocusTrap>,
+          <MockComponent>
+            <button type="button">Test button One</button>
+            <button type="button" disabled>
+              Disabled button One
+            </button>
+            <button type="button">Test button Two</button>
+            <button type="button" disabled>
+              Disabled button two
+            </button>
+          </MockComponent>,
           { attachTo: htmlElement }
         );
       });
@@ -283,31 +284,25 @@ describe("FocusTrap", () => {
 
     beforeEach(() => {
       wrapper = mount(
-        <FocusTrap>
-          <div id="myComponent">
-            <RadioButtonGroup
-              name="mybuttongroup"
-              legend="How do you want to create this address?"
-              legendInline
-              onChange={() => jest.fn()}
-              value="1"
-              legendWidth={40}
-            >
-              <RadioButton
-                value="1"
-                label="Create a new Address"
-                size="large"
-              />
-              <RadioButton
-                value="2"
-                label="Select an Existing address"
-                size="large"
-              />
-            </RadioButtonGroup>
-            <button type="button">Test button One</button>
-            <button type="button">Test button Two</button>
-          </div>
-        </FocusTrap>,
+        <MockComponent>
+          <RadioButtonGroup
+            name="mybuttongroup"
+            legend="How do you want to create this address?"
+            legendInline
+            onChange={() => jest.fn()}
+            value="1"
+            legendWidth={40}
+          >
+            <RadioButton value="1" label="Create a new Address" size="large" />
+            <RadioButton
+              value="2"
+              label="Select an Existing address"
+              size="large"
+            />
+          </RadioButtonGroup>
+          <button type="button">Test button One</button>
+          <button type="button">Test button Two</button>
+        </MockComponent>,
         { attachTo: htmlElement }
       );
     });
@@ -364,31 +359,25 @@ describe("FocusTrap", () => {
 
     beforeEach(() => {
       wrapper = mount(
-        <FocusTrap>
-          <div id="myComponent">
-            <button type="button">Test button One</button>
-            <RadioButtonGroup
-              name="mybuttongroup"
-              legend="How do you want to create this address?"
-              legendInline
-              onChange={() => jest.fn()}
-              value="1"
-              legendWidth={40}
-            >
-              <RadioButton
-                value="1"
-                label="Create a new Address"
-                size="large"
-              />
-              <RadioButton
-                value="2"
-                label="Select an Existing address"
-                size="large"
-              />
-            </RadioButtonGroup>
-            <button type="button">Test button Two</button>
-          </div>
-        </FocusTrap>,
+        <MockComponent>
+          <button type="button">Test button One</button>
+          <RadioButtonGroup
+            name="mybuttongroup"
+            legend="How do you want to create this address?"
+            legendInline
+            onChange={() => jest.fn()}
+            value="1"
+            legendWidth={40}
+          >
+            <RadioButton value="1" label="Create a new Address" size="large" />
+            <RadioButton
+              value="2"
+              label="Select an Existing address"
+              size="large"
+            />
+          </RadioButtonGroup>
+          <button type="button">Test button Two</button>
+        </MockComponent>,
         { attachTo: htmlElement }
       );
     });

diff --git a/src/components/dialog-full-screen/dialog-full-screen.component.js b/src/components/dialog-full-screen/dialog-full-screen.component.js
@@ -102,7 +102,7 @@ class DialogFullScreen extends Modal {
    */
   get modalHTML() {
     return (
-      <FocusTrap>
+      <FocusTrap wrapperRef={this._dialog}>
         <StyledDialogFullScreen
           ref={(d) => {
             this._dialog = d;

diff --git a/src/components/dialog/dialog.component.js b/src/components/dialog/dialog.component.js
@@ -266,6 +266,7 @@ class Dialog extends Modal {
         autoFocus={!this.props.disableAutoFocus}
         focusFirstElement={this.props.focusFirstElement}
         bespokeTrap={this.props.bespokeFocusTrap}
+        wrapperRef={this._dialog}
       >
         {this.renderDialog(dialogProps)}
       </FocusTrap>

diff --git a/src/components/sidebar/sidebar.component.js b/src/components/sidebar/sidebar.component.js
@@ -68,7 +68,9 @@ class Sidebar extends Modal {
     if (this.props.enableBackgroundUI) {
       return this.renderSidebar();
     }
-    return <FocusTrap>{this.renderSidebar()}</FocusTrap>;
+    return (
+      <FocusTrap wrapperRef={this.sideBarRef}>{this.renderSidebar()}</FocusTrap>
+    );
   }
 }