From 311cadf295afc9821728af90c7432d5d63f27fce Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Fri, 12 Sep 2025 16:03:41 +0000 Subject: [PATCH 1/6] Allow changing initial focus button in confirmation dialog --- .../ConfirmationDialog.test.tsx | 15 ++++++++++++++- .../src/ConfirmationDialog/ConfirmationDialog.tsx | 14 ++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx index f9ae828b448..7b5edd8a05c 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx @@ -11,7 +11,10 @@ import theme from '../theme' import {ThemeProvider} from '../ThemeProvider' import {Stack} from '../Stack' -const Basic = ({confirmButtonType}: Pick, 'confirmButtonType'>) => { +const Basic = ({ + confirmButtonType, + initialFocusButton, +}: Pick, 'confirmButtonType' | 'initialFocusButton'>) => { const [isOpen, setIsOpen] = useState(false) const buttonRef = useRef(null) const onDialogClose = useCallback(() => setIsOpen(false), []) @@ -28,6 +31,7 @@ const Basic = ({confirmButtonType}: Pick Lorem ipsum dolor sit Pippin good dog. @@ -187,6 +191,15 @@ describe('ConfirmationDialog', () => { expect(dialog.getAttribute('data-height')).toBe('small') }) + it('focuses the confirm button even when dangerous if initialButtonFocus is confirm', async () => { + const {getByText, getByRole} = render() + + fireEvent.click(getByText('Show dialog')) + + expect(getByRole('button', {name: 'Primary'})).toEqual(document.activeElement) + expect(getByRole('button', {name: 'Secondary'})).not.toEqual(document.activeElement) + }) + describe('loading states', () => { it('applies loading state to confirm button when confirmButtonLoading is true', async () => { const {getByText, getByRole} = render() diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx index 6223dfebfbf..b45e43ed218 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx @@ -51,6 +51,13 @@ export interface ConfirmationDialogProps { */ confirmButtonLoading?: boolean + /** + * The button that should be initially focused when the dialog is opened. By default, the confirm button + * is focused initially unless it is a dangerous action, in which case the cancel button is focused. This should + * rarely be overridden, in order to ensure that the user does not accidentally confirm a dangerous action. + */ + initialFocusButton?: 'cancel' | 'confirm' + /** * Additional class names to apply to the dialog */ @@ -125,6 +132,7 @@ export const ConfirmationDialog: React.FC { @@ -134,17 +142,19 @@ export const ConfirmationDialog: React.FC Date: Fri, 12 Sep 2025 16:05:36 +0000 Subject: [PATCH 2/6] Add changeset --- .changeset/wet-terms-argue.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/wet-terms-argue.md diff --git a/.changeset/wet-terms-argue.md b/.changeset/wet-terms-argue.md new file mode 100644 index 00000000000..bf655004a8c --- /dev/null +++ b/.changeset/wet-terms-argue.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Allow changing initially focused button in ConfirmationDialog From 1d0d31ddc0733f6e9f0064695e9dfe57ea58aa3f Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Fri, 12 Sep 2025 16:21:15 +0000 Subject: [PATCH 3/6] Update docs --- .../src/ConfirmationDialog/ConfirmationDialog.docs.json | 5 +++++ .../react/src/ConfirmationDialog/useConfirm.hookDocs.json | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json b/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json index 60c3891d184..1ef374f41f4 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json @@ -37,6 +37,11 @@ "defaultValue": "normal", "description": "The type of button to use for the confirm button." }, + { + "name": "initialButtonFocus", + "type": "'confirm' | 'cancel'", + "description": "The button that should be initially focused when the dialog is opened. By default, the initial button focus is the confirm button, unless the confirm button is dangerous, in which case the cancel button is focused. This prop should be used rarely, as it can allow dangerous actions to be taken accidentally." + }, { "name": "className", "type": "string", diff --git a/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json b/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json index b9220ed0afa..be96598b3e2 100644 --- a/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json +++ b/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json @@ -46,6 +46,11 @@ "type": "\"normal\" | \"primary\" | \"danger\"", "defaultValue": "normal", "description": "The type of button to use for the confirm button." + }, + { + "name": "initialButtonFocus", + "type": "'confirm' | 'cancel'", + "description": "The button that should be initially focused when the dialog is opened. By default, the initial button focus is the confirm button, unless the confirm button is dangerous, in which case the cancel button is focused. This prop should be used rarely, as it can allow dangerous actions to be taken accidentally." } ] } From a25bb78177e0f4f8abdd374060d8c9ff02ae84a3 Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Mon, 15 Sep 2025 09:45:43 -0400 Subject: [PATCH 4/6] Rename initialButtonFocus to initialFocusButton --- .../react/src/ConfirmationDialog/ConfirmationDialog.docs.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json b/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json index 1ef374f41f4..94984c0c30d 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json @@ -38,7 +38,7 @@ "description": "The type of button to use for the confirm button." }, { - "name": "initialButtonFocus", + "name": "initialFocusButton", "type": "'confirm' | 'cancel'", "description": "The button that should be initially focused when the dialog is opened. By default, the initial button focus is the confirm button, unless the confirm button is dangerous, in which case the cancel button is focused. This prop should be used rarely, as it can allow dangerous actions to be taken accidentally." }, From e954c6c6e1a62569a5cf78b169af4ef71a90bd41 Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Mon, 15 Sep 2025 09:46:03 -0400 Subject: [PATCH 5/6] Rename initialButtonFocus to initialFocusButton --- packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json b/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json index be96598b3e2..217794fe47e 100644 --- a/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json +++ b/packages/react/src/ConfirmationDialog/useConfirm.hookDocs.json @@ -48,7 +48,7 @@ "description": "The type of button to use for the confirm button." }, { - "name": "initialButtonFocus", + "name": "initialFocusButton", "type": "'confirm' | 'cancel'", "description": "The button that should be initially focused when the dialog is opened. By default, the initial button focus is the confirm button, unless the confirm button is dangerous, in which case the cancel button is focused. This prop should be used rarely, as it can allow dangerous actions to be taken accidentally." } From f33ebba7ae45c6b6fb655eb1950181e992e4d427 Mon Sep 17 00:00:00 2001 From: Cam McHenry Date: Mon, 15 Sep 2025 14:42:52 +0000 Subject: [PATCH 6/6] Rename initialFocusButton prop to overrideButtonFocus in ConfirmationDialog and update related documentation --- .../src/ConfirmationDialog/ConfirmationDialog.docs.json | 2 +- .../src/ConfirmationDialog/ConfirmationDialog.test.tsx | 8 ++++---- .../react/src/ConfirmationDialog/ConfirmationDialog.tsx | 8 ++++---- .../react/src/ConfirmationDialog/useConfirm.hookDocs.json | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json b/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json index 1ef374f41f4..4c7dbec7c6d 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.docs.json @@ -38,7 +38,7 @@ "description": "The type of button to use for the confirm button." }, { - "name": "initialButtonFocus", + "name": "overrideButtonFocus", "type": "'confirm' | 'cancel'", "description": "The button that should be initially focused when the dialog is opened. By default, the initial button focus is the confirm button, unless the confirm button is dangerous, in which case the cancel button is focused. This prop should be used rarely, as it can allow dangerous actions to be taken accidentally." }, diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx index 7b5edd8a05c..888bf09406a 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.test.tsx @@ -13,8 +13,8 @@ import {Stack} from '../Stack' const Basic = ({ confirmButtonType, - initialFocusButton, -}: Pick, 'confirmButtonType' | 'initialFocusButton'>) => { + overrideButtonFocus, +}: Pick, 'confirmButtonType' | 'overrideButtonFocus'>) => { const [isOpen, setIsOpen] = useState(false) const buttonRef = useRef(null) const onDialogClose = useCallback(() => setIsOpen(false), []) @@ -31,7 +31,7 @@ const Basic = ({ cancelButtonContent="Secondary" confirmButtonContent="Primary" confirmButtonType={confirmButtonType} - initialFocusButton={initialFocusButton} + overrideButtonFocus={overrideButtonFocus} > Lorem ipsum dolor sit Pippin good dog. @@ -192,7 +192,7 @@ describe('ConfirmationDialog', () => { }) it('focuses the confirm button even when dangerous if initialButtonFocus is confirm', async () => { - const {getByText, getByRole} = render() + const {getByText, getByRole} = render() fireEvent.click(getByText('Show dialog')) diff --git a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx index b45e43ed218..19152d10254 100644 --- a/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx +++ b/packages/react/src/ConfirmationDialog/ConfirmationDialog.tsx @@ -52,11 +52,11 @@ export interface ConfirmationDialogProps { confirmButtonLoading?: boolean /** - * The button that should be initially focused when the dialog is opened. By default, the confirm button + * Overrides the button that should be initially focused when the dialog is opened. By default, the confirm button * is focused initially unless it is a dangerous action, in which case the cancel button is focused. This should * rarely be overridden, in order to ensure that the user does not accidentally confirm a dangerous action. */ - initialFocusButton?: 'cancel' | 'confirm' + overrideButtonFocus?: 'cancel' | 'confirm' /** * Additional class names to apply to the dialog @@ -132,7 +132,7 @@ export const ConfirmationDialog: React.FC { @@ -143,7 +143,7 @@ export const ConfirmationDialog: React.FC