From 3756a1ede2054aee2179e58ac7bbe9d72b85e045 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 1 Aug 2022 12:24:33 -0400 Subject: [PATCH] Default ConfirmationDialog to focusing `cancel` when the confirmation is a dangerous one (#2185) * allow configuration of the initial focus in a confirmation dialog * changeset * explicitly set the autoFocus when danger to cancel * intermediate value Co-authored-by: Siddharth Kshetrapal --- .changeset/lucky-pianos-yell.md | 5 +++++ src/Dialog/ConfirmationDialog.tsx | 6 ++++-- src/__tests__/ConfirmationDialog.test.tsx | 25 +++++++++++++++++++++-- 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 .changeset/lucky-pianos-yell.md diff --git a/.changeset/lucky-pianos-yell.md b/.changeset/lucky-pianos-yell.md new file mode 100644 index 00000000000..c4d16e77efb --- /dev/null +++ b/.changeset/lucky-pianos-yell.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Set ConfirmationDialog initial focus based on the confirmationButtonVariant. When `danger` autoFocus the cancel button, otherwise autoFocus the confirmation button diff --git a/src/Dialog/ConfirmationDialog.tsx b/src/Dialog/ConfirmationDialog.tsx index 46bbda3bae5..3e8b4689b50 100644 --- a/src/Dialog/ConfirmationDialog.tsx +++ b/src/Dialog/ConfirmationDialog.tsx @@ -116,15 +116,17 @@ export const ConfirmationDialog: React.FC { onClose('confirm') }, [onClose]) + const isConfirmationDangerous = confirmButtonType === 'danger' const cancelButton: DialogButtonProps = { content: cancelButtonContent, - onClick: onCancelButtonClick + onClick: onCancelButtonClick, + autoFocus: isConfirmationDangerous } const confirmButton: DialogButtonProps = { content: confirmButtonContent, buttonType: confirmButtonType, onClick: onConfirmButtonClick, - autoFocus: true + autoFocus: !isConfirmationDangerous } const footerButtons = [cancelButton, confirmButton] return ( diff --git a/src/__tests__/ConfirmationDialog.test.tsx b/src/__tests__/ConfirmationDialog.test.tsx index 7de379edf25..7b62f0b8290 100644 --- a/src/__tests__/ConfirmationDialog.test.tsx +++ b/src/__tests__/ConfirmationDialog.test.tsx @@ -15,7 +15,7 @@ import {behavesAsComponent, checkExports} from '../utils/testing' expect.extend(toHaveNoViolations) -const Basic = () => { +const Basic = ({confirmButtonType}: Pick, 'confirmButtonType'>) => { const [isOpen, setIsOpen] = useState(false) const buttonRef = useRef(null) const onDialogClose = useCallback(() => setIsOpen(false), []) @@ -32,6 +32,7 @@ const Basic = () => { onClose={onDialogClose} cancelButtonContent="Secondary" confirmButtonContent="Primary" + confirmButtonType={confirmButtonType} > Lorem ipsum dolor sit Pippin good dog. @@ -95,7 +96,7 @@ describe('ConfirmationDialog', () => { cleanup() }) - it('focuses the primary action when opened', async () => { + it('focuses the primary action when opened and the confirmButtonType is not set', async () => { const {getByText} = HTMLRender() act(() => { fireEvent.click(getByText('Show dialog')) @@ -105,6 +106,26 @@ describe('ConfirmationDialog', () => { cleanup() }) + it('focuses the primary action when opened and the confirmButtonType is not danger', async () => { + const {getByText} = HTMLRender() + act(() => { + fireEvent.click(getByText('Show dialog')) + }) + expect(getByText('Primary')).toEqual(document.activeElement) + expect(getByText('Secondary')).not.toEqual(document.activeElement) + cleanup() + }) + + it('focuses the secondary action when opened and the confirmButtonType is danger', async () => { + const {getByText} = HTMLRender() + act(() => { + fireEvent.click(getByText('Show dialog')) + }) + expect(getByText('Primary')).not.toEqual(document.activeElement) + expect(getByText('Secondary')).toEqual(document.activeElement) + cleanup() + }) + it('supports nested `focusTrap`s', async () => { const {getByText} = HTMLRender() act(() => {