From 86faf46c4acc28fd93cf63b9536de8bb1d8641d3 Mon Sep 17 00:00:00 2001 From: EduardF1 Date: Sat, 30 May 2026 02:32:17 +0200 Subject: [PATCH 1/8] fix(AlertDialog): fire onCancel when dialog is dismissed via Escape key When a user presses Escape to close an AlertDialog, the onCancel callback was not being invoked. This happened because Escape is handled by the Modal/Tray overlay (which calls state.close() directly) and bypasses the cancel button's onPress handler that chains onClose() + onCancel(). Fix: Provide a custom DialogContext value inside AlertDialog that injects an onKeyDown handler into the dialog's merged props. When the dialog's
element receives a keydown event for Escape and an onCancel prop was provided, the onCancel callback fires before the overlay closes the dialog. The onKeyDown is merged into the dialog element's props via the existing useDialog(mergeProps(contextProps, props)) call in Dialog.tsx, which passes through all valid DOM attributes (including onKeyDown) from context. Adds two new tests: - Escape fires onCancel when the prop is provided - No unexpected calls when onCancel is not provided Fixes #1773 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../react-spectrum/src/dialog/AlertDialog.tsx | 19 +++++++- .../test/dialog/AlertDialog.test.js | 46 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx index e93f0ae8325..8eab7af9df1 100644 --- a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx @@ -65,7 +65,8 @@ export const AlertDialog = forwardRef(function AlertDialog( props: SpectrumAlertDialogProps, ref: DOMRef ) { - let {onClose = () => {}} = useContext(DialogContext) || ({} as DialogContextValue); + let outerContext = useContext(DialogContext) || ({} as DialogContextValue); + let {onClose = () => {}} = outerContext; let { variant, @@ -94,7 +95,22 @@ export const AlertDialog = forwardRef(function AlertDialog( } } + // Provide a context that forwards the onCancel callback when Escape dismisses the dialog. + // The Escape key is handled by the overlay (Modal/Tray), which calls state.close() directly + // and bypasses the cancel button's onPress handler. By injecting an onKeyDown into the + // dialog's context we ensure onCancel fires for keyboard-initiated dismissals too. + let contextWithEscapeCancel: DialogContextValue = { + ...outerContext, + onKeyDown: (e: React.KeyboardEvent) => { + if (e.key === 'Escape' && props.onCancel) { + props.onCancel(); + } + outerContext.onKeyDown?.(e); + } + }; + return ( + + ); }); diff --git a/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js b/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js index 9a85f56405a..27c4e2ec5d3 100644 --- a/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js +++ b/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js @@ -246,4 +246,50 @@ describe('AlertDialog', function () { let primaryBtn = getByTestId('rsp-AlertDialog-confirmButton'); expect(primaryBtn).toBeDefined(); }); + + it('fires onCancel when Escape key is pressed', async function () { + let user = userEvent.setup({delay: null, pointerMap}); + let onCancelSpy = jest.fn(); + let {getByRole} = render( + + + Content body + + + ); + + let dialog = getByRole('alertdialog'); + expect(document.activeElement).toBe(dialog); + + await user.keyboard('{Escape}'); + expect(onCancelSpy).toHaveBeenCalledTimes(1); + }); + + it('does not fire onCancel on Escape when onCancel prop is not provided', async function () { + let user = userEvent.setup({delay: null, pointerMap}); + let onPrimaryAction = jest.fn(); + let {getByRole} = render( + + + Content body + + + ); + + let dialog = getByRole('alertdialog'); + expect(document.activeElement).toBe(dialog); + + // Should not throw or call anything unexpected + await user.keyboard('{Escape}'); + expect(onPrimaryAction).toHaveBeenCalledTimes(0); + }); }); From 2d9e05a7df7ea796a84ee65132d1671728196814 Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:07:17 +0200 Subject: [PATCH 2/8] fix(dialog): forward onKeyDown from DialogContext to section element filterDOMProps strips event handlers, so onKeyDown injected via DialogContext (e.g., by AlertDialog for Escape key handling) was lost. Explicitly extract and merge contextOnKeyDown into dialogProps so it reaches the rendered
element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/@adobe/react-spectrum/src/dialog/Dialog.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx index 4bdcc4f298c..0a89bd517aa 100644 --- a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx @@ -55,7 +55,7 @@ let sizeMap = { */ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProps, ref: DOMRef) { props = useSlotProps(props, 'dialog'); - let {type = 'modal', ...contextProps} = useContext(DialogContext) || ({} as DialogContextValue); + let {type = 'modal', onKeyDown: contextOnKeyDown, ...contextProps} = useContext(DialogContext) || ({} as DialogContextValue); let { children, isDismissable = contextProps.isDismissable, @@ -71,7 +71,8 @@ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProp let domRef = useDOMRef(ref); let gridRef = useRef(null); let sizeVariant = sizeMap[type] || sizeMap[size]; - let {dialogProps, titleProps} = useDialog(mergeProps(contextProps, props), domRef); + let {dialogProps: rawDialogProps, titleProps} = useDialog(mergeProps(contextProps, props), domRef); + let dialogProps = contextOnKeyDown ? mergeProps(rawDialogProps, {onKeyDown: contextOnKeyDown}) : rawDialogProps; let hasHeader = useHasChild(`.${styles['spectrum-Dialog-header']}`, unwrapDOMRef(gridRef)); let hasHeading = useHasChild(`.${styles['spectrum-Dialog-heading']}`, unwrapDOMRef(gridRef)); From 8ca904a4b739349377121e4f3d99c2d33b1fd602 Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:07:57 +0200 Subject: [PATCH 3/8] refactor(AlertDialog): use method shorthand for onKeyDown, infer type TypeScript infers the parameter type from DialogContextValue.onKeyDown (React.KeyboardEventHandler), so the explicit annotation is not needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx index 8eab7af9df1..ba60ef394b5 100644 --- a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx @@ -101,7 +101,7 @@ export const AlertDialog = forwardRef(function AlertDialog( // dialog's context we ensure onCancel fires for keyboard-initiated dismissals too. let contextWithEscapeCancel: DialogContextValue = { ...outerContext, - onKeyDown: (e: React.KeyboardEvent) => { + onKeyDown(e) { if (e.key === 'Escape' && props.onCancel) { props.onCancel(); } From ee6026ac42f5e1ce7e44145347e381b9a79a1d71 Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:13:30 +0200 Subject: [PATCH 4/8] style: arrow function for onKeyDown --- packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx index ba60ef394b5..624c5cc3ef5 100644 --- a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx @@ -101,7 +101,7 @@ export const AlertDialog = forwardRef(function AlertDialog( // dialog's context we ensure onCancel fires for keyboard-initiated dismissals too. let contextWithEscapeCancel: DialogContextValue = { ...outerContext, - onKeyDown(e) { + onKeyDown: e => { if (e.key === 'Escape' && props.onCancel) { props.onCancel(); } From 8434a7b1fa64d360e0286c1e5999b25088c8225a Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:14:05 +0200 Subject: [PATCH 5/8] fix(Dialog): wrap long lines per formatter requirements --- packages/@adobe/react-spectrum/src/dialog/Dialog.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx index 0a89bd517aa..12637106369 100644 --- a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx @@ -55,7 +55,8 @@ let sizeMap = { */ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProps, ref: DOMRef) { props = useSlotProps(props, 'dialog'); - let {type = 'modal', onKeyDown: contextOnKeyDown, ...contextProps} = useContext(DialogContext) || ({} as DialogContextValue); + let {type = 'modal', onKeyDown: contextOnKeyDown, ...contextProps} = + useContext(DialogContext) || ({} as DialogContextValue); let { children, isDismissable = contextProps.isDismissable, @@ -72,7 +73,9 @@ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProp let gridRef = useRef(null); let sizeVariant = sizeMap[type] || sizeMap[size]; let {dialogProps: rawDialogProps, titleProps} = useDialog(mergeProps(contextProps, props), domRef); - let dialogProps = contextOnKeyDown ? mergeProps(rawDialogProps, {onKeyDown: contextOnKeyDown}) : rawDialogProps; + let dialogProps = contextOnKeyDown + ? mergeProps(rawDialogProps, {onKeyDown: contextOnKeyDown}) + : rawDialogProps; let hasHeader = useHasChild(`.${styles['spectrum-Dialog-header']}`, unwrapDOMRef(gridRef)); let hasHeading = useHasChild(`.${styles['spectrum-Dialog-heading']}`, unwrapDOMRef(gridRef)); From 342fa3e83fc4a884453a6849460cdc4626591bda Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:20:03 +0200 Subject: [PATCH 6/8] fix(AlertDialog): indent Dialog block inside DialogContext.Provider --- .../react-spectrum/src/dialog/AlertDialog.tsx | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx index 624c5cc3ef5..54ac621e20a 100644 --- a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx @@ -111,54 +111,54 @@ export const AlertDialog = forwardRef(function AlertDialog( return ( - - {title} - {(variant === 'error' || variant === 'warning') && ( - - )} - - {children} - - {cancelLabel && ( - + + {title} + {(variant === 'error' || variant === 'warning') && ( + + )} + + {children} + + {cancelLabel && ( + + )} + {secondaryActionLabel && ( + + )} - )} - - - + + ); }); From d88668347441974b6e6c801a1aea13dce061bc73 Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:21:11 +0200 Subject: [PATCH 7/8] fix(Dialog): use if-block for contextOnKeyDown merge to keep line lengths in check --- packages/@adobe/react-spectrum/src/dialog/Dialog.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx index 12637106369..ce6d2610e79 100644 --- a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx @@ -72,10 +72,10 @@ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProp let domRef = useDOMRef(ref); let gridRef = useRef(null); let sizeVariant = sizeMap[type] || sizeMap[size]; - let {dialogProps: rawDialogProps, titleProps} = useDialog(mergeProps(contextProps, props), domRef); - let dialogProps = contextOnKeyDown - ? mergeProps(rawDialogProps, {onKeyDown: contextOnKeyDown}) - : rawDialogProps; + let {dialogProps, titleProps} = useDialog(mergeProps(contextProps, props), domRef); + if (contextOnKeyDown) { + dialogProps = mergeProps(dialogProps, {onKeyDown: contextOnKeyDown}); + } let hasHeader = useHasChild(`.${styles['spectrum-Dialog-header']}`, unwrapDOMRef(gridRef)); let hasHeading = useHasChild(`.${styles['spectrum-Dialog-heading']}`, unwrapDOMRef(gridRef)); From 1beb002395d9c3561b7f84f7bb43e948dc4f05fb Mon Sep 17 00:00:00 2001 From: EduardF1 <50618110+EduardF1@users.noreply.github.com> Date: Sat, 30 May 2026 04:26:43 +0200 Subject: [PATCH 8/8] fix(Dialog): expand context destructuring to fit print width --- packages/@adobe/react-spectrum/src/dialog/Dialog.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx index ce6d2610e79..b6f0392e458 100644 --- a/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/Dialog.tsx @@ -55,8 +55,11 @@ let sizeMap = { */ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProps, ref: DOMRef) { props = useSlotProps(props, 'dialog'); - let {type = 'modal', onKeyDown: contextOnKeyDown, ...contextProps} = - useContext(DialogContext) || ({} as DialogContextValue); + let { + type = 'modal', + onKeyDown: contextOnKeyDown, + ...contextProps + } = useContext(DialogContext) || ({} as DialogContextValue); let { children, isDismissable = contextProps.isDismissable,