diff --git a/.ai/rules/dialogs.md b/.ai/rules/dialogs.md index bf1bf20..59a5700 100644 --- a/.ai/rules/dialogs.md +++ b/.ai/rules/dialogs.md @@ -140,6 +140,32 @@ import { DialogButtons, DialogResult, useDialogContext } from '@cratis/arc.react | `DialogButtons.Ok` | Ok only | | `null` | No buttons (content-only dialog) | +A custom `buttons` ReactNode is not just a different footer — the dialog can no longer tell which of your buttons means confirm and which means dismiss, so it also **removes the close (X), stops `Escape` closing the dialog, and never calls `onConfirm` / `onCancel` / `onClose`** (including the confirm handler `CommandDialog` uses to execute its command). A custom footer must close the dialog itself via `useDialogContext().closeDialog(...)`. + +## Initial Focus on Destructive Dialogs + +`Dialog` (and `CommandDialog`, which forwards it) focuses the confirm button when the dialog opens. A focused native button 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 for free, because `isValid` / `isCommandFormValid` keeps confirm disabled until the form is complete. A dialog that needs **no** input is not — which is backwards when the action is irreversible. Say where focus should go instead: + +```tsx +import { DialogInitialFocus } from '@cratis/components/Dialogs'; + + + This permanently removes the person and every record about them. + +``` + +| `DialogInitialFocus` | Focuses | +|---|---| +| `Confirm` (default) | The `Ok` / `Yes` button | +| `Cancel` | The dismissing button — `Cancel`, or `No` when the set has no `Cancel` | +| `Content` | The dialog's own title, so nothing is armed | + +`Cancel` falls back to `Content` when there is no dismissing button. Use `initialFocus` rather than a custom footer for this — it changes focus and nothing else. + ## Customizing Built-in Buttons Use `okLabel`/`cancelLabel` to rename the buttons, and `isValid` to disable the confirm button: @@ -213,7 +239,8 @@ Use `buttons={null}` for dialogs that contain their own internal actions (e.g. a |---|---|---| | `title` | `string` | Header text (replaces PrimeReact `header`) | | `visible` | `boolean` | Controls visibility | -| `buttons` | `DialogButtons \| ReactNode \| null` | Prefer `DialogButtons` enum; `null` for no footer | +| `buttons` | `DialogButtons \| ReactNode \| null` | Prefer `DialogButtons` enum; `null` for no footer. Anything but a `DialogButtons` value also drops the close (X), `Escape`, and the confirm/cancel callbacks | +| `initialFocus` | `DialogInitialFocus` | Where focus lands on open — `Confirm` (default), `Cancel`, `Content` | | `isValid` | `boolean` | Disables the confirm button when `false` | | `okLabel` | `string` | Override the Ok/Confirm button label | | `cancelLabel` | `string` | Override the Cancel button label | diff --git a/Documentation/CommandDialog/index.md b/Documentation/CommandDialog/index.md index 5b7b70d..2eb0580 100644 --- a/Documentation/CommandDialog/index.md +++ b/Documentation/CommandDialog/index.md @@ -104,6 +104,7 @@ function MyComponent() { - `cancelLabel`: Custom text for cancel button (default: "Cancel") - `yesLabel`, `noLabel`: Labels for `YesNo` and `YesNoCancel` button modes - `buttons`: `DialogButtons` value or custom footer content +- `initialFocus`: Where keyboard focus lands when the dialog opens — forwarded to `Dialog` (see below) - `resizable`: Whether dialog can be resized - `isValid`: Additional validity gate combined with command form validity - `onFieldValidate`: Custom validation function for fields @@ -135,6 +136,33 @@ Multiple callbacks may fire for the same execution. For example, both `onFailed` - `onCancel` follows the same behavior as `Dialog` (`true` closes). - `onClose` closes unless it returns `false`. +## Destructive Commands and Initial Focus + +The confirm button is focused when the dialog opens, and a focused native button +fires `click` from the `keydown` of `Enter`. A command whose form has required +fields is protected from a held or double-tapped `Enter` for free, because the +form's validity keeps confirm disabled until something is filled in. A command +that takes **no** input — the typical "delete this, permanently" command — has +no such gate, so its confirm button is armed the instant the dialog appears. + +Pass `initialFocus` for those. It is forwarded straight to +[`Dialog`](../Dialogs/dialog.md#initial-focus) and changes nothing else — the +footer, the close (X), `Escape`, and the confirm wiring that runs the command +all stay intact. + +```tsx +import { DialogInitialFocus } from '@cratis/components/Dialogs'; + + + command={DeletePersonalData} + title="Delete personal data?" + okLabel="Delete" + initialFocus={DialogInitialFocus.Cancel} + onSuccess={() => closeDialog(DialogResult.Ok)}> + This cannot be undone. + +``` + ## Busy State `CommandDialog` automatically manages a busy state during command execution: diff --git a/Documentation/Dialogs/dialog.md b/Documentation/Dialogs/dialog.md index 8719c04..47f8142 100644 --- a/Documentation/Dialogs/dialog.md +++ b/Documentation/Dialogs/dialog.md @@ -66,15 +66,71 @@ const MyComponent = () => { - `onConfirm`: Callback for confirm actions - `onCancel`: Callback for cancel actions - `onClose`: Fallback close callback -- `buttons`: Predefined `DialogButtons` or custom footer content +- `buttons`: Predefined `DialogButtons` or custom footer content. A custom + footer also removes the close (X), stops `Escape` closing the dialog, and + leaves `onConfirm` / `onCancel` / `onClose` uncalled — the dialog cannot tell + which of your buttons means what, so a custom footer must close the dialog + itself through `useDialogContext().closeDialog(...)` - `width`: Dialog width - `style`: Custom dialog style forwarded to PrimeReact `Dialog` - `contentStyle`: Custom content area style forwarded to PrimeReact `Dialog` - `resizable`: Enables resize - `isValid`: Enables or disables confirm actions - `isBusy`: When `true`, disables all buttons and shows a loading spinner on the primary action button +- `initialFocus`: Where keyboard focus lands when the dialog opens (see below) - `okLabel`, `cancelLabel`, `yesLabel`, `noLabel`: Button labels +## Initial focus + +By default the confirm button is focused when a dialog opens, which makes the +common "read it, press Enter" flow cost one keystroke. That default also *arms* +the confirm button: browsers fire `click` from the `keydown` of `Enter`, so a +key still held down from the control that opened the dialog — or the ordinary +habit of pressing `Enter` twice — confirms it immediately. + +A dialog with input is protected from this for free, because `isValid` keeps +confirm disabled until the form is complete. A dialog that needs **no** input +is not, which is exactly backwards when the action is destructive. Say where +focus should go with `initialFocus`: + +| `DialogInitialFocus` | Focuses | +|---|---| +| `Confirm` (default) | The `Ok` / `Yes` button | +| `Cancel` | The dismissing button — `Cancel`, or `No` when the set has no `Cancel` | +| `Content` | The dialog's own title, so nothing is armed | + +```typescript +import { Dialog, DialogInitialFocus } from '@cratis/components/Dialogs'; +import { DialogButtons, DialogResult, useDialogContext } from '@cratis/arc.react/dialogs'; + +const DeletePersonalDataDialog = () => { + const { closeDialog } = useDialogContext(); + + return ( + closeDialog(DialogResult.Yes)} + onCancel={() => closeDialog(DialogResult.No)} + > + This permanently removes the person and every record about them. + + ); +}; +``` + +`Cancel` falls back to `Content` when the button set has nothing to dismiss +with (`DialogButtons.Ok`, a custom footer, or no footer). Focus never stays on +`document.body`: a modal that does not move focus into itself leaves keyboard +and screen-reader users stranded outside the content that just interrupted +them. + +`initialFocus` is forwarded by `CommandDialog`, and it changes **only** focus — +the footer, the close (X), `Escape`, and every callback keep working. That is +the difference from the older workaround of replacing `buttons` with a custom +node, which silently gives all of those up. + ## Notes - Prefer `onConfirm` and `onCancel` over `onClose` for clear intent. diff --git a/Source/CommandDialog/CommandDialog.stories.tsx b/Source/CommandDialog/CommandDialog.stories.tsx index 6300e8d..166cb3b 100644 --- a/Source/CommandDialog/CommandDialog.stories.tsx +++ b/Source/CommandDialog/CommandDialog.stories.tsx @@ -8,6 +8,7 @@ import { Command, CommandResult, CommandValidator } from '@cratis/arc/commands'; import { PropertyDescriptor } from '@cratis/arc/reflection'; import { InputTextField, NumberField, TextAreaField } from '../CommandForm/fields'; import { DialogResult, useDialog, useDialogContext } from '@cratis/arc.react/dialogs'; +import { DialogInitialFocus } from '../Dialogs/DialogInitialFocus'; import '@cratis/arc/validation'; const meta: Meta = { @@ -849,3 +850,97 @@ export const WithResponseTypeAndCallbacks: Story = { ); }, }; + +/** + * A destructive command that needs **no** input. Every other story here is + * protected from a held or double-tapped `Enter` for free, because + * `isCommandFormValid` keeps the confirm button disabled until its fields are + * filled in — but a command with no fields is valid the moment it appears, so + * its confirm button is armed on mount. + * + * `initialFocus` moves the keyboard off it without giving up the footer, the + * close (X), `Escape`, or the confirm wiring that runs the command. + */ +export const DestructiveCommandFocusesDismiss: Story = { + render: () => { + const [result, setResult] = useState(''); + + class NothingToValidate extends CommandValidator { + } + + class DeletePersonalDataCommand extends Command { + readonly route: string = '/api/people/delete'; + readonly validation: CommandValidator = new NothingToValidate(); + readonly propertyDescriptors: PropertyDescriptor[] = [ + new PropertyDescriptor('personId', String), + ]; + + personId = ''; + + constructor() { + super(Object, false); + } + + get requestParameters(): string[] { + return []; + } + + get properties(): string[] { + return ['personId']; + } + + override async validate(): Promise> { + return CommandResult.empty; + } + + override async execute(): Promise> { + await new Promise(resolve => setTimeout(resolve, 300)); + return CommandResult.empty; + } + } + + const DeletePersonalDataDialog = () => { + const { closeDialog } = useDialogContext>(); + + return ( + + command={DeletePersonalDataCommand} + title="Delete personal data?" + okLabel="Delete" + cancelLabel="Keep" + autoServerValidate={false} + initialFocus={DialogInitialFocus.Cancel} + initialValues={{ personId: '8f1b9c1e-0000-4000-8000-000000000000' }} + onSuccess={() => closeDialog(DialogResult.Ok)} + onCancel={() => closeDialog(DialogResult.Cancelled)} + > +

This permanently removes the person and every record about them. It cannot be undone.

+ + ); + }; + + const [DeletePersonalDataDialogComponent, showDeletePersonalDataDialog] = useDialog>(DeletePersonalDataDialog); + + return ( +
+ + + {result && ( +
+ Outcome: {result} +
+ )} + + +
+ ); + }, +}; diff --git a/Source/CommandDialog/CommandDialog.tsx b/Source/CommandDialog/CommandDialog.tsx index 3fe4d38..59e730e 100644 --- a/Source/CommandDialog/CommandDialog.tsx +++ b/Source/CommandDialog/CommandDialog.tsx @@ -57,6 +57,7 @@ const CommandDialogWrapper = ({ contentStyle, resizable, buttons, + initialFocus, okLabel, cancelLabel, yesLabel, @@ -82,6 +83,7 @@ const CommandDialogWrapper = ({ contentStyle?: DialogProps['contentStyle']; resizable?: boolean; buttons?: DialogProps['buttons']; + initialFocus?: DialogProps['initialFocus']; okLabel?: string; cancelLabel?: string; yesLabel?: string; @@ -176,6 +178,7 @@ const CommandDialogWrapper = ({ contentStyle={contentStyle} resizable={resizable} buttons={buttons} + initialFocus={initialFocus} onClose={onClose} onConfirm={handleConfirm} onCancel={onCancel} @@ -231,6 +234,32 @@ const CommandDialogWrapper = ({ * Throughout, the dialog is in the `isBusy` state — every action button is * disabled and the confirm button shows a spinner. * + * ## Destructive commands and initial focus + * + * The confirm button is focused when the dialog opens, and a focused native + * button fires `click` from the `keydown` of `Enter`. A command whose form + * has required fields is protected from a held or double-tapped `Enter` for + * free, because `isCommandFormValid` keeps confirm disabled until something + * is filled in. A command that takes **no** input — the typical "delete + * this, permanently" command — has no such gate, so its confirm button is + * armed the instant the dialog appears. + * + * Pass `initialFocus` (forwarded straight to {@link Dialog}) for those: + * + * ```tsx + * + * command={DeletePerson} + * title="Delete personal data?" + * okLabel="Delete" + * initialFocus={DialogInitialFocus.Cancel} + * onSuccess={() => closeDialog(DialogResult.Ok)}> + * This cannot be undone. + * + * ``` + * + * Everything else — the footer, the close (X), `Escape`, and the confirm + * wiring that runs the command — is untouched. + * * ## Field binding * * Children that are `CommandFormField` instances (`InputTextField`, @@ -296,6 +325,7 @@ const CommandDialogComponent = ({ + executeCommand: vi.fn(async () => ({ isSuccess: true, isValid: true, validationResults: [] })), + succeeded: vi.fn() +})); + +vi.mock('@cratis/arc.react/commands', () => ({ + CommandForm: (props: { children?: React.ReactNode }) => + React.createElement('div', null, props.children), + useCommandFormContext: () => ({ + isValid: true, + setCommandValues: () => { /* not part of this scenario */ }, + setCommandResult: () => { /* not part of this scenario */ } + }), + useCommandInstance: () => ({ execute: executeCommand }), + CommandFormFieldWrapper: (props: { field?: React.ReactNode }) => + React.createElement('div', null, props.field) +})); + +class DeletePersonalData { + personId: string = ''; +} + +describe('when a command dialog is given an initial focus', () => { + let dialog: DialogInTheDom; + + const renderDialog = async (initialFocus?: DialogInitialFocus) => { + executeCommand.mockClear(); + succeeded.mockClear(); + + const { CommandDialog } = await import('../CommandDialog'); + + dialog = await render(React.createElement(CommandDialog, { + command: DeletePersonalData as unknown as new () => object, + title: 'Delete personal data', + visible: true, + initialFocus, + onSuccess: succeeded, + children: React.createElement('p', null, 'This cannot be undone') + })); + }; + + afterEach(async () => await unmount(dialog)); + + it('should focus the Ok button when nothing is specified', async () => { + await renderDialog(); + + focusedElement().should.equal('button:Ok'); + }); + + it('should forward the choice to the dialog it wraps', async () => { + await renderDialog(DialogInitialFocus.Cancel); + + focusedElement().should.equal('button:Cancel'); + }); + + it('should forward a request to focus the content', async () => { + await renderDialog(DialogInitialFocus.Content); + + focusedElement().should.equal('span:Delete personal data'); + }); + + it('should not run the command on an Enter that repeats onto the freshly mounted dialog', async () => { + await renderDialog(DialogInitialFocus.Cancel); + + await pressEnterOnFocusedElement(); + + executeCommand.should.not.have.been.called; + }); + + it('should still run the command when the user deliberately confirms', async () => { + await renderDialog(DialogInitialFocus.Cancel); + + await click('Ok'); + + executeCommand.should.have.been.calledOnce; + succeeded.should.have.been.calledOnce; + }); +}); diff --git a/Source/Dialogs/BusyIndicatorDialog.tsx b/Source/Dialogs/BusyIndicatorDialog.tsx index d6b052e..4e13e0b 100644 --- a/Source/Dialogs/BusyIndicatorDialog.tsx +++ b/Source/Dialogs/BusyIndicatorDialog.tsx @@ -4,6 +4,7 @@ import { BusyIndicatorDialogRequest } from '@cratis/arc.react/dialogs'; import { ProgressSpinner } from 'primereact/progressspinner'; import { Dialog } from './Dialog'; +import { DialogInitialFocus } from './DialogInitialFocus'; /** * Modal "busy" dialog used by the `@cratis/arc.react` dialog host whenever a @@ -43,7 +44,10 @@ import { Dialog } from './Dialog'; * * - **No interactive buttons.** A busy indicator is a wait-state, not a * confirmation prompt. The dialog has no Ok / Cancel / X — only the host - * can dismiss it. + * can dismiss it. Because there is nothing focusable inside it, initial + * focus is put on the dialog's own title, so a keyboard or screen-reader + * user is told what is happening instead of being left on `document.body` + * behind the modal mask. * - **No per-instance pass-through.** The request type * ({@link BusyIndicatorDialogRequest}) is owned by `@cratis/arc.react`, so * `pt` / `unstyled` are not exposed on a per-call basis. Restyle the @@ -59,6 +63,7 @@ export const BusyIndicatorDialog = (props: BusyIndicatorDialogRequest) => { visible={true} onCancel={() => undefined} buttons={null} + initialFocus={DialogInitialFocus.Content} >
diff --git a/Source/Dialogs/Dialog.stories.tsx b/Source/Dialogs/Dialog.stories.tsx index 702b3e1..419a8a6 100644 --- a/Source/Dialogs/Dialog.stories.tsx +++ b/Source/Dialogs/Dialog.stories.tsx @@ -4,6 +4,7 @@ import React, { useState } from 'react'; import { Meta, StoryObj } from '@storybook/react'; import { Dialog } from './Dialog'; +import { DialogInitialFocus } from './DialogInitialFocus'; import { DialogButtons, DialogResult, useDialog, useDialogContext } from '@cratis/arc.react/dialogs'; import { Button } from 'primereact/button'; import { InputText } from 'primereact/inputtext'; @@ -19,7 +20,7 @@ const meta: Meta = { export default meta; type Story = StoryObj; -const DialogWrapper = ({ buttons, title, children, isValid }: { buttons: DialogButtons; title: string; children: React.ReactNode; isValid?: boolean }) => { +const DialogWrapper = ({ buttons, title, children, isValid, initialFocus }: { buttons: DialogButtons; title: string; children: React.ReactNode; isValid?: boolean; initialFocus?: DialogInitialFocus }) => { const ResultDialog = () => { const { closeDialog } = useDialogContext(); @@ -27,6 +28,7 @@ const DialogWrapper = ({ buttons, title, children, isValid }: { buttons: DialogB closeDialog(DialogResult.Ok)} onCancel={() => closeDialog(DialogResult.Cancelled)} isValid={isValid} @@ -78,6 +80,33 @@ export const Ok: Story = { ) }; +/** + * A destructive dialog that needs no input. Initial focus is put on the + * dismissing button, so the `Enter` still held down from the row that opened + * the dialog — or a reflexive second press — cannot confirm it. Hold `Enter` + * on the trigger button to see the difference against the stories above. + */ +export const DestructiveFocusesDismiss: Story = { + render: () => ( + +

This permanently removes the person and every record about them. It cannot be undone.

+ + ) +}; + +/** + * Nothing is armed at all: focus goes to the dialog's own title, so screen + * readers announce the dialog from the top and the first `Tab` walks the + * content. Use it when the dialog should be read before it is answered. + */ +export const DestructiveArmsNothing: Story = { + render: () => ( + +

This permanently removes the person and every record about them. It cannot be undone.

+ + ) +}; + export const WithForm: Story = { render: () => { type NameResult = { name: string }; 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 = ( <> -