fix: Render modal outside app root by default. (#1890)

* Render modal outside app root by default. * Render to a modal root if one exists. * Disable proptype linting Co-authored-by: haworku <[email protected]>
trussworks · Jan 27, 2022 · 0828351 · 0828351
1 parent 3c326be
commit 0828351
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 47 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -59,6 +59,7 @@
     "plugin:security/recommended"
   ],
   "rules": {
-    "no-only-tests/no-only-tests": "error"
+    "no-only-tests/no-only-tests": "error",
+    "react/prop-types": "off"
   }
 }
diff --git a/src/components/Modal/Modal.stories.tsx b/src/components/Modal/Modal.stories.tsx
@@ -48,39 +48,41 @@ export const defaultModal = (): React.ReactElement => {
   const modalRef = useRef<ModalRef>()
 
   return (
-    <>
-      <ModalToggleButton modalRef={modalRef} opener>
-        Open default modal
-      </ModalToggleButton>
-      <Modal
-        ref={modalRef}
-        id="example-modal-1"
-        aria-labelledby="modal-1-heading"
-        aria-describedby="modal-1-description">
-        <ModalHeading id="modal-1-heading">
-          Are you sure you want to continue?
-        </ModalHeading>
-        <div className="usa-prose">
-          <p id="modal-1-description">
-            You have unsaved changes that will be lost.
-          </p>
-        </div>
-        <ModalFooter>
-          <ButtonGroup>
-            <ModalToggleButton modalRef={modalRef} closer>
-              Continue without saving
-            </ModalToggleButton>
-            <ModalToggleButton
-              modalRef={modalRef}
-              closer
-              unstyled
-              className="padding-105 text-center">
-              Go back
-            </ModalToggleButton>
-          </ButtonGroup>
-        </ModalFooter>
-      </Modal>
-    </>
+    <div>
+      <div>
+        <ModalToggleButton modalRef={modalRef} opener>
+          Open default modal
+        </ModalToggleButton>
+        <Modal
+          ref={modalRef}
+          id="example-modal-1"
+          aria-labelledby="modal-1-heading"
+          aria-describedby="modal-1-description">
+          <ModalHeading id="modal-1-heading">
+            Are you sure you want to continue?
+          </ModalHeading>
+          <div className="usa-prose">
+            <p id="modal-1-description">
+              You have unsaved changes that will be lost.
+            </p>
+          </div>
+          <ModalFooter>
+            <ButtonGroup>
+              <ModalToggleButton modalRef={modalRef} closer>
+                Continue without saving
+              </ModalToggleButton>
+              <ModalToggleButton
+                modalRef={modalRef}
+                closer
+                unstyled
+                className="padding-105 text-center">
+                Go back
+              </ModalToggleButton>
+            </ButtonGroup>
+          </ModalFooter>
+        </Modal>
+      </div>
+    </div>
   )
 }
 

diff --git a/src/components/Modal/Modal.test.tsx b/src/components/Modal/Modal.test.tsx
@@ -1,5 +1,6 @@
 import React, { createRef, useRef } from 'react'
 import {
+  cleanup,
   render,
   screen,
   waitFor,
@@ -29,12 +30,19 @@ const renderWithModalRoot = (
   ui: React.ReactElement,
   options: RenderOptions = {}
 ) => {
+  const appContainer = document.createElement("div")
+  appContainer.setAttribute("id", "app-root")
+
   const modalContainer = document.createElement('div')
   modalContainer.setAttribute('id', 'modal-root')
 
+  document.body.appendChild(appContainer)
+  document.body.appendChild(modalContainer)
+
   return render(ui, {
     ...options,
-    container: document.body.appendChild(modalContainer),
+    container: appContainer,
+    baseElement: document.body,
   })
 }
 
@@ -138,13 +146,15 @@ const ExampleModalWithFocusElement = (): React.ReactElement => {
 
 describe('Modal component', () => {
   beforeEach(() => {
+    cleanup()
+    document.body.innerHTML = ''
     document.body.style.paddingRight = '0px'
   })
 
   it('renders its children inside a modal wrapper', () => {
     const testModalId = 'testModal'
 
-    render(<Modal id={testModalId}>Test modal</Modal>)
+    renderWithModalRoot(<Modal id={testModalId}>Test modal</Modal>)
 
     // Modal wrapper
     const modalWrapper = screen.getByRole('dialog')
@@ -171,7 +181,7 @@ describe('Modal component', () => {
   it('passes aria props to the modal wrapper', () => {
     const testModalId = 'testModal'
 
-    render(
+    renderWithModalRoot(
       <Modal
         id={testModalId}
         aria-labelledby="modal-label"
@@ -239,7 +249,7 @@ describe('Modal component', () => {
     const modalRef = createRef<ModalRef>()
     const handleOpen = () => modalRef.current?.toggleModal(undefined, true)
 
-    render(
+    renderWithModalRoot(
       <Modal id={testModalId} ref={modalRef}>
         Test modal
       </Modal>
@@ -256,7 +266,7 @@ describe('Modal component', () => {
   it('renders a large modal window when isLarge is true', () => {
     const testModalId = 'testModal'
 
-    render(
+    renderWithModalRoot(
       <Modal id={testModalId} isLarge>
         Test modal
       </Modal>
@@ -268,7 +278,7 @@ describe('Modal component', () => {
   it('does not render a close button when forceAction is true', () => {
     const testModalId = 'testModal'
 
-    render(
+    renderWithModalRoot(
       <Modal id={testModalId} forceAction>
         Test modal
       </Modal>
@@ -292,7 +302,7 @@ describe('Modal component', () => {
       const handleOpen = () => modalRef.current?.toggleModal(undefined, true)
       const handleClose = () => modalRef.current?.toggleModal(undefined, false)
 
-      const { baseElement } = render(
+      const { baseElement } = renderWithModalRoot(
         <Modal id="testModal" ref={modalRef}>
           Test modal
         </Modal>
@@ -318,7 +328,7 @@ describe('Modal component', () => {
       const handleClose = () => modalRef.current?.toggleModal(undefined, false)
       document.body.style.paddingRight = '20px'
 
-      const { baseElement } = render(
+      const { baseElement } = renderWithModalRoot(
         <Modal id="testModal" ref={modalRef}>
           Test modal
         </Modal>
@@ -441,7 +451,7 @@ describe('Modal component', () => {
       const modalRef = createRef<ModalRef>()
       const handleOpen = () => modalRef.current?.toggleModal(undefined, true)
 
-      render(
+      renderWithModalRoot(
         <Modal id="testModal" ref={modalRef}>
           Test modal
         </Modal>
@@ -579,7 +589,7 @@ describe('Modal component', () => {
         const handleClose = () =>
           modalRef.current?.toggleModal(undefined, false)
 
-        const { baseElement } = render(
+        const { baseElement } = renderWithModalRoot(
           <Modal id="testModal" ref={modalRef} forceAction>
             {testModalChildren}
           </Modal>
@@ -602,7 +612,7 @@ describe('Modal component', () => {
 
         const testModalId = 'testModal'
 
-        render(
+        renderWithModalRoot(
           <Modal id={testModalId} ref={modalRef} forceAction>
             {testModalChildren}
           </Modal>

diff --git a/src/components/Modal/Modal.tsx b/src/components/Modal/Modal.tsx
@@ -10,6 +10,7 @@ import FocusTrap from 'focus-trap-react'
 import { useModal, getScrollbarWidth } from './utils'
 import { ModalWindow } from './ModalWindow/ModalWindow'
 import { ModalWrapper } from './ModalWrapper/ModalWrapper'
+import ReactDOM from 'react-dom'
 
 interface ModalComponentProps {
   id: string
@@ -18,6 +19,7 @@ interface ModalComponentProps {
   isLarge?: boolean
   forceAction?: boolean
   modalRoot?: string
+  renderToPortal?: boolean
 }
 
 export type ModalProps = ModalComponentProps & JSX.IntrinsicElements['div']
@@ -28,6 +30,12 @@ export type ModalRef = {
   toggleModal: (event?: React.MouseEvent, open?: boolean) => boolean
 }
 
+// Modals are rendered into the document body default. If an element exists with the id
+// `modal-root`, that element will be used as the parent instead.
+// 
+// If you wish to override this behavior, `renderToPortal` to `false` and the modal
+// will render in its normal location in the document. Note that this may cause the modal to
+// be inaccessible due to no longer being in the document's accessbility tree.
 export const Modal = forwardRef(
   (
     {
@@ -36,6 +44,7 @@ export const Modal = forwardRef(
       isLarge = false,
       forceAction = false,
       modalRoot = '.usa-modal-wrapper',
+      renderToPortal = true,
       ...divProps
     }: ModalProps,
     ref: React.Ref<ModalRef>
@@ -162,7 +171,7 @@ export const Modal = forwardRef(
       },
     }
 
-    return (
+    const modal = 
       <FocusTrap active={isOpen} focusTrapOptions={focusTrapOptions}>
         <ModalWrapper
           role="dialog"
@@ -185,7 +194,17 @@ export const Modal = forwardRef(
           </ModalWindow>
         </ModalWrapper>
       </FocusTrap>
-    )
+
+    if (renderToPortal) {
+      const modalRoot = document.getElementById("modal-root")
+      const target = modalRoot || document.body
+      return ReactDOM.createPortal(
+        modal,
+        target
+      )
+    } else {
+      return modal
+    }
   }
 )