Skip to content

Unify the floating-surface UI layer: one dismiss/focus/layer system + shared surface primitives #512

Closed

Description

@Julius-o1

Status (quality-loop audit, 2026-09-25)

Landed: Most of the modal half landed in the UI Component Rewrite (#529): the shared dialog.tsx is used by 46 files, plus one interaction-ownership marker.

Remaining: There is no named layer scale (z-[1100] is hardcoded in several places), no popover or side-panel primitive, and CaffeinateToastSurface has its own toast.

Acceptance: Named layers, popover and side-panel primitives, and toast consolidation. Parts overlap the keyboard-first PR #1221; coordinate with it.

Original report

Problem

Every modal, sheet, panel, popover, and toast in the app is a hand-rolled one-off. There is no component-based system: 20 independent implementations of "fixed inset-0 + tint + centered panel", 8 copy-pasted side-panel shells, 3 hand-rolled anchored popovers, and 2 toast systems that bypass the one shared toast that exists. The result is exactly the un-scalable mess you'd expect — behavior (especially quick-escape) differs surface by surface, and every new feature re-invents the shell.

We do NOT want a styled component library, and we do NOT want a bespoke invented-here design system. We want a small primitive set that implements the standard behaviors (the W3C WAI-ARIA APG dialog/menu patterns: Escape dismisses topmost, focus trap, focus restore, layered stacking) so surfaces stop owning those semantics individually.

Full audit (2026-07-09, one-agent sweep of every floating surface, post-#505): evidence below is condensed from it; line numbers from main @ ec7dedf-era tree.

Evidence — the inconsistency matrix

Escape: 6 distinct mechanisms + dead zones.

  1. Global capture in workspace/tile-tree/useKeybinds.ts:189-203 — but only for a hardcoded 3-surface list (placement, reorder, pin) + spotlight/reader/settings/bury branches.
  2. React onKeyDown on an inner tabIndex={-1} div (Reorder, Pin, ViewPrompts, Rewind, AgentViewModePicker, AgentActivity) — breaks the moment focus leaves that div.
  3. onKeyDown on the input/textarea (CommandPalette, PathInput, BuryPane, DebugBundleNote, TiledDispatchCount).
  4. onKeyDown on the backdrop element (CloseOldAgents, BulkProviderSwitch).
  5. window/document listeners while open (UsageModal, CustomAppearanceModal, AppearanceMenu, NewAgentPlacementOverlay — the last with capture+stopPropagation).
  6. Deliberately forwarded to the PTY (AskUserQuestionRow, features/feed/ui/semantic/AskUserQuestionRow.tsx:318) — any global stack must NOT break this.

Escape-dead surfaces: TileTabsModal, RemotePanel, PerformancePanel, SystemPerfPopover, all 8 side panels.
Double-handled (local + global, double-fire hazard on partial migration): BuryPane, Reorder, Pin, placement overlay.
Live bug: Esc inside CustomAppearanceModal also closes the entire Settings page (useKeybinds.ts:348 capture fires first — SettingsPage.tsx:127-133).

Global shortcuts leak under modals. useKeybinds bails out for exactly 3 surfaces; with the palette, UsageModal, PromptSearch, etc. open, ⌘W still closes a pane, ⌘T opens the path picker, alt-D splits — the same class of bug the useKeybinds.ts:122-128 comment says was fixed for those 3.

Backdrop: four tints (black/30, black/40, black/50, canvas/80+blur; palette is fully transparent) and four click-close idioms (mousedown target check ×13; onMouseDown+stopPropagation ×1; onClick ×2 — which has the select-text-drag-release-outside-closes bug, incl. the CommandPalette; a full-inset <button> ×1).

Focus: no focus trap anywhere (Tab walks out of every modal into the app), no focus restore anywhere, autofocus via 4 idioms (rAF ref ×9, autoFocus ×1, effect ×2, none ×5).

Z-layers: accidental 4-tier scale — z-40 (dictation, placement), z-50 (palette, remote, setup, popovers, toasts), z-[1000] (4 modals), z-[1100] (9 modals). The CommandPalette paints UNDER the modals it launches; three surfaces depend on DOM sibling order instead (documented in app/surfaces/registry.tsx:44-59); the registry's own z comment is already stale.

Chrome: ~12 distinct widths, 5 max-heights, two header patterns, three confirm-button styles, and cancel is variously Cancel/Skip/Close/close/an Esc chip/×. Only 2 of 20 shells use the .modal-fade/.modal-pop animations that styles.css:761-769 provides for everyone.

Misc: RemotePanel is registered in sidePanelSurfaces but renders a centered modal (wrong group); CaffeinateToast + PaneToast bypass ui/GlobalToast.tsx; the black/30 z-[1100] family alone is 7 near-identical copies.

What already exists to build on (don't reinvent)

  • refactor: App.tsx god render-root → thin composition root + surface registry (#494) #505's app/surfaces/registry.tsx — the mounting/ordering layer is DONE; this issue is about what the registered wrappers render.
  • useKeybinds — already the de-facto Escape/shortcut router with a working modal-bailout pattern (:189-203); it just needs a real anySurfaceOpen() predicate instead of a hardcoded 3-surface list.
  • ui/GlobalToast.tsx, ui/PathInput.tsx, .modal-fade/.modal-pop/.toast-enter CSS, useResizableSplitter, SessionPreviewPane.

Design — five primitives, standards-anchored

The behaviors we standardize are the WAI-ARIA APG patterns, not inventions:

  1. ui/layers.ts — a named z-scale (panel / overlay / palette / modal / prompt / toast). Kills magic z-[1100]s; registry order stops being load-bearing for stacking.
  2. useSurfaceDismiss(ref, { onDismiss, escape?, outsideClick? }) + a module-level dismiss stack — the keystone. Each open surface registers; ONE document capture keydown pops only the TOPMOST on Escape; one outside-mousedown implementation (target check, the correct idiom of the four). Must support: escape: false while busy (BulkProviderSwitch), two-phase dismissal (PathInput dropdown → modal; palette sub-modes; reorder draft-cancel), and events already consumed by AskUserQuestionRow. Exposes anySurfaceOpen() so useKeybinds' bailout generalizes to every modal.
  3. <ModalScrim> — one standard tint + modal-fade, layer + align: center | top props, wired to the dismiss hook.
  4. <ModalPanel> — bg-surface border border-border-hi + modal-pop, width presets (sm/md/lg/xl), max-h-[82vh], optional standard <ModalHeader>/<ModalFooter>, built-in initial focus + focus restore on close + a simple Tab-loop trap.
  5. <AnchoredPopover> — for AppearanceMenu / PerformancePanel / SystemPerfPopover / PathInput dropdown; same dismiss hook. Side panels get one <SidePanelShell> (8 copies today). CaffeinateToast/PaneToast route through the existing GlobalToast.

The library question, answered explicitly: the standard alternative is headless primitives (Radix/Base UI) with vendored thin shells (shadcn pattern) — standards-compliant behavior we don't own, styling we do. It remains the fallback if the focus-trap/dismiss-stack proves fiddlier than expected, and the five shells above are shaped so Base UI could slot inside them without changing call sites. But the recommendation is the in-house five: the app already owns the two genuinely hard parts (#505's mount/order registry and useKeybinds' capture routing), portals — which Radix's layering is built around — are exactly what the registry deliberately avoids, the dependency tree is deliberately lean, and five small files implementing a W3C spec is not an invented design system.

Migration plan (multi-PR; each behavior-preserving except the named bug fixes)

  1. PR-1 — primitives + dismiss stack, migrating only the 3 surfaces useKeybinds already bails out for (Reorder, Pin, placement) so the double-handling is retired in the same change, plus generalize the useKeybinds bailout to anySurfaceOpen(). This alone fixes the shortcut-leak class.
  2. PR-2 — the black/30 z-[1100] family (7 near-identical shells: BuryPane, DebugBundleNote, ViewPrompts, Rewind, AgentActivity, CloseOldAgents, BulkProviderSwitch) + UsageModal + TileTabsModal + AgentViewModePicker + RemotePanel (re-group + give it Escape) + CustomAppearanceModal (fixes the Esc-nukes-Settings bug).
  3. PR-3 — panels/popovers/toasts: <SidePanelShell> ×8, <AnchoredPopover> ×3 (+Escape/outside-click where dead), toast consolidation.
  4. PR-4 — the risky three, individually: CommandPalette (10 modes, mode-aware Escape, keepPaletteOpen + count-prompt-above-palette contract, native-menu bridge), NewAgentPlacementOverlay (capture-phase history, PR chore: address review follow-ups from #73 / #74 / #8 #75 regression class), PathPickerModal+PathInput (two-phase Escape, entry flow). Each with explicit before/after keyboard behavior notes.

Acceptance

  • Every floating surface dismisses on Escape — topmost-only — unless explicitly documented otherwise (SetupGate, busy BulkProviderSwitch, PTY-forwarding AskUserQuestion).
  • Global shortcuts do not fire underneath ANY open modal.
  • One scrim idiom, one outside-click idiom, named layers only (grep for z-\[1[01]00\] returns nothing), focus restored on close everywhere.
  • The 20 modal shells / 8 panel shells / 3 popovers render through the shared primitives; a new modal is <20 lines of feature-specific code.
  • No new styled-component-library dependency; if headless primitives are adopted as the behavior engine, they are invisible outside ui/.

Related: #505/#494 (surface registry — the foundation), #97 (pin-tab UX — touches PinAgentsModal en route), #117 (Vim mode — wants exactly this single key-routing stack to hook into).

Activity

  1. Juliusolsson05 commented on Sep 27, 2026

    @Juliusolsson05
    Owner

    Evidence check (W2) against origin/main b7ae6e30, item by item against the Acceptance line:

    The dismiss stack was never in the Acceptance line. #1398's review b showed its wiring contract cannot work as first drawn: a document capture-phase Escape runs before the inner two-phase handlers, and effect order is not visual stacking order. It should come back only as its own issue, with a concrete Escape bug that needs it.

    Proposal: close #512 as complete. Manager/owner call because it drops the popover primitive from the original scope.

  2. Juliusolsson05 commented on Sep 27, 2026

    @Juliusolsson05
    Owner

    Closing (owner decision, 2026-09-27): done by #1398. Named layers live in ui/layers.ts, there's a shared components/ui/side-panel.tsx, the old hard-coded stacking numbers remain only in comments, and toasts go through one GlobalToast (#1221). Accepted: no shared popover component; the three remaining popovers share no common shape. Reopen when: a new floating surface needs its own stacking or Escape rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    sev:P3Minortype:uxUI/UX polish or consistency

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions