From 3215cef13b74d6549feed0d9825e48cb3b1591ee Mon Sep 17 00:00:00 2001 From: Kabarza Date: Sat, 25 Jul 2026 01:46:57 +0200 Subject: [PATCH] fix(ui): restore Escape dismissal in settings and while tooltips show MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Escape silently stopped closing surfaces in two independent ways. SettingsModal bound Escape as a React `onKeyDown` on the dialog element, so it only fired while focus sat inside the dialog subtree. Clicking any non-focusable spot in the panel (a label, the section header, empty space) blurs to `` — outside that subtree — after which Escape was dead for the rest of the session while backdrop click kept working. It was the only modal built this way; Dialog, ModuleInserterDialog, MediaPickerModal, StepUpDialog and ConfirmDeleteDialog all bind on document/window. Moved it onto a document listener to match, gated so only the topmost dialog reacts (a nested media picker must not collapse the whole stack), and added tabIndex={-1} to the panel so dead-space clicks keep focus inside and the Tab trap holds. Tooltip's window capture-phase handler called preventDefault() + stopPropagation() on Escape and only hid the tooltip. Tooltips open on plain hover, so resting the pointer on any trigger swallowed Escape app-wide — no modal, context menu or spotlight could close. It now hides the tooltip and lets the key through. Also fixes the focus-return branch in SettingsModal, which was dead: all three layouts unmount the modal on close, so `open` never transitioned to false while mounted and focus was never returned to the trigger (WCAG 2.4.3 / Guideline #225). Restoring in the effect cleanup makes it run. The gate tests asserted the broken behaviour — one required Tooltip to consume Escape, three matched the dead triggerRef branch as source strings. Replaced with behavioural coverage for focus-independent close, stacked-dialog layering and focus return on unmount. Verified: bun test (6282 pass), bun run build, bun run lint, plus a live repro in the running dev admin before and after. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Se93H46Eck37iJBJwjmWV2 --- docs/reference/ui-primitives.md | 4 + src/__tests__/settings/settingsModal.test.tsx | 68 +++++++++++++-- src/__tests__/toolbar/toolbar.test.ts | 32 +++---- src/__tests__/ui/tooltip.test.tsx | 12 ++- src/admin/modals/Settings/SettingsModal.tsx | 85 +++++++++++++------ src/ui/components/Tooltip/Tooltip.tsx | 10 ++- 6 files changed, 153 insertions(+), 58 deletions(-) diff --git a/docs/reference/ui-primitives.md b/docs/reference/ui-primitives.md index a76c36fae..188fdf52c 100644 --- a/docs/reference/ui-primitives.md +++ b/docs/reference/ui-primitives.md @@ -319,6 +319,8 @@ import { Dialog } from '@ui/components/Dialog' `Dialog` traps focus, restores it on close, escape-closes, click-outside-closes (configurable). Modal by default. Replaces native `alert()` / `confirm()` (which are banned by `no-native-browser-dialogs.test.ts`). +**Modal shells must bind Escape on `document`, not as a React `onKeyDown` on the dialog element.** React's delegated handler only fires while focus sits inside the dialog subtree, and a click on any non-focusable spot in a panel blurs to `` — outside that subtree — leaving Escape silently dead while backdrop click still works. Pair it with `tabIndex={-1}` on the panel so such clicks land on the panel instead of `` and the Tab trap holds. + --- ## `Tooltip` @@ -333,6 +335,8 @@ import { Tooltip } from '@ui/components/Tooltip' Replaces native `title="..."` (gated by `no-native-title-tooltips.test.ts`). Works on disabled buttons because `mouseenter` fires on disabled `