Skip to content

fix(AlertDialog): fire onCancel when dialog is dismissed via Escape key#10136

Closed
EduardF1 wants to merge 1 commit into
adobe:mainfrom
EduardF1:fix/alert-dialog-escape-fires-on-cancel
Closed

fix(AlertDialog): fire onCancel when dialog is dismissed via Escape key#10136
EduardF1 wants to merge 1 commit into
adobe:mainfrom
EduardF1:fix/alert-dialog-escape-fires-on-cancel

Conversation

@EduardF1
Copy link
Copy Markdown

Summary

Fixes #1773

When a user presses Escape to close an AlertDialog, the onCancel callback was not being invoked. This happened because the Escape key is intercepted by the Modal/Tray overlay (which calls state.close() directly) and completely bypasses the cancel button's onPress handler that chains onClose() + onCancel().

Root Cause

In AlertDialog.tsx, the cancel button wires up:
sx onPress={() => chain(onClose(), onCancel())}

But when Escape is pressed, the Modal overlay handles it by calling state.close() directly, never touching onCancel. The AlertDialog had no mechanism to intercept this.

Fix

Provide a custom DialogContext inside AlertDialog that injects an onKeyDown handler. When the dialog's section element receives a keydown event for Escape and onCancel was provided, the callback fires before the overlay closes the dialog.

The onKeyDown reaches the section element via the existing useDialog(mergeProps(contextProps, props)) call in Dialog.tsx, which passes all valid DOM attributes (including onKeyDown) through filterDOMProps into the rendered element.

Tests

Added two new test cases to AlertDialog.test.js:

  • Escape fires onCancel when the prop is provided
  • No unexpected calls when onCancel is not provided

@EduardF1 EduardF1 force-pushed the fix/alert-dialog-escape-fires-on-cancel branch from 1beb002 to d41bf6c Compare June 1, 2026 23:07
@LFDanLu
Copy link
Copy Markdown
Member

LFDanLu commented Jun 3, 2026

@EduardF1 Thanks for the PR, the approach looks good to me. Mind signing the CLA so we can move forward with this? As an aside, we should probably implement this for the Spectrum 2 Alert Dialog as well, but no need to handle that here

@EduardF1 EduardF1 closed this Jun 4, 2026
@EduardF1 EduardF1 force-pushed the fix/alert-dialog-escape-fires-on-cancel branch from d41bf6c to 168242e Compare June 4, 2026 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

onCancel should fire in AlertDialog if Escape is pressed

2 participants