diff --git a/.changeset/confirm-dialog-host-cleanup.md b/.changeset/confirm-dialog-host-cleanup.md new file mode 100644 index 00000000000..27b2c8114e6 --- /dev/null +++ b/.changeset/confirm-dialog-host-cleanup.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +ConfirmationDialog: `useConfirm`/`confirm` now removes its host element from `document.body` after the dialog is closed, and uses a fresh host element per call, so the empty container no longer lingers or leaks into other components and tests diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx index 77d5b7fe520..5c2301017dd 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx @@ -1,4 +1,4 @@ -import {render, fireEvent} from '@testing-library/react' +import {render, fireEvent, waitFor} from '@testing-library/react' import {describe, it, expect, vi} from 'vitest' import type React from 'react' import {useCallback, useRef, useState} from 'react' @@ -311,4 +311,30 @@ describe('ConfirmationDialog', () => { }) implementsClassName(ConfirmationDialog, dialogClasses.Dialog) + + describe('useConfirm', () => { + it('removes the host element from the document body when the dialog is closed', async () => { + const {getByText, getByRole} = render() + + fireEvent.click(getByText('Show menu')) + + // Capture children after the menu opens so we can reliably detect the confirm() host element + const bodyChildrenBeforeDialog = Array.from(document.body.children) + + fireEvent.click(getByText('Show dialog')) + + expect(getByRole('alertdialog')).toBeInTheDocument() + + const hostElement = Array.from(document.body.children).find(el => !bodyChildrenBeforeDialog.includes(el)) + if (!hostElement) throw new Error('Expected confirm() to append a host element to ') + + fireEvent.click(getByRole('button', {name: 'Secondary'})) + + // After closing, neither the dialog nor its host element should linger in the DOM + await waitFor(() => { + expect(document.querySelector('[role="alertdialog"]')).toBeNull() + expect(hostElement.isConnected).toBe(false) + }) + }) + }) }) diff --git a/packages/react/src/ConfirmationDialog/useConfirm.ts b/packages/react/src/ConfirmationDialog/useConfirm.ts index a7acda6000e..fe750bd59a6 100644 --- a/packages/react/src/ConfirmationDialog/useConfirm.ts +++ b/packages/react/src/ConfirmationDialog/useConfirm.ts @@ -3,23 +3,18 @@ import {createRoot} from 'react-dom/client' import BaseStyles from '../BaseStyles' import {ConfirmationDialog, type ConfirmationDialogProps} from './ConfirmationDialog' -let hostElement: Element | null = null - export type ConfirmOptions = Omit & {content: React.ReactNode} async function confirm(options: ConfirmOptions): Promise { const {content, ...confirmationDialogProps} = options return new Promise(resolve => { - hostElement ||= document.createElement('div') - if (!hostElement.isConnected) document.body.append(hostElement) + const hostElement = document.createElement('div') + document.body.append(hostElement) const root = createRoot(hostElement) const onClose: ConfirmationDialogProps['onClose'] = gesture => { root.unmount() - if (gesture === 'confirm') { - resolve(true) - } else { - resolve(false) - } + hostElement.remove() + resolve(gesture === 'confirm') } root.render( React.createElement(