From d9e042534205f518bb2f281d7684eed773171e70 Mon Sep 17 00:00:00 2001 From: woksin Date: Sat, 24 Oct 2026 11:30:31 +0200 Subject: [PATCH 1/2] Guard dialog dismissal around portaled popovers --- .../primereact-package/component-reference.md | 2 + .../for_dialog/when_dismissing_a_dialog.tsx | 133 ++++++++++++++++++ .../primereact/overlay/PrimeDialog.tsx | 21 ++- 3 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx diff --git a/Documentation/primereact-package/component-reference.md b/Documentation/primereact-package/component-reference.md index 34aceb03..6fb8cb24 100644 --- a/Documentation/primereact-package/component-reference.md +++ b/Documentation/primereact-package/component-reference.md @@ -113,6 +113,8 @@ from the earlier .NET chart enum, and reports any other type instead of drawing | `sidebar` | `overlay/PrimeSidebar.tsx` | `primereact/button`, `primereact/drawer` | | `tooltip` | `overlay/PrimeTooltip.tsx` | `primereact/tooltip` | +For `PrimeReact:dialog`, set `element.properties.dismissable` to `false` to prevent outside clicks from closing the dialog. It defaults to `true`. The close button and Escape key remain available, and interactions inside portaled content such as a calendar popover do not dismiss the dialog. + ## Menu | Name | Adapter | Backed by | diff --git a/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx b/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx new file mode 100644 index 00000000..e618552d --- /dev/null +++ b/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx @@ -0,0 +1,133 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { createPortal } from 'react-dom'; +import { PrimeReactProvider } from '@primereact/core'; +import { PrimeDialog } from '../overlay/PrimeDialog'; +import { PrimeCalendar } from '../form/PrimeCalendar'; +import { sceneComponent } from '../storyElements'; + +function renderDialog(properties: Record = {}) { + render(, + createPortal(, document.body), + ] }} />); +} + +const backdrop = () => document.querySelector('[data-scope="dialog"][data-part="backdrop"]')!; +const positioner = () => document.querySelector('[data-scope="dialog"][data-part="positioner"]')!; +const pointerClick = (target: HTMLElement) => { fireEvent.pointerDown(target); fireEvent.pointerUp(target); }; +const shouldBeClosed = () => waitFor(() => (screen.queryByRole('dialog', { name: 'Edit order' }) === null).should.equal(true)); +const shouldBeOpen = () => { + const dialog = screen.getByRole('dialog', { name: 'Edit order' }); + dialog.hasAttribute('data-open').should.equal(true); + return dialog; +}; + +describe('when dismissing a dialog', () => { + beforeEach(() => vi.stubGlobal('ResizeObserver', class { + observe = vi.fn(); + unobserve = vi.fn(); + disconnect = vi.fn(); + })); + afterEach(() => vi.unstubAllGlobals()); + + it('should dismiss on the backdrop by default', async () => { + renderDialog(); + pointerClick(backdrop()); + await shouldBeClosed(); + }); + + it('should dismiss on the empty positioner by default', async () => { + renderDialog(); + pointerClick(positioner()); + await shouldBeClosed(); + }); + + it('should allow outside dismissal when explicitly enabled', async () => { + renderDialog({ dismissable: true }); + pointerClick(backdrop()); + await shouldBeClosed(); + }); + + it('should retain the dialog and its input when outside dismissal is disabled', () => { + renderDialog({ dismissable: false }); + fireEvent.change(screen.getByRole('textbox', { name: 'Name' }), { target: { value: 'Unsaved draft' } }); + pointerClick(backdrop()); + pointerClick(positioner()); + shouldBeOpen(); + (screen.getByRole('textbox', { name: 'Name' }) as HTMLInputElement).value.should.equal('Unsaved draft'); + }); + + it('should keep ordinary dialog interactions inside', () => { + renderDialog(); + pointerClick(screen.getByRole('textbox', { name: 'Name' })); + shouldBeOpen(); + }); + + it('should keep interactions inside a portaled popover inside the dialog', () => { + renderDialog(); + const option = screen.getByRole('button', { name: 'Portaled option' }); + shouldBeOpen().contains(option).should.equal(false); + pointerClick(option); + shouldBeOpen(); + }); + + it('should not dismiss when a pointer press on the backdrop ends inside a portaled popover', () => { + renderDialog(); + fireEvent.pointerDown(backdrop()); + fireEvent.pointerUp(screen.getByRole('button', { name: 'Portaled option' })); + shouldBeOpen(); + }); + + it('should not dismiss when a pointer press on the positioner ends inside a portaled popover', () => { + renderDialog(); + fireEvent.pointerDown(positioner()); + fireEvent.pointerUp(screen.getByRole('button', { name: 'Portaled option' })); + shouldBeOpen(); + }); + + it('should not dismiss when a pointer press inside the dialog ends outside', () => { + renderDialog(); + fireEvent.pointerDown(screen.getByRole('textbox', { name: 'Name' })); + fireEvent.pointerUp(positioner()); + shouldBeOpen(); + }); + + it('should not dismiss after a cancelled outside pointer press', () => { + renderDialog(); + fireEvent.pointerDown(positioner()); + fireEvent.pointerCancel(positioner()); + fireEvent.pointerUp(positioner()); + shouldBeOpen(); + }); + + it('should keep the explicit close button available when outside dismissal is disabled', async () => { + renderDialog({ dismissable: false }); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + await shouldBeClosed(); + }); + + it('should keep Escape dismissal available when outside dismissal is disabled', async () => { + renderDialog({ dismissable: false }); + fireEvent.keyDown(document, { key: 'Escape' }); + await shouldBeClosed(); + }); + + it('should keep a real portaled calendar selection from closing the dialog', async () => { + render(, + ] }} />); + fireEvent.click(screen.getByRole('combobox', { name: 'Due date' })); + const day = await screen.findByText('16'); + shouldBeOpen().contains(day).should.equal(false); + fireEvent.pointerDown(positioner()); + fireEvent.pointerUp(day); + shouldBeOpen(); + pointerClick(day); + fireEvent.click(day); + shouldBeOpen(); + (screen.getByRole('combobox', { name: 'Due date' }) as HTMLInputElement).value.should.contain('16'); + }); +}); diff --git a/Source/JavaScript/primereact/overlay/PrimeDialog.tsx b/Source/JavaScript/primereact/overlay/PrimeDialog.tsx index 79f16bc8..5e6ab727 100644 --- a/Source/JavaScript/primereact/overlay/PrimeDialog.tsx +++ b/Source/JavaScript/primereact/overlay/PrimeDialog.tsx @@ -1,7 +1,7 @@ // Copyright (c) Cratis. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -import { useState } from 'react'; +import { useRef, useState, type PointerEvent } from 'react'; import { Button } from 'primereact/button'; import { Dialog } from 'primereact/dialog'; import { RegisteredComponentProps } from '@cratis/scene.react'; @@ -31,6 +31,19 @@ import { booleanProperty, stringProperty } from '../properties'; export function PrimeDialog({ element, slots }: RegisteredComponentProps) { const [visible, setVisible] = useState(booleanProperty(element, 'visible', false)); const header = stringProperty(element, 'header', 'Dialog'); + const dismissable = booleanProperty(element, 'dismissable', true); + const pointerDownTarget = useRef(null); + // Only a press and release on the dialog's own outside surface may dismiss it. + // React events from portaled children still bubble through the positioner. + const outsidePointerProps = { + onPointerDown: (event: PointerEvent) => { pointerDownTarget.current = event.target; }, + onPointerUp: (event: PointerEvent) => { + const outsideClick = event.target === event.currentTarget && pointerDownTarget.current === event.currentTarget; + pointerDownTarget.current = null; + if (dismissable && outsideClick) setVisible(false); + }, + onPointerCancel: () => { pointerDownTarget.current = null; }, + }; return (
@@ -38,14 +51,14 @@ export function PrimeDialog({ element, slots }: RegisteredComponentProps) { open={visible} modal={booleanProperty(element, 'modal', true)} draggable={booleanProperty(element, 'draggable', false)} - dismissable + dismissable={false} closeOnEscape onOpenChange={(event) => { if (!event.value) setVisible(false); }}> - - + + {header} From 6083d076274db37dedbcf466667784f54fc8aa12 Mon Sep 17 00:00:00 2001 From: woksin Date: Sat, 24 Oct 2026 11:45:56 +0200 Subject: [PATCH 2/2] Honor dialog Escape options and popover hit targets --- .../primereact-package/component-reference.md | 2 +- ...hen_clicking_a_portaled_popover.browser.ts | 130 ++++++++++++++++++ .../for_dialog/when_dismissing_a_dialog.tsx | 29 +++- .../primereact/overlay/PrimeDialog.tsx | 15 +- 4 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 Source/JavaScript/primereact/for_dialog/when_clicking_a_portaled_popover.browser.ts diff --git a/Documentation/primereact-package/component-reference.md b/Documentation/primereact-package/component-reference.md index 6fb8cb24..b96e8417 100644 --- a/Documentation/primereact-package/component-reference.md +++ b/Documentation/primereact-package/component-reference.md @@ -113,7 +113,7 @@ from the earlier .NET chart enum, and reports any other type instead of drawing | `sidebar` | `overlay/PrimeSidebar.tsx` | `primereact/button`, `primereact/drawer` | | `tooltip` | `overlay/PrimeTooltip.tsx` | `primereact/tooltip` | -For `PrimeReact:dialog`, set `element.properties.dismissable` to `false` to prevent outside clicks from closing the dialog. It defaults to `true`. The close button and Escape key remain available, and interactions inside portaled content such as a calendar popover do not dismiss the dialog. +For `PrimeReact:dialog`, set `element.properties.dismissable` to `false` to prevent outside clicks from closing the dialog, or `element.properties.closeOnEscape` to `false` to prevent Escape from closing it. Both properties default to `true` and work independently. The close button remains available. Interactions at a portaled Cratis popover do not dismiss the dialog, even if the dialog layer covers the popover at the click point; this guard does not change the popover's stacking order. ## Menu diff --git a/Source/JavaScript/primereact/for_dialog/when_clicking_a_portaled_popover.browser.ts b/Source/JavaScript/primereact/for_dialog/when_clicking_a_portaled_popover.browser.ts new file mode 100644 index 00000000..d4a4a663 --- /dev/null +++ b/Source/JavaScript/primereact/for_dialog/when_clicking_a_portaled_popover.browser.ts @@ -0,0 +1,130 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { build } from 'esbuild'; +import { Browser, Page, chromium } from 'playwright-core'; +import { existsSync } from 'node:fs'; +import { resolve } from 'node:path'; + +const workspace = resolve(import.meta.dirname, '..'); +const javascript = resolve(workspace, '..'); +let browser: Browser; +let script: string; +let css: string; + +beforeAll(async () => { + const bundle = await build({ + stdin: { contents: application, resolveDir: workspace, loader: 'tsx' }, + bundle: true, write: false, outdir: 'out', platform: 'browser', format: 'iife', jsx: 'automatic', + define: { 'process.env.NODE_ENV': '"production"' }, + alias: { + '@cratis/scene.model': resolve(javascript, 'model/index.ts'), + '@cratis/scene.engine': resolve(javascript, 'engine/index.ts'), + '@cratis/scene.react': resolve(javascript, 'react/index.ts'), + }, + }); + script = bundle.outputFiles.find(file => file.path.endsWith('.js'))!.text; + css = bundle.outputFiles.find(file => file.path.endsWith('.css'))!.text; + const executablePath = process.env.CHROMIUM_PATH ?? (existsSync('/usr/bin/chromium') ? '/usr/bin/chromium' : undefined); + browser = await chromium.launch({ headless: true, executablePath, args: ['--no-sandbox'] }); +}); + +afterAll(async () => { await browser?.close(); }); + +async function show(mode: 'covered' | 'real'): Promise { + const page = await browser.newPage(); + page.setDefaultTimeout(5000); + await page.setContent(`Dialog calendar
`); + await page.addStyleTag({ content: css }); + await page.addScriptTag({ content: script }); + await page.getByRole('textbox', { name: 'Name' }).fill('Unsaved draft'); + return page; +} + +const dialog = (page: Page) => page.locator('[data-scope="dialog"][data-part="popup"]'); + +describe('when clicking a portaled popover', () => { + it('should keep the dialog and draft when its positioner covers a Cratis popover at the click point', async () => { + const page = await show('covered'); + try { + await page.getByRole('button', { name: 'Open calendar' }).click(); + const popover = page.locator('[data-cratis-part="popover"]'); + const box = (await popover.boundingBox())!; + const point = { x: box.x + box.width / 2, y: box.y + box.height / 2 }; + const hits = await page.evaluate(({ x, y }) => document.elementsFromPoint(x, y).map(node => ({ + primePart: node.getAttribute('data-part'), + cratisPart: node.getAttribute('data-cratis-part'), + })), point); + hits[0].primePart!.should.equal('positioner'); + hits.some(hit => hit.cratisPart === 'popover').should.equal(true); + // Browser hit-testing dispatches to the covering dialog surface, not the portal node. + await page.mouse.click(point.x, point.y); + ((await dialog(page).getAttribute('data-open')) !== null).should.equal(true); + (await page.getByRole('textbox', { name: 'Name' }).inputValue()).should.equal('Unsaved draft'); + } finally { + await page.close(); + } + }); + + it('should still dismiss for an outside click away from the open popover', async () => { + const page = await show('covered'); + try { + await page.getByRole('button', { name: 'Open calendar' }).click(); + const positioner = page.locator('[data-scope="dialog"][data-part="positioner"]'); + const box = (await positioner.boundingBox())!; + await page.mouse.click(box.x + box.width - 10, box.y + box.height - 10); + await dialog(page).waitFor({ state: 'hidden' }); + } finally { + await page.close(); + } + }); + + it('should select a date with the real Cratis DatePickerInput without dismissing the dialog', async () => { + const page = await show('real'); + try { + await page.locator('[data-cratis-part="trigger"]').click(); + const cell = page.locator('[data-cratis-part="cell"]').filter({ hasText: /^16$/ }); + await cell.click(); + (await page.getByLabel('Selected day').textContent())!.should.equal('16'); + ((await dialog(page).getAttribute('data-open')) !== null).should.equal(true); + (await page.getByRole('textbox', { name: 'Name' }).inputValue()).should.equal('Unsaved draft'); + } finally { + await page.close(); + } + }); +}); + +const application = ` +import { useState } from 'react'; +import { createRoot } from 'react-dom/client'; +import { createPortal } from 'react-dom'; +import { PrimeReactProvider } from '@primereact/core'; +import { styles as dialogStyles } from '@primereact/styles/dialog'; +import Lara from '@primeuix/themes/lara'; +import { DatePickerInput } from '@cratis/components/Common'; +import '@cratis/components/tokens'; +import '@cratis/components/Common/styles'; +import { PrimeDialog } from './overlay/PrimeDialog'; +function CoveredPopover() { + const [open, setOpen] = useState(false); + return <> + + {open && createPortal(, document.body)} + ; +} +function App() { + const [date, setDate] = useState(new Date(2026, 9, 15)); + return + , + document.body.dataset.mode === 'covered' ? : + , + {date.getDate()}, + ] }} /> + ; +} +createRoot(document.getElementById('root')).render(); +`; diff --git a/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx b/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx index e618552d..42d6d6d1 100644 --- a/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx +++ b/Source/JavaScript/primereact/for_dialog/when_dismissing_a_dialog.tsx @@ -109,12 +109,38 @@ describe('when dismissing a dialog', () => { await shouldBeClosed(); }); - it('should keep Escape dismissal available when outside dismissal is disabled', async () => { + it('should dismiss on Escape by default independently of outside dismissal', async () => { renderDialog({ dismissable: false }); fireEvent.keyDown(document, { key: 'Escape' }); await shouldBeClosed(); }); + it('should dismiss on Escape when explicitly enabled', async () => { + renderDialog({ closeOnEscape: true }); + fireEvent.keyDown(document, { key: 'Escape' }); + await shouldBeClosed(); + }); + + it('should retain the dialog and draft when Escape dismissal is disabled', () => { + renderDialog({ closeOnEscape: false }); + fireEvent.change(screen.getByRole('textbox', { name: 'Name' }), { target: { value: 'Unsaved draft' } }); + fireEvent.keyDown(document, { key: 'Escape' }); + shouldBeOpen(); + (screen.getByRole('textbox', { name: 'Name' }) as HTMLInputElement).value.should.equal('Unsaved draft'); + }); + + it('should keep outside dismissal independent of Escape dismissal', async () => { + renderDialog({ closeOnEscape: false }); + pointerClick(positioner()); + await shouldBeClosed(); + }); + + it('should keep the explicit close button available when both dismissal gestures are disabled', async () => { + renderDialog({ dismissable: false, closeOnEscape: false }); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + await shouldBeClosed(); + }); + it('should keep a real portaled calendar selection from closing the dialog', async () => { render(, @@ -125,7 +151,6 @@ describe('when dismissing a dialog', () => { fireEvent.pointerDown(positioner()); fireEvent.pointerUp(day); shouldBeOpen(); - pointerClick(day); fireEvent.click(day); shouldBeOpen(); (screen.getByRole('combobox', { name: 'Due date' }) as HTMLInputElement).value.should.contain('16'); diff --git a/Source/JavaScript/primereact/overlay/PrimeDialog.tsx b/Source/JavaScript/primereact/overlay/PrimeDialog.tsx index 5e6ab727..d20947c4 100644 --- a/Source/JavaScript/primereact/overlay/PrimeDialog.tsx +++ b/Source/JavaScript/primereact/overlay/PrimeDialog.tsx @@ -32,15 +32,18 @@ export function PrimeDialog({ element, slots }: RegisteredComponentProps) { const [visible, setVisible] = useState(booleanProperty(element, 'visible', false)); const header = stringProperty(element, 'header', 'Dialog'); const dismissable = booleanProperty(element, 'dismissable', true); + const closeOnEscape = booleanProperty(element, 'closeOnEscape', true); const pointerDownTarget = useRef(null); // Only a press and release on the dialog's own outside surface may dismiss it. // React events from portaled children still bubble through the positioner. const outsidePointerProps = { - onPointerDown: (event: PointerEvent) => { pointerDownTarget.current = event.target; }, + onPointerDown: (event: PointerEvent) => { pointerDownTarget.current = isPopoverAtPoint(event) ? null : event.target; }, onPointerUp: (event: PointerEvent) => { const outsideClick = event.target === event.currentTarget && pointerDownTarget.current === event.currentTarget; pointerDownTarget.current = null; - if (dismissable && outsideClick) setVisible(false); + if (!dismissable || !outsideClick) return; + if (isPopoverAtPoint(event)) return; + setVisible(false); }, onPointerCancel: () => { pointerDownTarget.current = null; }, }; @@ -52,7 +55,7 @@ export function PrimeDialog({ element, slots }: RegisteredComponentProps) { modal={booleanProperty(element, 'modal', true)} draggable={booleanProperty(element, 'draggable', false)} dismissable={false} - closeOnEscape + closeOnEscape={closeOnEscape} onOpenChange={(event) => { if (!event.value) setVisible(false); }}> @@ -74,3 +77,9 @@ export function PrimeDialog({ element, slots }: RegisteredComponentProps) {
); } + +function isPopoverAtPoint(event: PointerEvent): boolean { + // A dialog layer can receive the event even when it covers a portaled Cratis popover. + const hits = event.currentTarget.ownerDocument.elementsFromPoint?.(event.clientX, event.clientY) ?? []; + return hits.some(node => node.closest('[data-cratis-part="popover"]') !== null); +}