Skip to content

refactor(shell): one SurfaceLifecycle primitive and a docked right-inspector slot #586

Description

@BorisTyshkevich

Part of the ADR-0004 vanilla-shell investment track (see docs/ADR-0004-ui-shell.md); scoped from the 2026-08-03 architecture review.

Goal

Replace six copy-pasted overlay lifecycles with one ~60-line SurfaceLifecycle helper, and give .main-row a shell-owned docked right slot. This is deliberate prep that delivers most of #488's skeleton without committing to #488's full product contract yet.

Context

.main-row (src/ui/app-shell.ts:188) currently composes only sidebar, sideHandle, queryHost, and dashboardHost — there is NO right-hand slot. Three surfaces that conceptually belong on the right side are each independently mounted position:fixed overlays on document.body:

  • cell drawer — src/ui/results.ts:1258
  • rows viewer — src/ui/results.ts (near-identical drawer, :484–530)
  • Reference pane — src/ui/doc-pane.ts:214
  • detached/fullscreen view — src/ui/detached-view.ts:153

The open/close/resize/Escape/focus lifecycle is duplicated roughly six times (~330 lines total):

  • cell drawer: results.ts:1187–1262
  • rows viewer: results.ts:484–530 (near line-for-line clone of the cell drawer)
  • doc pane: doc-pane.ts:139–217
  • detached-view: detached-view.ts:128–156
  • dialog-shell.ts:84–200 and popover.ts:100–196 — the two BEST implementations in the repo, reused by nobody on the right side

Five different Escape policies coexist:

  1. isTopDrawer DOM-scan of .cd-backdrop (results.ts:1182–1185, used at :1195)
  2. panel.contains(activeElement) (doc-pane.ts:205–212)
  3. unconditional capture (dialog-shell / popover)
  4. delegated-to-caller (detached-view)
  5. the global close-overlay shortcut (shortcuts.ts:53)

Five focus-restore patterns exist, and the cell drawer and rows viewer have NO focus restore at all.

Three places hard-code .cd-backdrop knowledge outside results.ts's own drawer builder: results.ts:1183, results.ts:1153, and a comment at app.ts:2163–2166.

drawer.ts shares only the cheap part (markup + drag) with these overlays, none of the lifecycle; schema-detail.ts:311–341 rolled its own drag handling because attachDrawerResize wasn't general enough to reuse.

Width preferences cellDrawerPx and docPanePx should collapse into one rightInspectorPx, with a compatibility read order (already specified in #488's body under "Width and migration").

Deliverables

  1. SurfaceLifecycle helper (new, ~60 lines): idempotent single-funnel close(), optional keyboard-owner acquisition, capture-phase Escape with an explicit escapePolicy: 'always' | 'focus-inside' | 'none', and a returnFocusTo contract borrowing dialog-shell.ts:41–56's element-or-resolver design (a resolver is called at close time so it can hand back whatever is on screen now, rather than a possibly-detached captured element).
  2. A docked inspectorHost plus its resize handle, added as siblings of queryHost/dashboardHost in app-shell.ts's mainRow composition (app-shell.ts:188), a new 'rightInspector' axis in splitters.ts, folded state expressed as hidden (the same pattern showHost already uses, app-shell.ts:323–327).
  3. Fold the cell/rows/reference content builders into the new slot, built on SurfaceLifecycle.
  4. Delete isTopDrawer (results.ts:1182–1185), the .cd-backdrop DOM probes (results.ts:1153, :1183), the .cd-backdrop CSS, the 'docPane' splitter branch, and drawer.ts's stateKey plumbing that only existed to support the old per-surface persistence.

Caveat — do not delete, rewrite: the header comments of drawer.ts, doc-pane.ts:1–33, and results.ts:481–482 document the current three-surface split as a deliberate architectural decision from #60/#101/#166/#313. This issue reverses that decision, so those comment blocks need to be rewritten to explain the new shared-lifecycle model, not silently deleted — a future reader must not conclude the split is still intentional.

Expected net: ~250–300 lines removed, ~120 added. Escape policies 5 → 1. Focus-restore patterns 5 → 2 (SurfaceLifecycle's, and whatever dialog-shell.ts/popover.ts keep for their own reasons if not yet unified).

Tests

  • SurfaceLifecycle unit tests at 100/95/90/100: idempotent close (double-close doesn't double-fire teardown or throw), each escapePolicy value in isolation, returnFocusTo element vs. resolver vs. null, keyboard-owner acquisition/release.
  • rightInspectorPx read/write: compatibility read order across cellDrawerPxdocPanePxrightInspectorPx, single canonical write.
  • Fold state renders hidden and consumes no layout width; open state reduces centre width (mirrors showHost coverage).
  • Regression coverage that cell drawer, rows viewer, and Reference each still open/close/resize/Escape/focus-restore correctly once rebuilt on the shared primitive.
  • Existing results.ts, doc-pane.ts, detached-view.ts test files updated in place rather than duplicated.

Acceptance criteria

  1. One SurfaceLifecycle helper exists and is the sole implementation backing cell drawer, rows viewer, and Reference pane lifecycle.
  2. .main-row has a real inspectorHost slot as a layout sibling of queryHost/dashboardHost, not a position:fixed overlay.
  3. isTopDrawer, the .cd-backdrop probes/CSS, the 'docPane' splitter branch, and drawer.ts's stateKey plumbing are deleted.
  4. cellDrawerPx/docPanePx collapse into rightInspectorPx with a documented compatibility read order and no double-write.
  5. Escape policy count is 1 (or an explicitly justified small number backed by SurfaceLifecycle's escapePolicy parameter), down from 5.
  6. drawer.ts, doc-pane.ts, results.ts header comments describing the old three-surface split are rewritten to reflect the shared-lifecycle model.
  7. npm test (coverage gate) and tsc --noEmit pass.

Non-goals

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorRestructuring without user-facing behavior changetech-debt

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions