[LG-5601] fix(modal): restore conditional rendering of children when closed (#3189)

* fix(modal): conditionally render children when open toggles

* chore(modal): changeset

* fix(modal): tests

Author: stephl3

.changeset/two-dogs-smash.md

@@ -0,0 +1,5 @@
+---
+'@leafygreen-ui/modal': patch
+---
+
+[LG-5601](https://jira.mongodb.org/browse/LG-5601) Fix regression where `Modal` children remained mounted when closed. Restores v19 behavior where children are conditionally rendered based on the `open` prop.

packages/confirmation-modal/src/ConfirmationModal/ConfirmationModal.spec.tsx

@@ -74,8 +74,8 @@ describe('packages/confirmation-modal', () => {
   });
 
   test('does not render if closed', () => {
-    const { getByText } = renderModal();
-    expect(getByText('Content text')).not.toBeVisible();
+    const { queryByText } = renderModal();
+    expect(queryByText('Content text')).toBeNull();
   });
 
   test('renders if open', () => {
@@ -329,14 +329,18 @@ describe('packages/confirmation-modal', () => {
             </ConfirmationModal>,
           );
 
+          const rerenderedConfirmationButton = await findByTestId(
+            lgIds.confirm,
+          );
+
           if (requiredInputText) {
             textInput = getByLabelText(
               `Type "${requiredInputText}" to confirm your action`,
             );
             expect(textInput).toHaveValue('');
           }
 
-          expect(confirmationButton).toHaveAttribute(
+          expect(rerenderedConfirmationButton).toHaveAttribute(
             'aria-disabled',
             disabledAfterReopeningModal.toString(),
           );

packages/marketing-modal/src/MarketingModal/MarketingModal.spec.tsx

@@ -77,8 +77,8 @@ describe('packages/marketing-modal', () => {
     });
   });
   test('does not render if closed', () => {
-    const { getByText } = renderModal();
-    expect(getByText('Content text')).not.toBeVisible();
+    const { queryByText } = renderModal();
+    expect(queryByText('Content text')).toBeNull();
   });
 
   test('renders if open', () => {

packages/modal/src/Modal/Modal.spec.tsx

@@ -253,11 +253,13 @@ describe('packages/modal', () => {
   });
 
   describe('when "open" prop is false', () => {
-    test('renders to DOM but dialog is not open', () => {
-      const { queryModal } = renderModal({ open: false });
+    test('renders dialog element but children are not rendered', () => {
+      const { queryModal, queryByText } = renderModal({ open: false });
       const modal = queryModal();
+      expect(modal).toBeInTheDocument();
       expect(modal).not.toHaveAttribute('open');
       expect(modal).not.toBeVisible();
+      expect(queryByText(modalContent)).not.toBeInTheDocument();
     });
   });
 

packages/modal/src/Modal/ModalView.tsx

@@ -108,16 +108,23 @@ const ModalView = React.forwardRef<HTMLDialogElement, ModalProps>(
           // @ts-ignore React17 - `onCancel` is unavailable in React 17 types
           onCancel={e => e.preventDefault()}
         >
-          {children}
-          <CloseButton
-            data-lgid={lgIds.close}
-            data-testid={lgIds.close}
-            closeIconColor={closeIconColor}
-            onClick={handleClose}
-          />
-          {/* Backdrop portal container for floating elements that need to escape dialog overflow */}
-          {open && dialogEl && (
-            <div className={portalContainerStyles} ref={setPortalContainerEl} />
+          {open && (
+            <>
+              {children}
+              <CloseButton
+                data-lgid={lgIds.close}
+                data-testid={lgIds.close}
+                closeIconColor={closeIconColor}
+                onClick={handleClose}
+              />
+              {/* Backdrop portal container for floating elements that need to escape dialog overflow */}
+              {dialogEl && (
+                <div
+                  className={portalContainerStyles}
+                  ref={setPortalContainerEl}
+                />
+              )}
+            </>
           )}
         </dialog>
       </LeafyGreenProvider>