[ARGG-851] Fix withScrimmedPortal component for deterministic order o…

…f execution (#3060) * [ARGG-851] Fix withScrimmedPortal component for deterministic order of execution * [ARGG-851] Fix withScrimmedPortal component for deterministic order of execution * [ARGG-851] Fix withScrimmedPortal component for deterministic order of execution * [ARGG-851] Fix withScrimmedPortal component for deterministic order of execution * [ARGG-851] Fix withScrimmedPortal component for deterministic order of execution
Skyscanner · Nov 6, 2023 · 769b239 · 769b239
1 parent 9a21730
commit 769b239
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 45 deletions.
diff --git a/packages/bpk-scrim-utils/README.md b/packages/bpk-scrim-utils/README.md
@@ -17,4 +17,30 @@ const Box = props => (
 );
 
 const BoxWithScrim = withScrim(Box);
-```
+```
+
+The version using a [React portal](https://react.dev/reference/react-dom/createPortal) renders the wrapped component in a different part of the DOM. It also provides an `isPortalReady` prop to notify when the component inside the portal is ready to be used. This may be necessary to interact with the content of the component in a `useEffect` hook, for example to set the focus on mount.
+
+```js
+import { withScrimmedPortal } from '@skyscanner/backpack-web/bpk-scrim-utils';
+
+const Box = props => {
+  const dialogRef = useRef(null);
+  const { isPortalReady, onClose } = props;
+
+  useEffect(() => {
+    if (isPortalReady) {
+      dialogRef.current?.focus();
+    }
+  }, [isPortalReady]);
+
+  return (
+    <div>
+      <BpkButton ref={dialogRef} onClick={onClose}>Close</BpkButton>
+      Hello in a portal
+    </div>
+  );
+};
+
+const BoxWithScrimmedPortal = withScrimmedPortal(Box);
+```
diff --git a/packages/bpk-scrim-utils/src/__snapshots__/withScrimmedPortal-test.tsx.snap b/packages/bpk-scrim-utils/src/__snapshots__/withScrimmedPortal-test.tsx.snap
@@ -14,17 +14,15 @@ exports[`withScrimmedPortal renders the wrapped component inside a portal correc
       </div>
     </div>
   </div>
-  <div>
+  <div
+    class="bpk-scrim-content"
+  >
     <div
-      class="bpk-scrim-content"
-    >
-      <div
-        class="bpk-scrim"
-        role="presentation"
-      />
-      <div>
-        Dialog content
-      </div>
+      class="bpk-scrim"
+      role="presentation"
+    />
+    <div>
+      Dialog content
     </div>
   </div>
 </body>
@@ -47,17 +45,15 @@ exports[`withScrimmedPortal renders the wrapped component inside a portal with r
       <div
         id="modal-container"
       >
-        <div>
+        <div
+          class="bpk-scrim-content"
+        >
           <div
-            class="bpk-scrim-content"
-          >
-            <div
-              class="bpk-scrim"
-              role="presentation"
-            />
-            <div>
-              Wrapped Component
-            </div>
+            class="bpk-scrim"
+            role="presentation"
+          />
+          <div>
+            Wrapped Component
           </div>
         </div>
       </div>

diff --git a/packages/bpk-scrim-utils/src/withScrimmedPortal-test.tsx b/packages/bpk-scrim-utils/src/withScrimmedPortal-test.tsx
@@ -16,9 +16,12 @@
  * limitations under the License.
  */
 
-import { render, within } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { render, within, screen } from '@testing-library/react';
+import { useEffect, useState } from 'react';
 
 import withScrimmedPortal from './withScrimmedPortal';
+import type { Props } from './withScrimmedPortal';
 
 describe('withScrimmedPortal', () => {
   it('renders the wrapped component inside a portal correctly with fallback to document.body', () => {
@@ -73,4 +76,33 @@ describe('withScrimmedPortal', () => {
     const hiddenElements = document.getElementById('pagewrap');
     expect(within(hiddenElements as HTMLElement).queryByText('Wrapped Component')).toBeNull();
   });
-});
+
+  it('notifies the child component when the portal is ready', () => {
+    const WrappedComponent = ({ isPortalReady }: Props) => {
+      const [portalStatus, setPortalStatus] = useState('');
+      useEffect(() => {
+        if (isPortalReady) {
+          setPortalStatus(`${portalStatus} portal is now ready`);
+        } else {
+          setPortalStatus(`${portalStatus} portal is not ready yet /`);
+        }
+      }, [isPortalReady]);
+
+      const content = `Wrapped Component /${portalStatus}`;
+
+      return <div>{content}</div>;
+    };
+
+    const ScrimmedComponent = withScrimmedPortal(WrappedComponent);
+    render(
+      <div id="pagewrap">
+        <div> Content hidden from AT</div>
+        <ScrimmedComponent
+          getApplicationElement={() => document.getElementById('pagewrap')}
+        />
+      </div>
+    );
+
+    expect(screen.getByText('Wrapped Component / portal is not ready yet / portal is now ready')).toBeInTheDocument();
+  });
+});
diff --git a/packages/bpk-scrim-utils/src/withScrimmedPortal.tsx b/packages/bpk-scrim-utils/src/withScrimmedPortal.tsx
@@ -17,7 +17,7 @@
  */
 
 import type { ComponentType} from 'react';
-import { useEffect, useRef } from 'react';
+import { useState, useEffect } from 'react';
 import { createPortal } from 'react-dom';
 
 import withScrim from './withScrim';
@@ -29,11 +29,11 @@ export type Props = ScrimProps & {
 
 const getPortalElement = (target: (() => HTMLElement | null) | null | undefined) => {
     const portalElement = target && typeof target === 'function' ? target() : null;
-    
+
     if (portalElement) {
         return portalElement;
     }
-    
+
     if (document.body) {
         return document.body;
     }
@@ -44,29 +44,17 @@ const withScrimmedPortal = (WrappedComponent: ComponentType<ScrimProps>) => {
     const Scrimmed = withScrim(WrappedComponent);
 
     const ScrimmedComponent = ({ renderTarget, ...rest}: Props) => {
-        const node = useRef<HTMLDivElement | null>(null);
+        const portalElement = getPortalElement(renderTarget);
 
-        if (!node.current) {
-            node.current = document.createElement('div');            
-        }
+        const [isPortalReady, setIsPortalReady] = useState(false);
 
         useEffect(() => {
-            const portalElement = getPortalElement(renderTarget);
-
-            if (node.current) {
-                portalElement.appendChild(node.current);
-            }
-
-            return () => {
-              if (node.current) {
-                portalElement.removeChild(node.current);
-              }
-            };
+            setIsPortalReady(true);
           }, []);
-        
-        return createPortal(<Scrimmed {...rest} />, node.current);
-        }
-    
+
+        return createPortal(<Scrimmed {...rest} isPortalReady={isPortalReady} />, portalElement);
+    }
+
     return ScrimmedComponent;
 }