diff --git a/.changeset/two-dogs-smash.md b/.changeset/two-dogs-smash.md new file mode 100644 index 0000000000..c6d4d342ae --- /dev/null +++ b/.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. diff --git a/packages/confirmation-modal/src/ConfirmationModal/ConfirmationModal.spec.tsx b/packages/confirmation-modal/src/ConfirmationModal/ConfirmationModal.spec.tsx index 803bee5336..4eb5609bdb 100644 --- a/packages/confirmation-modal/src/ConfirmationModal/ConfirmationModal.spec.tsx +++ b/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,6 +329,10 @@ describe('packages/confirmation-modal', () => { , ); + const rerenderedConfirmationButton = await findByTestId( + lgIds.confirm, + ); + if (requiredInputText) { textInput = getByLabelText( `Type "${requiredInputText}" to confirm your action`, @@ -336,7 +340,7 @@ describe('packages/confirmation-modal', () => { expect(textInput).toHaveValue(''); } - expect(confirmationButton).toHaveAttribute( + expect(rerenderedConfirmationButton).toHaveAttribute( 'aria-disabled', disabledAfterReopeningModal.toString(), ); diff --git a/packages/marketing-modal/src/MarketingModal/MarketingModal.spec.tsx b/packages/marketing-modal/src/MarketingModal/MarketingModal.spec.tsx index bea688bf08..ad2dcdbd31 100644 --- a/packages/marketing-modal/src/MarketingModal/MarketingModal.spec.tsx +++ b/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', () => { diff --git a/packages/modal/src/Modal/Modal.spec.tsx b/packages/modal/src/Modal/Modal.spec.tsx index 1c15c94567..1d64eeb3b1 100644 --- a/packages/modal/src/Modal/Modal.spec.tsx +++ b/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(); }); }); diff --git a/packages/modal/src/Modal/ModalView.tsx b/packages/modal/src/Modal/ModalView.tsx index dd24a26574..ee5eca27d1 100644 --- a/packages/modal/src/Modal/ModalView.tsx +++ b/packages/modal/src/Modal/ModalView.tsx @@ -108,16 +108,23 @@ const ModalView = React.forwardRef( // @ts-ignore React17 - `onCancel` is unavailable in React 17 types onCancel={e => e.preventDefault()} > - {children} - - {/* Backdrop portal container for floating elements that need to escape dialog overflow */} - {open && dialogEl && ( -
+ {open && ( + <> + {children} + + {/* Backdrop portal container for floating elements that need to escape dialog overflow */} + {dialogEl && ( +
+ )} + )}