Cloonar/deckle

Fork 0

Escape closes only the topmost dialog, menu or panel; menus close when focus leaves #140

New issue

Closed

opened 2026-10-04 02:35:04 +02:00 by dominik.polakovics · 0 comments

dominik.polakovics commented

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

Owner

This was generated by AI during triage.

Summary

Give the editor one shared module that decides which open layer (a dialog, a menu or the Media details panel) an Escape, an outside press or a focus change closes, and move every dialog and menu and the details panel onto it. One Escape then closes only the topmost layer: today an Escape in a dialog also closes the Media details panel behind it. Every menu also closes when focus leaves it, as the header's menus already do: today the editor's own menus stay open, so two element menus can be open at once.

Current behavior

Escape closes more than the topmost layer. Reproduced in vitest/jsdom on main (16bb339) during triage; the reporter saw the first case in headless Chromium.

Media module with a file's details panel open: Escape in the rename dialog (name field or Cancel), in the delete dialog, or in any other dialog the module opens closes the dialog and the panel. Escape in an open tile or folder ⋯ menu closes the menu and the panel. With nothing in front of the panel, Escape closes it, which is intended.
Page pane: open row A's ⋮ menu, then open row B's ⋮ from the keyboard (no mouse press, so menu A's outside-press dismissal never fires) and choose Delete…. Menu A is still open behind the dialog, and one Escape in the dialog closes the dialog and menu A.
Cause: Solid delegates onKeyDown. It registers one keydown listener on document and calls the handlers itself, so the stopPropagation() in Dialog's and the menus' Escape handlers runs at document, the node where the details panel's and the element menu's own listeners also sit, and cannot keep the event from them. The comment above the panel's listener claims the opposite. Only the header's host-navigation menus (#139) avoid this, by taking Escape in the capture phase and stopping it there.

Every layer handles dismissal on its own, with different rules (checked in code; the focus column and the rich-text row confirmed in jsdom):

Layer	Escape	Press outside	Focus leaves (incl. a click into the preview)	Focus after Escape
Header host-navigation menus (`HostNav.tsx`)	yes	yes	yes	the menu's button
Page tree row menu (`TreePane.tsx`)	yes	yes	no	the menu's button
Page editor's header menu (`PageEditorChrome.tsx`)	yes	yes	no	the menu's button
Media menus: tile, folder tile, folder-tree row (`PopMenu` in `MediaModule.tsx`)	yes	yes	no	the page body
Element ⋮ menu (`ElementMenu` in `PagePane.tsx`)	yes	yes	no	the page body
Rich-text Insert and Table menus (`web/src/rte/RichTextField.tsx`)	no	no	no	(stays open)
Dialogs (`Dialog.tsx`)	yes	backdrop	focus is trapped	the opener, on close
Media details panel (`MediaModule.tsx`)	yes, every Escape	no	no	–

The rich-text menus close only on their own button, the other toolbar menu's button, a choice, or a click on a chip.

Desired behavior

One stack of open layers. Every dialog, every menu and the Media details panel is on the stack while it is open, in the order they opened.

Escape closes the topmost layer and only that one, wherever focus is, the page body included. With nothing open, the module does nothing. An Escape that belongs to text composition in an input method (isComposing), or that something already handled with preventDefault() (a field that uses Escape itself), closes nothing.
Menus (header host-navigation, page tree row, page editor header, the Media menus, element ⋮, rich-text Insert and Table) close on Escape, on a press outside the menu and its button, when focus moves outside the menu and its button, and when the window loses focus (a click into the preview frame), as well as after a choice, as today. Escape returns focus to the menu's button; the rich-text menus leave focus where it is, so the editor keeps its selection. Closing for any other reason leaves focus alone. As a consequence at most one menu is open at a time.
Dialogs keep everything else they do: focus on open, the Tab trap, a press on the backdrop closes, focus returns to the opener. A dialog opened from inside another (the link or media picker from the inline-element dialog) is above it, so Escape closes the inner one first.
The Media details panel closes on Escape only when it is the topmost layer, and on its ✕. It has no outside-press or focus rule.
Layers are independent. Closing one never closes another; a layer whose component unmounts leaves the stack.
No listeners while nothing is open. With no layer open, the module has nothing registered on document or window. #139 pinned this for the header menus; it now holds for all layers.

Decisions

One shared module written in deckle, no library (maintainer, in triage). Considered:
- The browser's own <dialog> with showModal() and popovers stack themselves, but jsdom, which runs every UI test, has neither (showModal and showPopover don't exist there), the details panel is neither, and CloseWatcher is limited availability on MDN.
- Kobalte and Ark UI are full component kits: Dialog and every menu would be rebuilt on their components. Kobalte 2.0 is in alpha.
- Corvu's solid-dismissible has had no release since August 2024, and its stack follows the component tree, so sibling layers such as the details panel and a dialog each think they are on top, which is this bug again.
- Zag's @zag-js/dismissable is maintained and small, but it never sees a click into the cross-origin preview frame (it listens only inside same-origin frames and has no window-blur fallback), so deckle would keep its own code for that anyway, and deckle already maintains this logic, tested, in the header menus.
The module generalizes the header menus' dismissal (#139), the way dragReorder.ts became the one drag-and-drop skeleton (#124).
The stack decides who closes, not event propagation. Under Solid's delegation every onKeyDown handler runs at document, so stopPropagation() cannot order handlers there, and the order of listener registration must not matter either.
A field that handles Escape itself wins by calling preventDefault(), the convention every Escape handler follows today, so a field inside a dialog can use Escape without closing the dialog.
Every menu closes when focus leaves it, matching the header menus' documented behavior (README, "Dismissal and keyboard") and the WAI-ARIA menu pattern (Tab out of a menu closes it). This also fixes the element menu's two-open-at-once state; the maintainer chose to fix it in this issue rather than a separate one.
Escape returns focus to the menu's button (WAI-ARIA menu button pattern), as the header, page tree and page editor menus already do; the Media and element menus drop focus to the page body today.
The rich-text Insert and Table menus join. Today they ignore Escape, outside presses and focus changes; leaving them out would leave two menus outside the rules.
Layers are independent, unlike Zag's model, where closing a layer also closes every layer opened after it. In deckle a nested layer is a child component and unmounts with its parent anyway, while siblings such as the details panel and a dialog are unrelated.
Delete what the module replaces, in this issue: each component's own Escape handling (the Escape branches in Dialog, PopMenu, the page tree and page editor menus; the document Escape listeners of the details panel and ElementMenu, including its stopPropagation(), which does nothing at document), the outside-press mousedown listeners, the header menus' pointerdown/focusin/keydown/blur dismissal listeners, and the details panel's wrong comment. What stays where it is: arrow-key navigation inside menus, positioning (the header menus' scroll/resize placement, the fixed position of right-click menus), Dialog's focus handling and Tab trap, and the page tree's inline rename and new-page inputs, which keep their own Escape.

Key interfaces

New: the shared module. Starting hint: beside dragReorder.ts in web/src/. Its name and API are the implementer's choice. A layer will need to give it its element (for a menu also its button), which rules apply to it, and a close callback that learns why it is closing (Escape, a press outside, focus leaving), so a menu can decide where focus goes.
Moving onto it:
- Dialog (web/src/components/Dialog.tsx), whose handleKeyDown keeps only the Tab trap.
- PopMenu and the details-panel effect in web/src/components/MediaModule.tsx.
- The row menu in web/src/components/TreePane.tsx (handleMenuKeyDown and its mousedown effect).
- The header menu in web/src/components/PageEditorChrome.tsx (onMenuKeyDown and its mousedown effect).
- ElementMenu in web/src/components/PagePane.tsx.
- The host-navigation menus in web/src/components/HostNav.tsx.
- The Insert and Table menus in web/src/rte/RichTextField.tsx (insertOpen, tableOpen).

Acceptance criteria

Escape:

Media module, a file's details panel open: Escape in the rename dialog's name field, on its Cancel button, or in the delete dialog closes only the dialog.
Media module, details panel open: Escape in an open tile ⋯ menu closes only the menu.
Media module, details panel open with nothing in front of it: Escape closes the panel, also with focus on the page body.
A picker opened from the inline-element dialog: Escape closes only the picker.
An Escape with isComposing: true, or one a field already handled with preventDefault(), closes nothing.
With nothing open, the page tree's inline rename and new-page inputs still cancel on Escape.

Menus (header host-navigation, page tree row, page editor header, Media tile / folder tile / folder-tree row, element ⋮, rich-text Insert and Table):

Each closes on Escape, on a press outside it and its button, when focus moves outside it and its button, and when the window loses focus.
After Escape, focus is on the menu's button; the rich-text menus leave focus where it was.
Page pane: with row A's ⋮ menu open, moving focus to row B's ⋮ and opening it leaves only menu B open, and a dialog opened from menu B has no menu behind it.
The header menus still behave as README's "Dismissal and keyboard" paragraph describes.

The module:

With no layer open, it has nothing registered on document or window; closing the last layer removes exactly what opening the first one added.
None of the components above keeps Escape or outside-press handling of its own.
The module's header comment states the rules, including why stopPropagation() can't order handlers under Solid's delegation.
cd web && npm test passes. Tests that pinned the old mechanics (the header menus' capture-phase keydown and their own listener set, outside presses fired as mouseDown) are rewritten to pin the behavior instead.

How to verify

cd web && npm test; make test runs the full suite (web build, vitest, Go).
New tests fail on main and pass after the change: unit tests for the module (beside src/test/dragReorder.test.ts), the cases above in src/test/MediaModule.test.tsx and src/test/ElementCrud.test.tsx, and the Escape-focus and focus-leaving cases next to each menu's existing tests (src/test/HostNav.test.tsx, src/test/PageManage.test.tsx, src/test/rte/tableMenu.test.tsx, …).
jsdom is enough: this is event order and focus, not layout. Fire Escape at the focused element (fireEvent.keyDown(el, { key: "Escape" })) so it goes through Solid's delegated listener as in a browser. A click into the preview frame is a blur on window. A keyboard activation of a button is fireEvent.click with no mouseDown before it.

Assumptions to check

The inventory above is complete. Before starting, search web/src again for Escape handling, document/window listeners and aria-haspopup; anything else that opens over the page joins the module.
A handler that keeps Escape for itself calls preventDefault(). True today for the page tree's inline inputs; the "already handled" rule depends on it.
A click into the cross-origin preview frame reaches the editor only as the window's blur; the header menus rely on this today (see the comment in HostNav.tsx).
The module has to see Escape after Solid's delegated handlers ran, without depending on registration order. A window listener in the bubble phase runs after everything on document, but a stopPropagation() on document hides the event from it. Check that every remaining stopPropagation() on a key event (Dialog's Tab trap, the page tree's inline inputs) is one that should hide it.

Out of scope

Escape pressed while focus is inside the preview frame: its key events never reach the editor's document.
The markup, ARIA roles, arrow-key navigation and positioning of dialogs and menus.
deckle-hosted: nothing it calls changes, and the header's host-navigation data is untouched.
scaffold/site/AGENTS.md and README: nothing they document changes (README's header-menu paragraph stays true).

Blocked by

None - can start immediately

Original report

Original title: "Media module: Escape in a dialog also closes the details panel behind it". The one-line fix it suggests was superseded in triage by the decisions above.

What happens

In the Media module, pressing Escape in a dialog that is open over the details panel closes the dialog and the details panel. One Escape should close only the dialog.

How to reproduce

Seen in headless Chromium against a harness built from main at 9fb6f57.

Open the Media module with at least one file in the library.

Open a file's details panel.

From the tile's ⋯ menu choose "Rename…".

Press Escape once, with focus in the name field or on Cancel.

Expected: the rename dialog closes and the details panel stays open.
Actual: both close.

Cause

web/src/components/MediaModule.tsx (around line 988) registers a keydown listener on document while the details panel is open and closes the panel on every Escape. Its comment says a dialog's own Escape handling stops propagation before this listener sees the event:
// Escape closes the details panel — unless a dialog is open, whose own
// Escape handling stops propagation before this ever sees it.
That does not hold. Dialog.tsx handles Escape through Solid's onKeyDown, and Solid delegates keydown to a single listener on document. So the dialog's stopPropagation() runs at document, the same node the panel's listener sits on, and cannot keep the event from it.

Suggested fix

Dialog already calls preventDefault() on Escape. Let the panel's listener ignore an Escape that something in front of it has handled, and correct the comment:
if (event.key === "Escape" && !event.defaultPrevented) {
  setDetail(null);
}
This one-line change was tried while working on #139 and the existing media tests still passed. It was taken back out there because changes to modules are out of scope for #139. A test for the case above does not exist yet and should come with the fix.

Also worth a look

web/src/components/PagePane.tsx (around line 744) has the same pattern: a document keydown listener that reacts to every Escape while the element menu is open. No user-visible defect was observed there; it was not tested.

Found

During the browser QA for #139. Not caused by #139 and not changed by it.

> *This was generated by AI during triage.* ## Summary Give the editor one shared module that decides which open layer (a dialog, a menu or the Media details panel) an Escape, an outside press or a focus change closes, and move every dialog and menu and the details panel onto it. One Escape then closes only the topmost layer: today an Escape in a dialog also closes the Media details panel behind it. Every menu also closes when focus leaves it, as the header's menus already do: today the editor's own menus stay open, so two element menus can be open at once. ## Current behavior **Escape closes more than the topmost layer.** Reproduced in vitest/jsdom on `main` (16bb339) during triage; the reporter saw the first case in headless Chromium. - Media module with a file's details panel open: Escape in the rename dialog (name field or Cancel), in the delete dialog, or in any other dialog the module opens closes the dialog **and** the panel. Escape in an open tile or folder ⋯ menu closes the menu **and** the panel. With nothing in front of the panel, Escape closes it, which is intended. - Page pane: open row A's ⋮ menu, then open row B's ⋮ from the keyboard (no mouse press, so menu A's outside-press dismissal never fires) and choose Delete…. Menu A is still open behind the dialog, and one Escape in the dialog closes the dialog **and** menu A. - Cause: Solid delegates `onKeyDown`. It registers one `keydown` listener on `document` and calls the handlers itself, so the `stopPropagation()` in `Dialog`'s and the menus' Escape handlers runs at `document`, the node where the details panel's and the element menu's own listeners also sit, and cannot keep the event from them. The comment above the panel's listener claims the opposite. Only the header's host-navigation menus (#139) avoid this, by taking Escape in the capture phase and stopping it there. **Every layer handles dismissal on its own, with different rules** (checked in code; the focus column and the rich-text row confirmed in jsdom): | Layer | Escape | Press outside | Focus leaves (incl. a click into the preview) | Focus after Escape | | --- | --- | --- | --- | --- | | Header host-navigation menus (`HostNav.tsx`) | yes | yes | yes | the menu's button | | Page tree row menu (`TreePane.tsx`) | yes | yes | no | the menu's button | | Page editor's header menu (`PageEditorChrome.tsx`) | yes | yes | no | the menu's button | | Media menus: tile, folder tile, folder-tree row (`PopMenu` in `MediaModule.tsx`) | yes | yes | no | the page body | | Element ⋮ menu (`ElementMenu` in `PagePane.tsx`) | yes | yes | no | the page body | | Rich-text Insert and Table menus (`web/src/rte/RichTextField.tsx`) | no | no | no | (stays open) | | Dialogs (`Dialog.tsx`) | yes | backdrop | focus is trapped | the opener, on close | | Media details panel (`MediaModule.tsx`) | yes, every Escape | no | no | – | The rich-text menus close only on their own button, the other toolbar menu's button, a choice, or a click on a chip. ## Desired behavior **One stack of open layers.** Every dialog, every menu and the Media details panel is on the stack while it is open, in the order they opened. - **Escape closes the topmost layer and only that one**, wherever focus is, the page body included. With nothing open, the module does nothing. An Escape that belongs to text composition in an input method (`isComposing`), or that something already handled with `preventDefault()` (a field that uses Escape itself), closes nothing. - **Menus** (header host-navigation, page tree row, page editor header, the Media menus, element ⋮, rich-text Insert and Table) close on Escape, on a press outside the menu and its button, when focus moves outside the menu and its button, and when the window loses focus (a click into the preview frame), as well as after a choice, as today. Escape returns focus to the menu's button; the rich-text menus leave focus where it is, so the editor keeps its selection. Closing for any other reason leaves focus alone. As a consequence at most one menu is open at a time. - **Dialogs** keep everything else they do: focus on open, the Tab trap, a press on the backdrop closes, focus returns to the opener. A dialog opened from inside another (the link or media picker from the inline-element dialog) is above it, so Escape closes the inner one first. - **The Media details panel** closes on Escape only when it is the topmost layer, and on its ✕. It has no outside-press or focus rule. - **Layers are independent.** Closing one never closes another; a layer whose component unmounts leaves the stack. - **No listeners while nothing is open.** With no layer open, the module has nothing registered on `document` or `window`. #139 pinned this for the header menus; it now holds for all layers. ## Decisions - **One shared module written in deckle, no library** (maintainer, in triage). Considered: - The browser's own `<dialog>` with `showModal()` and popovers stack themselves, but jsdom, which runs every UI test, has neither (`showModal` and `showPopover` don't exist there), the details panel is neither, and `CloseWatcher` is limited availability on MDN. - Kobalte and Ark UI are full component kits: `Dialog` and every menu would be rebuilt on their components. Kobalte 2.0 is in alpha. - Corvu's `solid-dismissible` has had no release since August 2024, and its stack follows the component tree, so sibling layers such as the details panel and a dialog each think they are on top, which is this bug again. - Zag's `@zag-js/dismissable` is maintained and small, but it never sees a click into the cross-origin preview frame (it listens only inside same-origin frames and has no window-blur fallback), so deckle would keep its own code for that anyway, and deckle already maintains this logic, tested, in the header menus. The module generalizes the header menus' dismissal (#139), the way `dragReorder.ts` became the one drag-and-drop skeleton (#124). - **The stack decides who closes, not event propagation.** Under Solid's delegation every `onKeyDown` handler runs at `document`, so `stopPropagation()` cannot order handlers there, and the order of listener registration must not matter either. - **A field that handles Escape itself wins** by calling `preventDefault()`, the convention every Escape handler follows today, so a field inside a dialog can use Escape without closing the dialog. - **Every menu closes when focus leaves it**, matching the header menus' documented behavior (README, "Dismissal and keyboard") and the WAI-ARIA menu pattern (Tab out of a menu closes it). This also fixes the element menu's two-open-at-once state; the maintainer chose to fix it in this issue rather than a separate one. - **Escape returns focus to the menu's button** (WAI-ARIA menu button pattern), as the header, page tree and page editor menus already do; the Media and element menus drop focus to the page body today. - **The rich-text Insert and Table menus join.** Today they ignore Escape, outside presses and focus changes; leaving them out would leave two menus outside the rules. - **Layers are independent**, unlike Zag's model, where closing a layer also closes every layer opened after it. In deckle a nested layer is a child component and unmounts with its parent anyway, while siblings such as the details panel and a dialog are unrelated. - **Delete what the module replaces**, in this issue: each component's own Escape handling (the Escape branches in `Dialog`, `PopMenu`, the page tree and page editor menus; the `document` Escape listeners of the details panel and `ElementMenu`, including its `stopPropagation()`, which does nothing at `document`), the outside-press `mousedown` listeners, the header menus' `pointerdown`/`focusin`/`keydown`/`blur` dismissal listeners, and the details panel's wrong comment. What stays where it is: arrow-key navigation inside menus, positioning (the header menus' `scroll`/`resize` placement, the fixed position of right-click menus), `Dialog`'s focus handling and Tab trap, and the page tree's inline rename and new-page inputs, which keep their own Escape. ## Key interfaces - New: the shared module. Starting hint: beside `dragReorder.ts` in `web/src/`. Its name and API are the implementer's choice. A layer will need to give it its element (for a menu also its button), which rules apply to it, and a close callback that learns why it is closing (Escape, a press outside, focus leaving), so a menu can decide where focus goes. - Moving onto it: - `Dialog` (`web/src/components/Dialog.tsx`), whose `handleKeyDown` keeps only the Tab trap. - `PopMenu` and the details-panel effect in `web/src/components/MediaModule.tsx`. - The row menu in `web/src/components/TreePane.tsx` (`handleMenuKeyDown` and its `mousedown` effect). - The header menu in `web/src/components/PageEditorChrome.tsx` (`onMenuKeyDown` and its `mousedown` effect). - `ElementMenu` in `web/src/components/PagePane.tsx`. - The host-navigation menus in `web/src/components/HostNav.tsx`. - The Insert and Table menus in `web/src/rte/RichTextField.tsx` (`insertOpen`, `tableOpen`). ## Acceptance criteria Escape: - [ ] Media module, a file's details panel open: Escape in the rename dialog's name field, on its Cancel button, or in the delete dialog closes only the dialog. - [ ] Media module, details panel open: Escape in an open tile ⋯ menu closes only the menu. - [ ] Media module, details panel open with nothing in front of it: Escape closes the panel, also with focus on the page body. - [ ] A picker opened from the inline-element dialog: Escape closes only the picker. - [ ] An Escape with `isComposing: true`, or one a field already handled with `preventDefault()`, closes nothing. - [ ] With nothing open, the page tree's inline rename and new-page inputs still cancel on Escape. Menus (header host-navigation, page tree row, page editor header, Media tile / folder tile / folder-tree row, element ⋮, rich-text Insert and Table): - [ ] Each closes on Escape, on a press outside it and its button, when focus moves outside it and its button, and when the window loses focus. - [ ] After Escape, focus is on the menu's button; the rich-text menus leave focus where it was. - [ ] Page pane: with row A's ⋮ menu open, moving focus to row B's ⋮ and opening it leaves only menu B open, and a dialog opened from menu B has no menu behind it. - [ ] The header menus still behave as README's "Dismissal and keyboard" paragraph describes. The module: - [ ] With no layer open, it has nothing registered on `document` or `window`; closing the last layer removes exactly what opening the first one added. - [ ] None of the components above keeps Escape or outside-press handling of its own. - [ ] The module's header comment states the rules, including why `stopPropagation()` can't order handlers under Solid's delegation. - [ ] `cd web && npm test` passes. Tests that pinned the old mechanics (the header menus' capture-phase `keydown` and their own listener set, outside presses fired as `mouseDown`) are rewritten to pin the behavior instead. ## How to verify - `cd web && npm test`; `make test` runs the full suite (web build, vitest, Go). - New tests fail on `main` and pass after the change: unit tests for the module (beside `src/test/dragReorder.test.ts`), the cases above in `src/test/MediaModule.test.tsx` and `src/test/ElementCrud.test.tsx`, and the Escape-focus and focus-leaving cases next to each menu's existing tests (`src/test/HostNav.test.tsx`, `src/test/PageManage.test.tsx`, `src/test/rte/tableMenu.test.tsx`, …). - jsdom is enough: this is event order and focus, not layout. Fire Escape at the focused element (`fireEvent.keyDown(el, { key: "Escape" })`) so it goes through Solid's delegated listener as in a browser. A click into the preview frame is a `blur` on `window`. A keyboard activation of a button is `fireEvent.click` with no `mouseDown` before it. ## Assumptions to check - The inventory above is complete. Before starting, search `web/src` again for Escape handling, `document`/`window` listeners and `aria-haspopup`; anything else that opens over the page joins the module. - A handler that keeps Escape for itself calls `preventDefault()`. True today for the page tree's inline inputs; the "already handled" rule depends on it. - A click into the cross-origin preview frame reaches the editor only as the window's `blur`; the header menus rely on this today (see the comment in `HostNav.tsx`). - The module has to see Escape after Solid's delegated handlers ran, without depending on registration order. A `window` listener in the bubble phase runs after everything on `document`, but a `stopPropagation()` on `document` hides the event from it. Check that every remaining `stopPropagation()` on a key event (`Dialog`'s Tab trap, the page tree's inline inputs) is one that should hide it. ## Out of scope - Escape pressed while focus is inside the preview frame: its key events never reach the editor's document. - The markup, ARIA roles, arrow-key navigation and positioning of dialogs and menus. - deckle-hosted: nothing it calls changes, and the header's host-navigation data is untouched. - `scaffold/site/AGENTS.md` and README: nothing they document changes (README's header-menu paragraph stays true). ## Blocked by None - can start immediately ## Original report Original title: "Media module: Escape in a dialog also closes the details panel behind it". The one-line fix it suggests was superseded in triage by the decisions above. > ## What happens > > In the Media module, pressing Escape in a dialog that is open over the details panel closes the dialog **and** the details panel. One Escape should close only the dialog. > > ## How to reproduce > > Seen in headless Chromium against a harness built from `main` at `9fb6f57`. > > 1. Open the Media module with at least one file in the library. > 2. Open a file's details panel. > 3. From the tile's ⋯ menu choose "Rename…". > 4. Press Escape once, with focus in the name field or on Cancel. > > Expected: the rename dialog closes and the details panel stays open. > Actual: both close. > > ## Cause > > `web/src/components/MediaModule.tsx` (around line 988) registers a `keydown` listener on `document` while the details panel is open and closes the panel on every Escape. Its comment says a dialog's own Escape handling stops propagation before this listener sees the event: > > ```ts > // Escape closes the details panel — unless a dialog is open, whose own > // Escape handling stops propagation before this ever sees it. > ``` > > That does not hold. `Dialog.tsx` handles Escape through Solid's `onKeyDown`, and Solid delegates `keydown` to a single listener on `document`. So the dialog's `stopPropagation()` runs at `document`, the same node the panel's listener sits on, and cannot keep the event from it. > > ## Suggested fix > > `Dialog` already calls `preventDefault()` on Escape. Let the panel's listener ignore an Escape that something in front of it has handled, and correct the comment: > > ```ts > if (event.key === "Escape" && !event.defaultPrevented) { > setDetail(null); > } > ``` > > This one-line change was tried while working on #139 and the existing media tests still passed. It was taken back out there because changes to modules are out of scope for #139. A test for the case above does not exist yet and should come with the fix. > > ## Also worth a look > > `web/src/components/PagePane.tsx` (around line 744) has the same pattern: a `document` `keydown` listener that reacts to every Escape while the element menu is open. No user-visible defect was observed there; it was not tested. > > ## Found > > During the browser QA for #139. Not caused by #139 and not changed by it.

dominik.polakovics added the

needs-triage

label

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

dominik.polakovics referenced this issue

2026-10-04 03:25:29 +02:00

feat(editor): header slot for host navigation, compact preview status when hosted #141

dominik.polakovics changed title from ~~Media module: Escape in a dialog also closes the details panel behind it~~ to Escape closes only the topmost dialog, menu or panel; menus close when focus leaves

2026-10-07 22:20:54 +02:00

dominik.polakovics added

bug

ready-for-agent

and removed

needs-triage

labels

2026-10-07 22:21:02 +02:00

dominik.polakovics referenced this issue from a commit

2026-10-07 22:43:04 +02:00

fix(editor): Escape closes only the topmost layer; menus close when focus leaves

dominik.polakovics referenced this issue from a pull request that will close it,