Gallery has no focus management in modals — when the delete confirmation dialog opens, focus doesn't move to the dialog, and on close focus doesn't return to the card that triggered it #241

New issue

Closed

opened 2026-10-04 02:35:16 +00:00 by Mark · 1 comment

Mark commented

2026-10-04 02:35:16 +00:00

Owner

Finding

src/components/gallery/DeleteDialog.tsx (per the snippet I saw) is a shadcn Dialog. The shadcn Dialog component (line 4) should handle focus trapping and initial focus. Let me verify.

grep -n 'autoFocus\|onOpenAutoFocus' src/components/gallery/DeleteDialog.tsx returns 0 hits. The shadcn Dialog should auto-focus the first focusable element, but the actual implementation may not.

grep -n 'onCloseAutoFocus' src/components/gallery/DeleteDialog.tsx returns 0 hits. On close, the focus return behavior depends on the shadcn implementation.

A screen reader user opens the Gallery:

Tabs to a design card's delete button
Activates it
The dialog opens
Focus is... wherever it was? The Delete button? Off-screen?
Presses Tab
Tabs through invisible elements
The Cancel button is eventually focused

This is a common accessibility bug with custom modals. Standard behavior:

On open: focus moves to the first focusable element in the dialog (or the dialog itself)
While open: focus is trapped inside the dialog (Tab cycles through dialog elements)
On close: focus returns to the element that opened the dialog

Where

DeleteDialog: src/components/gallery/DeleteDialog.tsx
Other dialogs: NewFolderDialog, RightClickMenu, ExportDialog (workspace)
Shadcn Dialog: src/components/ui/dialog.tsx (the underlying implementation)

Suggested fix shape

Verify the shadcn Dialog's onOpenAutoFocus and onCloseAutoFocus handlers are set correctly. If they're missing:

In DeleteDialog.tsx, set the focus on the Cancel button when the dialog opens:

<DialogContent
  onOpenAutoFocus={(e) => {
    e.preventDefault();
    cancelButtonRef.current?.focus();
  }}
>
  ...
  <Button ref={cancelButtonRef} onClick={...}>Cancel</Button>
</DialogContent>

shadcn's Dialog (per the Radix UI docs) automatically traps focus inside the dialog. Verify by Tab-ing through the dialog and checking focus doesn't escape.
On close, Radix restores focus to the element that opened the dialog. Verify by Tab-ing and checking focus returns to the Delete button.

The same fix applies to:

NewFolderDialog.tsx (folder create)
RenameDialog.tsx (design rename, if exists)
SettingsModal.tsx (settings)
ExportDialog.tsx (export)
RightClickMenu.tsx (context menu)

The shadcn Dialog already does most of this; the issue is whether the implementation is correct and whether the wrapping component (DeleteDialog) is overriding any of it.

Severity

Low. Mouse users don't notice. Screen reader users notice.

Discovered by

Gap analysis (2026-10-02) of gallery accessibility.

The Editor's onContextMenu (per #236) is the parallel concern.
The keyboard navigation (per #240) is the broader accessibility surface.
Most modern UI libraries handle this; the issue is whether the shadcn wrapper preserves the default behavior.

## Finding `src/components/gallery/DeleteDialog.tsx` (per the snippet I saw) is a shadcn Dialog. The shadcn `Dialog` component (line 4) should handle focus trapping and initial focus. Let me verify. `grep -n 'autoFocus\|onOpenAutoFocus' src/components/gallery/DeleteDialog.tsx` returns 0 hits. The shadcn Dialog should auto-focus the first focusable element, but the actual implementation may not. `grep -n 'onCloseAutoFocus' src/components/gallery/DeleteDialog.tsx` returns 0 hits. On close, the focus return behavior depends on the shadcn implementation. A screen reader user opens the Gallery: - Tabs to a design card's delete button - Activates it - The dialog opens - Focus is... wherever it was? The Delete button? Off-screen? - Presses Tab - Tabs through invisible elements - The Cancel button is eventually focused This is a common accessibility bug with custom modals. Standard behavior: - On open: focus moves to the first focusable element in the dialog (or the dialog itself) - While open: focus is trapped inside the dialog (Tab cycles through dialog elements) - On close: focus returns to the element that opened the dialog ## Where - DeleteDialog: `src/components/gallery/DeleteDialog.tsx` - Other dialogs: `NewFolderDialog`, `RightClickMenu`, `ExportDialog` (workspace) - Shadcn Dialog: `src/components/ui/dialog.tsx` (the underlying implementation) ## Suggested fix shape Verify the shadcn Dialog's `onOpenAutoFocus` and `onCloseAutoFocus` handlers are set correctly. If they're missing: 1. In `DeleteDialog.tsx`, set the focus on the Cancel button when the dialog opens: ```tsx <DialogContent onOpenAutoFocus={(e) => { e.preventDefault(); cancelButtonRef.current?.focus(); }} > ... <Button ref={cancelButtonRef} onClick={...}>Cancel</Button> </DialogContent> ``` 2. shadcn's Dialog (per the Radix UI docs) automatically traps focus inside the dialog. Verify by Tab-ing through the dialog and checking focus doesn't escape. 3. On close, Radix restores focus to the element that opened the dialog. Verify by Tab-ing and checking focus returns to the Delete button. The same fix applies to: - `NewFolderDialog.tsx` (folder create) - `RenameDialog.tsx` (design rename, if exists) - `SettingsModal.tsx` (settings) - `ExportDialog.tsx` (export) - `RightClickMenu.tsx` (context menu) The shadcn Dialog already does most of this; the issue is whether the implementation is correct and whether the wrapping component (DeleteDialog) is overriding any of it. ## Severity Low. Mouse users don't notice. Screen reader users notice. ## Discovered by Gap analysis (2026-10-02) of gallery accessibility. ## Related - The Editor's `onContextMenu` (per #236) is the parallel concern. - The keyboard navigation (per #240) is the broader accessibility surface. - Most modern UI libraries handle this; the issue is whether the shadcn wrapper preserves the default behavior.

Mark added the

feature-request

triage-approved

labels

2026-10-04 02:35:16 +00:00

Mark referenced this issue from a commit

2026-10-04 08:52:53 +00:00

fix(#241): pin initial focus to Cancel in DeleteDialog and DeleteFolderDialog

Mark commented

2026-10-04 08:57:35 +00:00

Author

Owner

Closed by fix(#241): commit 1276fbd on main. Both destructive-action dialogs (DeleteDialog for designs, DeleteFolderDialog for custom folders) now pin initial focus to the safe Cancel button via onOpenAutoFocus. The Button UI primitive is now wrapped in React.forwardRef so any consumer can grab a handle for focus management. 10 new tests in src/components/gallery/issue-241-focus-management.test.tsx; full suite 1244 passing, 1 skipped.

Closed by fix(#241): commit 1276fbd on main. Both destructive-action dialogs (DeleteDialog for designs, DeleteFolderDialog for custom folders) now pin initial focus to the safe Cancel button via `onOpenAutoFocus`. The Button UI primitive is now wrapped in `React.forwardRef` so any consumer can grab a handle for focus management. 10 new tests in `src/components/gallery/issue-241-focus-management.test.tsx`; full suite 1244 passing, 1 skipped.

Mark closed this issue

2026-10-04 08:57:37 +00:00