diff --git a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx index 8be387d4a8a..ff7394c0276 100644 --- a/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx +++ b/packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx @@ -93,6 +93,26 @@ export const AlertDialog = forwardRef(function AlertDialog( } } + // The Escape key is normally handled by the overlay, which closes the dialog without + // pressing the cancel button. Handle it on the dialog itself instead, so that dismissing + // an AlertDialog with the Escape key is equivalent to pressing the cancel button. + let onKeyDown = (e: React.KeyboardEvent) => { + if ( + e.key === 'Escape' && + !e.nativeEvent.isComposing && + !e.nativeEvent.repeat && + !e.altKey && + !e.ctrlKey && + !e.shiftKey && + !e.metaKey + ) { + onClose(); + onCancel(); + e.stopPropagation(); + e.preventDefault(); + } + }; + return ( {title} diff --git a/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js b/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js index 0c840198996..5d314b7a580 100644 --- a/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js +++ b/packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js @@ -10,8 +10,10 @@ * governing permissions and limitations under the License. */ +import {act, pointerMap, render, simulateDesktop} from '@react-spectrum/test-utils-internal'; +import {ActionButton} from '../../src/button/ActionButton'; import {AlertDialog} from '../../src/dialog/AlertDialog'; -import {pointerMap, render} from '@react-spectrum/test-utils-internal'; +import {DialogTrigger} from '../../src/dialog/DialogTrigger'; import {Provider} from '../../src/provider/Provider'; import React from 'react'; import {defaultTheme as theme} from '../../src/theme-default/defaultTheme'; @@ -279,4 +281,139 @@ describe('AlertDialog', function () { expect(getByRole('alertdialog')).toHaveAttribute('aria-describedby', 'content-id'); }); + + it('fires onCancel when the 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); + expect(onCancelSpy).toHaveBeenCalledWith(); + }); + + it('does not fire other handlers on Escape when onCancel 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); + + await user.keyboard('{Escape}'); + expect(onPrimaryAction).toHaveBeenCalledTimes(0); + }); + + describe('within a DialogTrigger', function () { + beforeAll(() => { + jest.useFakeTimers(); + }); + + beforeEach(() => { + simulateDesktop(); + }); + + afterEach(() => { + act(() => jest.runAllTimers()); + }); + + afterAll(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); + }); + + it('fires onCancel once and closes the dialog when the Escape key is pressed', async function () { + let user = userEvent.setup({delay: null, pointerMap}); + let onCancelSpy = jest.fn(); + let {getByRole} = render( + + + Trigger + + Content body + + + + ); + + let button = getByRole('button'); + await user.click(button); + act(() => { + jest.runAllTimers(); + }); + + let dialog = getByRole('alertdialog'); + + await user.keyboard('{Escape}'); + act(() => { + jest.runAllTimers(); + }); + expect(dialog).not.toBeInTheDocument(); + expect(onCancelSpy).toHaveBeenCalledTimes(1); + }); + + it('fires onCancel once when the cancel button is pressed', async function () { + let user = userEvent.setup({delay: null, pointerMap}); + let onCancelSpy = jest.fn(); + let {getByRole, getByText} = render( + + + Trigger + + Content body + + + + ); + + let button = getByRole('button'); + await user.click(button); + act(() => { + jest.runAllTimers(); + }); + + let dialog = getByRole('alertdialog'); + + await user.click(getByText('cancel')); + act(() => { + jest.runAllTimers(); + }); + expect(dialog).not.toBeInTheDocument(); + expect(onCancelSpy).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/@react-spectrum/s2/src/AlertDialog.tsx b/packages/@react-spectrum/s2/src/AlertDialog.tsx index 38d8ee5a0aa..8de3587da89 100644 --- a/packages/@react-spectrum/s2/src/AlertDialog.tsx +++ b/packages/@react-spectrum/s2/src/AlertDialog.tsx @@ -19,10 +19,11 @@ import {chain} from 'react-aria/chain'; import {Content, Heading} from './Content'; import {Dialog} from './Dialog'; import {filterDOMProps} from 'react-aria/filterDOMProps'; -import {forwardRef, ReactNode} from 'react'; +import {forwardRef, KeyboardEvent as ReactKeyboardEvent, ReactNode, useContext} from 'react'; import {IconContext} from './Icon'; import intlMessages from '../intl/*.json'; import NoticeSquare from '../s2wf-icons/S2_Icon_AlertDiamond_20_N.svg'; +import {OverlayTriggerStateContext} from 'react-aria-components/Dialog'; import {Provider} from 'react-aria-components/slots'; import {style} from '../style' with {type: 'macro'}; import {UnsafeStyles} from './style-utils' with {type: 'macro'}; @@ -108,9 +109,37 @@ export const AlertDialog = forwardRef(function AlertDialog(props: AlertDialogPro let domProps = filterDOMProps(props, {labelable: true}); + // The Escape key is normally handled by the modal overlay, which closes the dialog + // without pressing the cancel button. Handle it on the dialog itself instead, so that + // dismissing an AlertDialog with the Escape key is equivalent to pressing the cancel button. + let state = useContext(OverlayTriggerStateContext); + let onKeyDown = (e: ReactKeyboardEvent) => { + if ( + e.key === 'Escape' && + !e.nativeEvent.isComposing && + !e.nativeEvent.repeat && + !e.altKey && + !e.ctrlKey && + !e.shiftKey && + !e.metaKey + ) { + if (state) { + state.close(); + onCancel(); + e.stopPropagation(); + e.preventDefault(); + } else { + // Within a DialogContainer there is no trigger state at this point in the tree. + // Let the event propagate so the overlay closes the dialog, and only fire onCancel. + onCancel(); + } + } + }; + return ( ; } const image = style({ diff --git a/packages/@react-spectrum/s2/test/AlertDialog.test.tsx b/packages/@react-spectrum/s2/test/AlertDialog.test.tsx index 06a8f2d3fb8..cdb7eaabcb8 100644 --- a/packages/@react-spectrum/s2/test/AlertDialog.test.tsx +++ b/packages/@react-spectrum/s2/test/AlertDialog.test.tsx @@ -82,4 +82,91 @@ describe('AlertDialog', () => { let content = document.getElementById(description!); expect(content).toHaveTextContent('Test content'); }); + + it('fires onCancel once and closes the dialog when the Escape key is pressed', async () => { + let onCancelSpy = jest.fn(); + let {getByRole} = render( + + Open dialog + + Test content + + + ); + + let trigger = getByRole('button'); + await user.click(trigger); + act(() => { + jest.runAllTimers(); + }); + let dialog = getByRole('alertdialog'); + expect(dialog).toBeVisible(); + + await user.keyboard('{Escape}'); + act(() => { + jest.runAllTimers(); + }); + expect(dialog).not.toBeInTheDocument(); + expect(onCancelSpy).toHaveBeenCalledTimes(1); + }); + + it('closes the dialog on Escape when onCancel is not provided', async () => { + let {getByRole} = render( + + Open dialog + + Test content + + + ); + + let trigger = getByRole('button'); + await user.click(trigger); + act(() => { + jest.runAllTimers(); + }); + let dialog = getByRole('alertdialog'); + expect(dialog).toBeVisible(); + + await user.keyboard('{Escape}'); + act(() => { + jest.runAllTimers(); + }); + expect(dialog).not.toBeInTheDocument(); + }); + + it('fires onCancel once when the cancel button is pressed', async () => { + let onCancelSpy = jest.fn(); + let {getByRole, getByText} = render( + + Open dialog + + Test content + + + ); + + let trigger = getByRole('button'); + await user.click(trigger); + act(() => { + jest.runAllTimers(); + }); + let dialog = getByRole('alertdialog'); + expect(dialog).toBeVisible(); + + await user.click(getByText('Cancel')); + act(() => { + jest.runAllTimers(); + }); + expect(dialog).not.toBeInTheDocument(); + expect(onCancelSpy).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/react-aria-components/test/Dialog.test.js b/packages/react-aria-components/test/Dialog.test.js index 1ea070282cf..31442a0137b 100644 --- a/packages/react-aria-components/test/Dialog.test.js +++ b/packages/react-aria-components/test/Dialog.test.js @@ -51,6 +51,21 @@ describe('Dialog', () => { expect(dialog).toHaveAttribute('data-rac'); }); + it('should support onKeyDown on the dialog element', async () => { + let onKeyDown = jest.fn(); + let {getByRole} = render( + + Title + + ); + + let dialog = getByRole('dialog'); + act(() => dialog.focus()); + await user.keyboard('{Enter}'); + expect(onKeyDown).toHaveBeenCalledTimes(1); + expect(onKeyDown).toHaveBeenCalledWith(expect.objectContaining({key: 'Enter'})); + }); + it('works with modal', async () => { let {getByRole} = render( diff --git a/packages/react-aria/src/dialog/useDialog.ts b/packages/react-aria/src/dialog/useDialog.ts index 1a7a021a9a5..932fd07d49e 100644 --- a/packages/react-aria/src/dialog/useDialog.ts +++ b/packages/react-aria/src/dialog/useDialog.ts @@ -20,7 +20,7 @@ import { import {filterDOMProps} from '../utils/filterDOMProps'; import {focusSafely} from '../interactions/focusSafely'; import {getActiveElement, isFocusWithin} from '../utils/shadowdom/DOMFunctions'; -import {useEffect, useRef} from 'react'; +import {KeyboardEventHandler, useEffect, useRef} from 'react'; import {useOverlayFocusContain} from '../overlays/Overlay'; import {useSlotId} from '../utils/useId'; @@ -31,6 +31,8 @@ export interface AriaDialogProps extends DOMProps, AriaLabelingProps { * @default 'dialog' */ role?: 'dialog' | 'alertdialog'; + /** Handler that is called when a key is pressed while focus is inside the dialog. */ + onKeyDown?: KeyboardEventHandler; } export interface DialogAria { @@ -125,6 +127,7 @@ export function useDialog( tabIndex: -1, 'aria-labelledby': props['aria-labelledby'] ?? titleId, 'aria-describedby': ariaDescribedby, + onKeyDown: props.onKeyDown, // Prevent blur events from reaching useOverlay, which may cause // popovers to close. Since focus is contained within the dialog, // we don't want this to occur due to the above useEffect.