From ae968d48115fbb0e81ade654d394d3640f7be468 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Thu, 29 Oct 2026 15:10:39 +0100 Subject: [PATCH] fix(Modals): respect preventInitialFocus by opening modals after mount --- packages/main/src/components/Modals/index.tsx | 50 +++++++++++++------ .../components/Modals/test/Modals.gallery.tsx | 33 ++++++++++++ .../components/Modals/test/Modals.spec.tsx | 19 +++++++ 3 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 packages/main/src/components/Modals/test/Modals.gallery.tsx create mode 100644 packages/main/src/components/Modals/test/Modals.spec.tsx diff --git a/packages/main/src/components/Modals/index.tsx b/packages/main/src/components/Modals/index.tsx index c9ced506d8c..3b1d7c35e14 100644 --- a/packages/main/src/components/Modals/index.tsx +++ b/packages/main/src/components/Modals/index.tsx @@ -1,9 +1,11 @@ 'use client'; +import { useIsomorphicLayoutEffect, useSyncRef } from '@ui5/webcomponents-react-base/internal/hooks'; import type { MutableRefObject } from 'react'; import { createRef, useEffect, useSyncExternalStore } from 'react'; import { createPortal } from 'react-dom'; import { getRandomId } from '../../internal/getRandomId.js'; +import type { IModal } from '../../internal/ModalStore.js'; import { ModalStore } from '../../internal/ModalStore.js'; import type { DialogDomRef, DialogPropTypes } from '../../webComponents/Dialog/index.js'; import { Dialog } from '../../webComponents/Dialog/index.js'; @@ -84,7 +86,7 @@ function showDialogFn( Component: Dialog, props: { ...props, - open: true, + open: false, onClose: (event) => { if (typeof props.onClose === 'function') { props.onClose(event); @@ -129,7 +131,7 @@ function showPopoverFn( Component: Popover, props: { ...props, - open: true, + open: false, onClose: (event) => { if (typeof props.onClose === 'function') { props.onClose(event); @@ -177,7 +179,7 @@ function showResponsivePopoverFn( Component: ResponsivePopover, props: { ...props, - open: true, + open: false, onClose: (event) => { if (typeof props.onClose === 'function') { props.onClose(event); @@ -219,7 +221,7 @@ function showMenuFn( Component: Menu, props: { ...props, - open: true, + open: false, onClose: (event) => { if (typeof props.onClose === 'function') { props.onClose(event); @@ -262,7 +264,7 @@ function showMessageBoxFn( Component: MessageBox, props: { ...props, - open: true, + open: false, onClose: (event) => { if (typeof props.onClose === 'function') { props.onClose(event); @@ -301,7 +303,7 @@ function showToastFn( Component: Toast, props: { ...props, - open: true, + open: false, onClose: (event) => { if (typeof props.onClose === 'function') { props.onClose(event); @@ -319,6 +321,32 @@ function showToastFn( }; } +// TODO: Temporary workaround for #9116 — the web component resolves its initial focus synchronously in the +// `open` setter instead of when focus is applied. Remove once fixed upstream (UI5/webcomponents#14234). +/** + * Renders a single modal and opens it only after mount, so the web component's `open` setter runs after all + * other props (e.g. `preventInitialFocus`) are applied. Setting `open` in props applies it too early. + */ +function ModalRenderer({ modal }: { modal: IModal }) { + const { Component, props, ref, container, id } = modal; + const [componentRef, modalRef] = useSyncRef(ref); + + useIsomorphicLayoutEffect(() => { + const node = modalRef.current as (Element & { open?: boolean }) | null; + if (node && 'open' in node) { + node.open = true; + } + }, [modalRef]); + + // @ts-expect-error: ref is supported by all supported modals + const element = ; + + if (container) { + return createPortal(element, container); + } + return element; +} + /** * Utility class for opening modals in an imperative way. * @@ -344,15 +372,7 @@ export function Modals() { <> {modals.map((modal) => { if (modal?.Component) { - if (modal.container) { - return createPortal( - // @ts-expect-error: ref is supported by all supported modals - , - modal.container, - ); - } - // @ts-expect-error: ref is supported by all supported modals - return ; + return ; } })} diff --git a/packages/main/src/components/Modals/test/Modals.gallery.tsx b/packages/main/src/components/Modals/test/Modals.gallery.tsx new file mode 100644 index 00000000000..07c6f95de30 --- /dev/null +++ b/packages/main/src/components/Modals/test/Modals.gallery.tsx @@ -0,0 +1,33 @@ +import { useState } from 'react'; +import { Button } from '../../../webComponents/Button/index.js'; +import { Modals } from '../index.js'; + +export const ModalsPreventInitialFocus = () => { + const [opened, setOpened] = useState(false); + return ( + <> + + + ), + }); + }} + > + Open Dialog + + {opened ? 'opened' : 'closed'} + + ); +}; diff --git a/packages/main/src/components/Modals/test/Modals.spec.tsx b/packages/main/src/components/Modals/test/Modals.spec.tsx new file mode 100644 index 00000000000..a0eda458b7b --- /dev/null +++ b/packages/main/src/components/Modals/test/Modals.spec.tsx @@ -0,0 +1,19 @@ +import { expect, test } from '../../../../../../playwright/fixtures/gallery-fixtures.js'; + +test.describe('Modals.showDialog', () => { + test('preventInitialFocus is applied before the dialog opens', async ({ mount, page }) => { + await mount('Modals/ModalsPreventInitialFocus'); + + await page.getByTestId('open-dialog').click(); + + // The `open` event fires only after the dialog applied its initial focus, so waiting + // for it guarantees the focus decision has already been made before we assert on it. + await expect(page.getByTestId('opened-flag')).toHaveText('opened'); + await expect(page.locator('[ui5-dialog]')).toBeVisible(); + + // With preventInitialFocus, focus must not be moved into the dialog... + await expect(page.getByTestId('dialog-close')).not.toBeFocused(); + // ...it stays on the opener instead. + await expect(page.getByTestId('open-dialog')).toBeFocused(); + }); +});