From 5bc2ae3bcf5eb923fb6e35102494bc50f28ed1a4 Mon Sep 17 00:00:00 2001 From: FredrikMWold Date: Thu, 23 Jul 2026 15:03:40 +0200 Subject: [PATCH] api change proposal for dialog component --- .../components/next/Dialog/Dialog.stories.tsx | 29 +- .../src/components/next/Dialog/Dialog.tsx | 280 ++++++++++++------ .../components/next/Dialog/Dialog.types.ts | 8 +- 3 files changed, 205 insertions(+), 112 deletions(-) diff --git a/packages/eds-core-react/src/components/next/Dialog/Dialog.stories.tsx b/packages/eds-core-react/src/components/next/Dialog/Dialog.stories.tsx index c5777c299c..8dd120a7b3 100644 --- a/packages/eds-core-react/src/components/next/Dialog/Dialog.stories.tsx +++ b/packages/eds-core-react/src/components/next/Dialog/Dialog.stories.tsx @@ -33,23 +33,22 @@ See the stories below for usage patterns. export default meta export const Introduction: StoryFn = () => { - const [open, setOpen] = useState(false) return ( <> - - - - Dialog title - - - This is a short description of the action the user is about to take. - - - - - + + Open dialog + + + Dialog title + + + This is a short description of the action the user is about to take. + + + Close + + + ) diff --git a/packages/eds-core-react/src/components/next/Dialog/Dialog.tsx b/packages/eds-core-react/src/components/next/Dialog/Dialog.tsx index 1f5d35afb6..9a1d0f9f1b 100644 --- a/packages/eds-core-react/src/components/next/Dialog/Dialog.tsx +++ b/packages/eds-core-react/src/components/next/Dialog/Dialog.tsx @@ -11,137 +11,214 @@ import { type MouseEvent, } from 'react' import { close as closeIcon } from '@equinor/eds-icons' -import { Button } from '../Button' +import { Button, ButtonProps } from '../Button' import { Icon } from '../Icon' import type { DialogActionsProps, DialogContentProps, DialogHeaderProps, + DialogPopupProps, DialogProps, DialogTitleProps, } from './Dialog.types' +import { Slot } from '../Slot' type DialogContextValue = { titleId: string | undefined registerTitle: (id: string) => () => void close: () => void + open: boolean + onOpenChange: (open: boolean) => void + dialogRef: React.RefObject } const DialogContext = createContext(null) -const DialogRoot = forwardRef(function Dialog( - { - open, - onOpenChange, - scrim = true, - className, - children, - 'aria-labelledby': ariaLabelledBy, - 'aria-label': ariaLabel, - ...rest - }, - ref, -) { +const DialogRoot = ({ + open: openProp, + onOpenChange: onOpenChangeProp, + children, +}: DialogProps) => { const dialogRef = useRef(null) - // Suppresses the onOpenChange call fired by the native `close` event when - // we close the dialog in response to a consumer flipping `open` to false. - // Without this, the consumer's setter would be called once externally and - // once from handleClose — see review thread on PR #4956. - const expectedCloseRef = useRef(false) - // Records whether the mousedown that started a click landed on the dialog - // element itself. Click fires on the common ancestor of mousedown/up, so a - // text-selection drag that overshoots into the backdrop would otherwise be - // mistaken for a backdrop click and close the dialog. - const mouseDownOnDialogRef = useRef(false) const [titleId, setTitleId] = useState(undefined) + const [open, setOpen] = useState(openProp ?? false) - useEffect(() => { - const dialog = dialogRef.current - if (!dialog) return - if (open && !dialog.open) { - dialog.showModal() - } else if (!open && dialog.open) { - expectedCloseRef.current = true - dialog.close() - } - }, [open]) - - const setRef = useCallback( - (node: HTMLDialogElement | null) => { - dialogRef.current = node - if (typeof ref === 'function') ref(node) - else if (ref) ref.current = node - }, - [ref], - ) - - const handleClose = () => { - if (expectedCloseRef.current) { - expectedCloseRef.current = false - return - } - onOpenChange?.(false) - } - - const handleMouseDown = (event: MouseEvent) => { - mouseDownOnDialogRef.current = event.target === dialogRef.current - } - - // Native reports the dialog itself as the click target when the - // backdrop is clicked; children dispatch from their own elements. The - // additional mousedown check guards against drag-out from a selection. - const handleClick = (event: MouseEvent) => { - const wasOnBackdrop = - event.target === dialogRef.current && mouseDownOnDialogRef.current - mouseDownOnDialogRef.current = false - if (wasOnBackdrop) dialogRef.current?.close() - } + const onOpenChange = onOpenChangeProp ?? setOpen const close = useCallback(() => dialogRef.current?.close(), []) + const registerTitle = useCallback((id: string) => { setTitleId(id) return () => setTitleId((current) => (current === id ? undefined : current)) }, []) const ctxValue = useMemo( - () => ({ titleId, registerTitle, close }), - [titleId, registerTitle, close], + () => ({ + titleId, + registerTitle, + close, + open, + onOpenChange, + dialogRef, + }), + [titleId, registerTitle, close, open, onOpenChange, dialogRef], ) - // aria-labelledby resolves to the registered title id when a Dialog.Title is - // present. Explicit aria-labelledby or aria-label on Dialog still wins. - const resolvedAriaLabelledBy = - ariaLabelledBy ?? (ariaLabel ? undefined : titleId) - return ( - - {/* Native doesn't expose ::backdrop as a separately clickable + {children} + ) +} + +const DialogPopup = forwardRef( + function DialogPopup( + { + scrim = true, + className, + children, + 'aria-labelledby': ariaLabelledBy, + 'aria-label': ariaLabel, + ...rest + }, + ref, + ) { + // Records whether the mousedown that started a click landed on the dialog + // element itself. Click fires on the common ancestor of mousedown/up, so a + // text-selection drag that overshoots into the backdrop would otherwise be + // mistaken for a backdrop click and close the dialog. + const mouseDownOnDialogRef = useRef(false) + const ctx = useDialogContext() + const expectedCloseRef = useRef(false) + useEffect(() => { + const dialog = ctx.dialogRef.current + if (!dialog) return + if (ctx.open && !dialog.open) { + dialog.showModal() + } else if (!ctx.open && dialog.open) { + expectedCloseRef.current = true + dialog.close() + } + }, [ctx.open, ctx.dialogRef]) + + const setRef = useCallback( + (node: HTMLDialogElement | null) => { + ctx.dialogRef.current = node + if (typeof ref === 'function') ref(node) + else if (ref) ref.current = node + }, + [ref, ctx], + ) + + const resolvedAriaLabelledBy = + ariaLabelledBy ?? (ariaLabel ? undefined : ctx?.titleId) + const handleClose = () => { + if (expectedCloseRef.current) { + expectedCloseRef.current = false + return + } + ctx?.onOpenChange?.(false) + } + + const handleMouseDown = (event: MouseEvent) => { + mouseDownOnDialogRef.current = event.target === ctx?.dialogRef.current + } + + // Native reports the dialog itself as the click target when the + // backdrop is clicked; children dispatch from their own elements. The + // additional mousedown check guards against drag-out from a selection. + const handleClick = (event: MouseEvent) => { + const wasOnBackdrop = + event.target === ctx?.dialogRef.current && mouseDownOnDialogRef.current + mouseDownOnDialogRef.current = false + if (wasOnBackdrop) ctx?.dialogRef.current?.close() + } + + return ( + <> + {/* Native doesn't expose ::backdrop as a separately clickable element; a click whose target is the dialog itself comes from the backdrop. Keyboard dismissal (Escape) is handled by the native dialog and emits the `close` event we already wire up — no extra key handler. */} - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions */} - + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions */} + + {children} + + + ) + }, +) +DialogPopup.displayName = 'Dialog.Popup' + +const DialogTrigger = forwardRef( + function DialogTrigger( + { className, children, asChild, onClick, ...rest }, + ref, + ) { + const ctx = useDialogContext() + const classes = ['dialog-button', className].filter(Boolean).join(' ') + const onClickHandler = (e: MouseEvent) => { + ctx.onOpenChange(true) + onClick?.(e) + } + + const sharedProps = { + ref, + className: classes, + onClick: onClickHandler, + ...rest, + } + if (asChild) { + return {children} + } + return + }, +) +DialogTrigger.displayName = 'Dialog.Trigger' + +const DialogClose = forwardRef( + function DialogClose( + { className, children, asChild, onClick, ...rest }, + ref, + ) { + const ctx = useDialogContext() + const classes = ['dialog-button', className].filter(Boolean).join(' ') + const onClickHandler = (e: MouseEvent) => { + ctx.close() + onClick?.(e) + } + + const sharedProps = { + ref, + className: classes, + onClick: onClickHandler, + ...rest, + } + if (asChild) { + return {children} + } + + return ( + - - ) -}) -DialogRoot.displayName = 'Dialog' + + ) + }, +) +DialogClose.displayName = 'Dialog.Close' const DialogHeader = forwardRef( function DialogHeader({ children, className, ...rest }, ref) { - const ctx = useContext(DialogContext) + const ctx = useDialogContext() return (
( tone="accent" icon round - onClick={() => ctx?.close()} + onClick={() => ctx.close()} aria-label="Close" > @@ -168,12 +245,11 @@ DialogHeader.displayName = 'Dialog.Header' const DialogTitle = forwardRef( function DialogTitle({ id, className, children, ...rest }, ref) { - const ctx = useContext(DialogContext) + const ctx = useDialogContext() const generatedId = useId() const resolvedId = id ?? generatedId useEffect(() => { - if (!ctx) return return ctx.registerTitle(resolvedId) }, [ctx, resolvedId]) @@ -226,6 +302,9 @@ type CompoundDialog = typeof DialogRoot & { Title: typeof DialogTitle Content: typeof DialogContent Actions: typeof DialogActions + Trigger: typeof DialogTrigger + Popup: typeof DialogPopup + Close: typeof DialogClose } export const Dialog = DialogRoot as CompoundDialog @@ -233,3 +312,14 @@ Dialog.Header = DialogHeader Dialog.Title = DialogTitle Dialog.Content = DialogContent Dialog.Actions = DialogActions +Dialog.Trigger = DialogTrigger +Dialog.Popup = DialogPopup +Dialog.Close = DialogClose + +const useDialogContext = () => { + const ctx = useContext(DialogContext) + if (!ctx) { + throw new Error('Dialog compound components must be wrapped in ') + } + return ctx +} diff --git a/packages/eds-core-react/src/components/next/Dialog/Dialog.types.ts b/packages/eds-core-react/src/components/next/Dialog/Dialog.types.ts index f32f5c1466..38904063b8 100644 --- a/packages/eds-core-react/src/components/next/Dialog/Dialog.types.ts +++ b/packages/eds-core-react/src/components/next/Dialog/Dialog.types.ts @@ -5,13 +5,17 @@ export type DialogProps = { * Controlled open state. When true, the dialog is shown via the native * `HTMLDialogElement.showModal()` (modal mode: focus trap + inert background). */ - open: boolean + open?: boolean /** * Called when the dialog requests to close — via Escape key, backdrop click, * or the header close button. Update your `open` state in response; the * component is fully controlled. */ - onOpenChange: (open: boolean) => void + onOpenChange?: (open: boolean) => void + children: ReactNode +} + +export type DialogPopupProps = { /** * Whether to render a visible backdrop (scrim) behind the dialog. * Defaults to true. Set to false for transparent backdrops.