From a2da3d19689101e4d4167af8206516d1e09c8650 Mon Sep 17 00:00:00 2001 From: Albert Yu Date: Mon, 6 Jan 2025 18:34:27 +0800 Subject: [PATCH] [Dialog] Don't call `onNestedDialogOpen` when unmounting a closed nested dialog (#1280) --- .../src/dialog/popup/DialogPopup.test.tsx | 162 +++++++++++++++++- .../react/src/dialog/root/useDialogRoot.ts | 4 - 2 files changed, 161 insertions(+), 5 deletions(-) diff --git a/packages/react/src/dialog/popup/DialogPopup.test.tsx b/packages/react/src/dialog/popup/DialogPopup.test.tsx index 4c6ae9a404..13249250cf 100644 --- a/packages/react/src/dialog/popup/DialogPopup.test.tsx +++ b/packages/react/src/dialog/popup/DialogPopup.test.tsx @@ -2,9 +2,11 @@ import * as React from 'react'; import { expect } from 'chai'; import { Dialog } from '@base-ui-components/react/dialog'; import { AlertDialog } from '@base-ui-components/react/alert-dialog'; -import { act, waitFor, screen } from '@mui/internal-test-utils'; +import { act, describeSkipIf, waitFor, screen } from '@mui/internal-test-utils'; import { describeConformance, createRenderer } from '#test-utils'; +const isJSDOM = /jsdom/.test(window.navigator.userAgent); + describe('', () => { const { render } = createRenderer(); @@ -203,6 +205,164 @@ describe('', () => { }); }); + describeSkipIf(isJSDOM)('nested dialog count', () => { + it('provides the number of open nested dialogs as a CSS variable', async () => { + const { user } = await render( + + Trigger 0 + + + + Trigger 1 + + + + Trigger 2 + + + Close 2 + + + + Close 1 + + + + + + , + ); + + await user.click(screen.getByRole('button', { name: 'Trigger 0' })); + + await waitFor(() => { + expect(screen.getByTestId('popup0')).not.to.equal(null); + }); + + const computedStyles = getComputedStyle(screen.getByTestId('popup0')); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('0'); + + await user.click(screen.getByRole('button', { name: 'Trigger 1' })); + + await waitFor(() => { + expect(screen.getByTestId('popup1')).not.to.equal(null); + }); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('1'); + + await user.click(screen.getByRole('button', { name: 'Trigger 2' })); + + await waitFor(() => { + expect(screen.getByTestId('popup2')).not.to.equal(null); + }); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('2'); + + await user.click(screen.getByRole('button', { name: 'Close 2' })); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('1'); + + await user.click(screen.getByRole('button', { name: 'Close 1' })); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('0'); + }); + + it('decrements the count when an open nested dialog is unmounted', async () => { + function App() { + const [showNested, setShowNested] = React.useState(true); + return ( + + + + Trigger 0 + + + {showNested && ( + + Trigger 1 + + + Close 1 + + + + )} + Close 0 + + + + + ); + } + + const { user } = await render(); + + await user.click(screen.getByRole('button', { name: 'Trigger 0' })); + + await waitFor(() => { + expect(screen.getByTestId('popup0')).not.to.equal(null); + }); + + const computedStyles = getComputedStyle(screen.getByTestId('popup0')); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('0'); + + await user.click(screen.getByRole('button', { name: 'Trigger 1' })); + + await waitFor(() => { + expect(screen.getByTestId('popup1')).not.to.equal(null); + }); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('1'); + + await user.click(screen.getByRole('button', { name: 'toggle' })); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('0'); + }); + + it('does not change the count when a closed nested dialog is unmounted', async () => { + function App() { + const [showNested, setShowNested] = React.useState(true); + return ( + + Trigger 0 + + + {showNested && ( + + + + + + + )} + + Close 0 + + + + ); + } + + const { user } = await render(); + + await user.click(screen.getByRole('button', { name: 'Trigger 0' })); + + await waitFor(() => { + expect(screen.getByTestId('popup0')).not.to.equal(null); + }); + + const computedStyles = getComputedStyle(screen.getByTestId('popup0')); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('0'); + + await user.click(screen.getByRole('button', { name: 'toggle' })); + + expect(computedStyles.getPropertyValue('--nested-dialogs')).to.equal('0'); + }); + }); + describe('style hooks', () => { it('adds the `nested` and `has-nested-dialogs` style hooks if a dialog has a parent dialog', async () => { await render( diff --git a/packages/react/src/dialog/root/useDialogRoot.ts b/packages/react/src/dialog/root/useDialogRoot.ts index 0f373c8e92..b6da8e8cde 100644 --- a/packages/react/src/dialog/root/useDialogRoot.ts +++ b/packages/react/src/dialog/root/useDialogRoot.ts @@ -102,10 +102,6 @@ export function useDialogRoot(parameters: useDialogRoot.Parameters): useDialogRo if (onNestedDialogClose && open) { onNestedDialogClose(); } - - if (onNestedDialogOpen && !open) { - onNestedDialogOpen(ownNestedOpenDialogs); - } }; }, [open, onNestedDialogClose, onNestedDialogOpen, ownNestedOpenDialogs]);