From 0769cded92038d25cd7edd2fc2f1ccdfc08af31e Mon Sep 17 00:00:00 2001 From: woksin Date: Fri, 7 Aug 2026 13:58:38 +0200 Subject: [PATCH 1/6] Add jsdom so specs can cover dialog focus behavior Initial focus only exists in a DOM: which element ends up under the user's keyboard cannot be observed from server-rendered markup. The specs that cover it therefore render into jsdom with the real PrimeReact components. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XRf73wBo2QQQ2FRoVLqgJu --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 0d17475..b5c36c5 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "eslint-plugin-react": "^7.37.5", "glob": "^13.0.6", "globals": "^17.7.0", + "jsdom": "^30.0.1", "mocha": "^11.7.6", "module-alias": "^2.3.4", "npm-check-updates": "^22.2.9", From 9f47b08501c78c28bb52217f9e92cd2a6e65e2d1 Mon Sep 17 00:00:00 2001 From: woksin Date: Fri, 7 Aug 2026 13:58:50 +0200 Subject: [PATCH 2/6] Add an initialFocus prop for choosing where dialog focus lands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dialog focused its confirm button on open with no way to say otherwise. That also arms it: a browser fires click from the keydown of Enter, so a key still held from the control that opened the dialog, or the ordinary habit of pressing Enter twice, confirms it immediately. A dialog that collects input is protected by isValid; a dialog that needs no input is not, which is backwards when the action is irreversible. The only escape was replacing the footer with a custom ReactNode, which also removes the close (X), stops Escape closing the dialog and leaves onConfirm/onCancel/onClose uncalled — so it was never a workaround. initialFocus changes focus and nothing else. Confirm stays the default so no existing dialog changes; Cancel focuses the dismissing button; Content focuses the dialog's own title so nothing is armed. Cancel degrades to Content where there is no dismissing button, because a modal that leaves focus on document.body strands keyboard and screen-reader users outside the content that just interrupted them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XRf73wBo2QQQ2FRoVLqgJu --- Source/Dialogs/Dialog.tsx | 102 ++++++++++++++++++++++++--- Source/Dialogs/DialogInitialFocus.ts | 49 +++++++++++++ Source/Dialogs/index.ts | 1 + 3 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 Source/Dialogs/DialogInitialFocus.ts diff --git a/Source/Dialogs/Dialog.tsx b/Source/Dialogs/Dialog.tsx index a021d1c..093df66 100644 --- a/Source/Dialogs/Dialog.tsx +++ b/Source/Dialogs/Dialog.tsx @@ -4,7 +4,8 @@ import { Dialog as PrimeDialog, type DialogProps as PrimeDialogProps } from 'primereact/dialog'; import { Button } from 'primereact/button'; import { DialogResult, DialogButtons, useDialogContext } from '@cratis/arc.react/dialogs'; -import { ReactNode } from 'react'; +import { ReactNode, useRef } from 'react'; +import { DialogInitialFocus } from './DialogInitialFocus'; /** * Callback used by {@link Dialog} (and its wrappers) when the dialog is about to @@ -54,9 +55,34 @@ export interface DialogProps { * the predefined sets (`Ok`, `OkCancel`, `YesNo`, `YesNoCancel`), `null` for * no footer, or a custom React node to fully render your own footer. * Defaults to `DialogButtons.OkCancel`. + * + * ⚠️ **Anything other than a {@link DialogButtons} value also opts the dialog + * out of three unrelated behaviors**, because the dialog can no longer know + * which of your buttons means "confirm" and which means "dismiss": + * the header close (X) is removed, `Escape` no longer closes, and + * `onClose` / `onCancel` / `onConfirm` are never invoked — including the + * confirm handler that {@link CommandDialog} uses to execute its command. + * A custom footer must therefore close the dialog itself through + * `useDialogContext().closeDialog(...)`. + * + * Reach for {@link initialFocus} rather than a custom footer when all you + * want is to change which button starts out focused. */ buttons?: DialogButtons | ReactNode; + /** + * Where keyboard focus lands when the dialog becomes visible. Defaults to + * {@link DialogInitialFocus.Confirm}, which focuses the `Ok` / `Yes` button. + * + * Set it to {@link DialogInitialFocus.Cancel} or + * {@link DialogInitialFocus.Content} for a dialog whose confirm action is + * destructive and needs no input to become valid — otherwise the confirm + * button sits armed under the same `Enter` that opened the dialog, and key + * auto-repeat (or the habit of pressing `Enter` twice) collapses a two-step + * confirmation into one. + */ + initialFocus?: DialogInitialFocus; + /** Dialog body content. */ children: ReactNode; @@ -144,6 +170,36 @@ export interface DialogProps { * - **Validity gate** (`isValid`) that disables the confirm button without * disabling the cancel button. Used by command-executing wrappers to * block submission while form validation fails. + * - **Initial focus** (`initialFocus`) choosing which element inside the + * dialog the keyboard lands on when it opens. + * + * ## Initial focus + * + * By default the confirm button is focused when the dialog opens, so the + * common "read it, press Enter" flow costs one keystroke. That default also + * *arms* the confirm button: browsers fire `click` from the `keydown` of + * `Enter`, so the key still held down from opening the dialog — or the very + * ordinary habit of pressing `Enter` twice — confirms it immediately. + * + * A dialog whose confirm action is destructive **and** needs no input to + * become valid gets no protection from `isValid`, because there is nothing + * to fill in. Give those dialogs an explicit + * {@link DialogInitialFocus} instead: + * + * ```tsx + * + * This permanently removes the person and every record about them. + * + * ``` + * + * `Cancel` focuses the dismissing button (`Cancel`, or `No` when the set has + * no `Cancel`); `Content` focuses the dialog's title so nothing at all is + * armed and screen readers announce the dialog from the top. Both keep the + * footer, the close (X), `Escape`, and every callback intact — unlike + * replacing `buttons` with a custom node. * * ## Arc dialog host integration * @@ -203,6 +259,7 @@ export const Dialog = ({ onConfirm, onCancel, buttons = DialogButtons.OkCancel, + initialFocus = DialogInitialFocus.Confirm, children, width = '450px', style, @@ -231,9 +288,35 @@ export const Dialog = ({ } const isDialogValid = isValid !== false; + + // A dismissing button only exists for the predefined sets that have one — Cancel for + // OkCancel / YesNoCancel, No for YesNo. Asking for Cancel focus without one (DialogButtons.Ok, + // a custom footer node, or no footer) degrades to focusing the title rather than silently + // leaving focus on document.body outside the modal. + const hasDismissingButton = typeof buttons === 'number' && buttons !== DialogButtons.Ok; + const resolvedInitialFocus = (initialFocus === DialogInitialFocus.Cancel && !hasDismissingButton) + ? DialogInitialFocus.Content + : initialFocus; + + const focusesConfirmButton = resolvedInitialFocus === DialogInitialFocus.Confirm; + const focusesDismissingButton = resolvedInitialFocus === DialogInitialFocus.Cancel; + const focusesTitle = resolvedInitialFocus === DialogInitialFocus.Content; + + // PrimeReact's focus trap parks focus on the first focusable element inside the dialog while + // it transitions in, and its own onShow focus only fires when nothing in the dialog has focus. + // Moving focus to the title from onShow therefore runs last and wins, without having to fight + // the trap or turn focusOnShow off (which would leave focus outside the modal for the duration + // of the transition). + const titleRef = useRef(null); + const handleShow = () => { + if (focusesTitle) { + titleRef.current?.focus({ preventScroll: true }); + } + }; + const headerElement = (
- {title} + {title}
); @@ -265,29 +348,29 @@ export const Dialog = ({ const okFooter = ( <> -