fix(webui): broaden escape overlay guard + restore two-step cancel

webui/src/features/board/harness/canvas/use-board-keyboard.test.ts

@@ -0,0 +1,92 @@
+import { describe, expect, it } from "vitest"
+import { decideBoardEscape, type EscapeAppState } from "./use-board-keyboard"
+
+
+type EscKey = Pick<KeyboardEvent, "key" | "metaKey" | "ctrlKey" | "altKey" | "shiftKey">
+
+
+/** A bare Escape press (no modifiers). */
+const escape = (over: Partial<EscKey> = {}): EscKey => ({
+  key: "Escape",
+  metaKey: false,
+  ctrlKey: false,
+  altKey: false,
+  shiftKey: false,
+  ...over,
+})
+
+
+/** Board state with a create tool active on the canvas and no overlay open. */
+const appState = (over: Partial<EscapeAppState> = {}): EscapeAppState => ({
+  viewMode: "board",
+  chromeDialog: null,
+  activeNodeSurface: null,
+  presentationMode: false,
+  chatSheetOpen: false,
+  tool: "rect",
+  ...over,
+})
+
+
+const dom = (over: Partial<{ isTyping: boolean; insideOverlayDom: boolean }> = {}) => ({
+  isTyping: false,
+  insideOverlayDom: false,
+  ...over,
+})
+
+
+describe("decideBoardEscape", () => {
+  it("switches a create tool back to select (the core feature)", () => {
+    expect(decideBoardEscape(escape(), appState({ tool: "rect" }), dom())).toBe("switch-to-select")
+    expect(decideBoardEscape(escape(), appState({ tool: "arrow" }), dom())).toBe("switch-to-select")
+  })
+
+  it("is a no-op when the tool is already select (library owns the 2nd-press deselect)", () => {
+    // Two-step parity: with nothing to put away we don't consume, so the
+    // harness's own Escape handler runs and deselects.
+    expect(decideBoardEscape(escape(), appState({ tool: "select" }), dom())).toBe("no-op")
+  })
+
+  it("ignores non-Escape keys and modified Escape presses", () => {
+    expect(decideBoardEscape(escape({ key: "a" }), appState(), dom())).toBe("no-op")
+    expect(decideBoardEscape(escape({ metaKey: true }), appState(), dom())).toBe("no-op")
+    expect(decideBoardEscape(escape({ ctrlKey: true }), appState(), dom())).toBe("no-op")
+    expect(decideBoardEscape(escape({ altKey: true }), appState(), dom())).toBe("no-op")
+    expect(decideBoardEscape(escape({ shiftKey: true }), appState(), dom())).toBe("no-op")
+  })
+
+  it("ignores Escape while typing", () => {
+    expect(decideBoardEscape(escape(), appState(), dom({ isTyping: true }))).toBe("no-op")
+  })
+
+  it("is a no-op off the board canvas (files / list views)", () => {
+    expect(decideBoardEscape(escape(), appState({ viewMode: "files" }), dom())).toBe("no-op")
+    expect(decideBoardEscape(escape(), appState({ viewMode: "list" }), dom())).toBe("no-op")
+  })
+
+  it("defers to store-tracked overlays instead of resetting the tool (#244/#245)", () => {
+    const cases: Partial<EscapeAppState>[] = [
+      { chromeDialog: "shape-menu" },
+      { activeNodeSurface: { nodeId: "n1", kind: "sheet" } },
+      { presentationMode: true },
+      { chatSheetOpen: true }, // non-modal → focus on canvas, needs the flag
+    ]
+    for (const over of cases) {
+      expect(decideBoardEscape(escape(), appState(over), dom())).toBe("defer-to-overlay")
+    }
+  })
+
+  it("defers when the Escape is focused inside a store-less Radix overlay (#244)", () => {
+    // Covers Select / combobox / listbox / popover — matched via the DOM guard,
+    // surfaced here as `insideOverlayDom`.
+    expect(decideBoardEscape(escape(), appState(), dom({ insideOverlayDom: true }))).toBe(
+      "defer-to-overlay",
+    )
+  })
+
+  it("checks typing before overlay state (a focused input always wins)", () => {
+    expect(
+      decideBoardEscape(escape(), appState({ chromeDialog: "shape-menu" }), dom({ isTyping: true })),
+    ).toBe("no-op")
+  })
+})

webui/src/features/board/harness/canvas/use-board-keyboard.ts

@@ -1,7 +1,7 @@
 import { useEffect } from "react"
 import type { CanvasStore } from "@canvas-harness/core"
 import { isTypingTarget } from "@/lib/dom/is-typing-target"
-import { useBoardAppStore } from "../store/board-app-store"
+import { useBoardAppStore, type BoardAppState } from "../store/board-app-store"
 
 
 /**
@@ -32,6 +32,66 @@ const TOOL_SHORTCUTS: Record<string, string> = {
 }
 
 
+/**
+ * Focused-overlay selector: store-less Radix overlays (dropdown / context menu,
+ * dialog, alert, and popper-based Select / combobox / listbox / popover). Escape
+ * focused inside one should close the overlay, not reset the tool.
+ * `[data-radix-popper-content-wrapper]` is the generic wrapper Radix renders
+ * around all popper content.
+ */
+const OVERLAY_ROLE_SELECTOR =
+  "[role='menu'],[role='dialog'],[role='alertdialog'],[role='listbox'],[data-radix-popper-content-wrapper]"
+
+
+/**
+ * Board-app state the Escape decision reads — a `Pick` of the real store type so
+ * the literal comparisons (`viewMode !== "board"`, `tool !== "select"`) stay
+ * checked against the store's precise unions and can't drift out of sync.
+ */
+export type EscapeAppState = Pick<
+  BoardAppState,
+  "viewMode" | "chromeDialog" | "activeNodeSurface" | "presentationMode" | "chatSheetOpen" | "tool"
+>
+
+
+/** Outcome of an Escape press on the board. */
+export type EscapeDecision = "no-op" | "defer-to-overlay" | "switch-to-select"
+
+
+/**
+ * Decide what a board Escape press does — pure, so the branch logic is unit-
+ * tested without a DOM / React mount. The caller supplies the two DOM-derived
+ * facts: `isTyping` (focus in input/textarea/contentEditable) and
+ * `insideOverlayDom` (focus inside a store-less Radix overlay).
+ *   - "switch-to-select" — consume: put an active create tool away. The caller
+ *     also aborts the in-progress draft + stopPropagation so the selection is
+ *     kept (canvas-harness couples abort + deselect into its own Escape).
+ *   - "defer-to-overlay" — an overlay owns this Escape; leave the tool alone.
+ *   - "no-op" — not Escape, modified, typing, off the board canvas, or the tool
+ *     is already `select` (the library handles the 2nd-press deselect).
+ */
+export const decideBoardEscape = (
+  e: Pick<KeyboardEvent, "key" | "metaKey" | "ctrlKey" | "altKey" | "shiftKey">,
+  app: EscapeAppState,
+  ctx: { isTyping: boolean; insideOverlayDom: boolean },
+): EscapeDecision => {
+  if (e.key !== "Escape") return "no-op"
+  if (e.metaKey || e.ctrlKey || e.altKey || e.shiftKey) return "no-op"
+  if (ctx.isTyping) return "no-op"
+  // The tool only exists on the board canvas — no-op in files / list views.
+  if (app.viewMode !== "board") return "no-op"
+  // Store-tracked overlays own the Escape (their own handlers close them).
+  // `chatSheetOpen` needs the explicit flag: the CopilotSheet is non-modal, so
+  // focus stays on the canvas and the `insideOverlayDom` DOM check misses it.
+  if (app.chromeDialog || app.activeNodeSurface || app.presentationMode || app.chatSheetOpen)
+    return "defer-to-overlay"
+  if (ctx.insideOverlayDom) return "defer-to-overlay"
+  // Tool already `select` → fall through so the library handles the deselect.
+  if (app.tool !== "select") return "switch-to-select"
+  return "no-op"
+}
+
+
 /**
  * Global keyboard bindings for the canvas-harness board. Mirrors
  * prod's `use-board-shortcuts` keymap so muscle memory carries over:
@@ -48,7 +108,8 @@ const TOOL_SHORTCUTS: Record<string, string> = {
  *  - M                            → toggle Slides panel
  *  - G                            → open Icons search dialog
  *  - I                            → open Images search dialog
- *  - Escape                       → return to select tool (unless an overlay owns it)
+ *  - Escape                       → put the active create tool away (→ select),
+ *                                   then deselect on a 2nd press (overlays win)
  *
  * Skipped when focus is in an input / textarea / contentEditable so
  * inline editing keeps the native shortcuts. canvas-harness already
@@ -121,48 +182,31 @@ export const useBoardKeyboard = (store: CanvasStore): void => {
       }
     }
 
-    // Escape returns the canvas to the `select` tool (matches tldraw/excalidraw),
-    // so a create tool (rect / note / arrow / …) isn't left stuck after one shape.
-    // Registered in the CAPTURE phase on purpose: it must read overlay state
-    // BEFORE the same Escape is consumed by a handler that clears it — Radix
-    // dialogs/menus close on a document-level capture handler (clearing
-    // `chromeDialog`), and the presentation / node-surface handlers are window
-    // bubble listeners that clear their own flags. Reading in bubble phase would
-    // race all of them. When an overlay owns the Escape we defer to its close and
-    // leave the tool untouched; canvas-harness's own Escape handler still clears
-    // the selection + aborts an in-progress drag / marquee / draft edge.
-    //
-    // "Is an overlay open?" is a hand-maintained enumeration (store flags below +
-    // the focused-overlay DOM guard). A shared open-overlay signal would be less
-    // fragile — new overlays must remember to opt in here — but that's a broader
-    // refactor; this list covers every dismissable the board mounts today.
+    // Escape implements the tldraw/excalidraw two-step cancel (see
+    // `decideBoardEscape`). Registered in the CAPTURE phase on purpose: it reads
+    // the store-tracked overlay flags, which must be read BEFORE the same Escape
+    // is consumed by a handler that clears them — Radix dialogs/menus close on a
+    // document-capture handler, and the presentation / node-surface handlers are
+    // window bubble listeners; a bubble-phase read would race all of them.
     const onEscape = (e: KeyboardEvent): void => {
+      // Fires for every keydown (capture, window-wide) — bail before the DOM walk
+      // + store read so only actual Escape presses pay for them.
       if (e.key !== "Escape") return
-      if (e.metaKey || e.ctrlKey || e.altKey || e.shiftKey) return
-      if (isTypingTarget(e.target)) return
-      const app = useBoardAppStore.getState()
-      // The tool only exists on the board canvas — no-op in files / list views.
-      if (app.viewMode !== "board") return
-      // Overlays whose open-state lives in the store own the Escape; their own
-      // handlers close them, so don't also steal the tool switch. `chatSheetOpen`
-      // needs the explicit flag because the CopilotSheet is non-modal — focus
-      // stays on the canvas, so the focused-overlay DOM guard below misses it.
-      if (
-        app.chromeDialog ||
-        app.activeNodeSurface ||
-        app.presentationMode ||
-        app.chatSheetOpen
-      )
-        return
-      // Store-less Radix overlays (view menu, context menu, delete-confirm alert)
-      // keep open-state locally — skip when the Escape is focused inside one.
       const target = e.target
-      if (
-        target instanceof HTMLElement &&
-        target.closest("[role='menu'],[role='dialog'],[role='alertdialog']")
-      )
-        return
-      if (app.tool !== "select") app.setTool("select")
+      const insideOverlayDom =
+        target instanceof HTMLElement && target.closest(OVERLAY_ROLE_SELECTOR) !== null
+      const app = useBoardAppStore.getState()
+      const decision = decideBoardEscape(e, app, {
+        isTyping: isTypingTarget(target),
+        insideOverlayDom,
+      })
+      if (decision !== "switch-to-select") return
+      // Consume: put the create tool away. canvas-harness couples abort + deselect
+      // into its own (bubble-phase) Escape, so we abort the draft ourselves and
+      // stopPropagation to suppress the lib's deselect — keeping the selection.
+      app.setTool("select")
+      store.resetInteractionState()
+      e.stopPropagation()
     }
 
     window.addEventListener("keydown", onKey)