render: open Modal content on trigger activation - #51
Conversation
A Modal is now interactive instead of a trigger-only placeholder. The modal joins the focus ring as a single element drawn as its trigger (behind a ▹/▸ cue glyph); Enter toggles it open and Esc closes the most recently opened modal, with nested modals tracked as a stack. Open content renders as a bordered in-flow block below the trigger — the honest terminal equivalent of an overlay — and its focusables join the ring only while open. Open state lives beside fieldValues as user-interaction state: it survives Apply merges (pruned only when the component stops being a modal) and is cleared by deleteSurface. Standalone gains a HasOpenModal probe, mirroring EditingText, so an open modal consumes Esc before the esc-quit fires; Ctrl+C stays an unconditional quit. Fixes #46 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
joestump
left a comment
There was a problem hiding this comment.
Adversarial review against the #46 contract. Verified locally: go build ./... && go vet ./... && go test ./... green on the PR head (1a4a96c).
What I verified
- Ticket Done-when: trigger opens content (Enter), Esc closes, Standalone still quits when no modal is open, open state preserved across
Apply, docs "Not yet" lists updated (README +docs/wire-format.md;website/correctly deferred). - State preservation:
applyComponentsprunes before re-collecting focusables (render/composite.go:80-82), so a replaced modal can't leak content focusables into the ring;deleteSurfaceclearsopenModals; focus preservation across merges works for modals since it keys off the genericfocusables[focusIdx]. - Key routing: Enter checks
focusedIsModalbeforefocusedIsButton(render/render.go:245), so a focused TextField or button inside open content behaves exactly as before; Esc has no text-editing semantics to steal;q-while-editing and unconditional Ctrl+C both covered by tests. - Guards:
focusedComponentzero-value on empty ring,closeTopModalempty-stack no-op, missing trigger/content IDs fall through to the existing[a2tea: missing component]path, cycle guard covers modal content in both render and focus walks. - Tests assert behavior (rendered content, ring composition, focus restore, quit commands), not just execution. Good coverage.
Blockers
None.
Nits
-
Phantom nested-modal open state (
render/modal.gotoggleModal/HasOpenModal). Enter-toggling an outer modal closed while an inner modal (inside its content) is still open leaves the inner ID inopenModals. Verified with a runtime probe:HasOpenModal()returnstruewhile the view shows no open modal, so in Standalone the next Esc is silently swallowed (it pops the invisible inner entry) and the user needs a second Esc to quit.pruneOpenModalsdoesn't catch this — the inner component is still a modal, just unreachable. Suggested fix: after any close/toggle, drop open entries not reachable from the root through open content (a reachability variant ofpruneOpenModals), or makeHasOpenModalcount only reachable entries. Edge case (nested modals + Enter-toggle of the outer instead of Esc), self-corrects on the extra Esc — not blocking. -
Trigger cue overflows the width budget by 2 cells (
render/containers.go:131). The trigger child renders under the fulls.widthbudget and then gets the 2-cell"▹ "prefix, so an exact-width trigger line lands at width+2; continuation lines of a multi-line trigger also aren't indented under the cue. TextField wrapscue+valuetos.width(render/fields.go:72) — mirroring that (render trigger unders.width-2, indent continuations likebullet) would keep the convention consistent. Cosmetic. -
Stale comment (
render/composite.go:71): "Remember which button held focus" — the ring now also holds text fields and modals.
Approving direction: nit 1 is worth a follow-up ticket if not fixed here; nits 2-3 are take-or-leave.
🤖 Posted on behalf of @joestump by Claude.
Union merge of interactive tab switching (#45) with the five feature PRs that landed on main since the branch forked (#48 createSurface no-op, #49 ChildList templates, #51 Modal open/close, #52 editable inputs, #53 InputSubmitted/ChoiceSelected dispatch). Nothing from either side was dropped: - render/render.go: Surface keeps ALL state — activeTabs (branch) plus fieldValues/checkValues/choiceValues/sliderValues/choiceCursor/ openModals/scope (main). isInteractive now also covers non-empty Tabs bars. collectFocusables walks only the ACTIVE tab's subtree, so inputs and modals inside inactive tabs are excluded from the focus ring, and only an open modal's content joins it. Key routing: left/right step a focused slider and switch tabs on a focused tab bar; h/l switch tabs only on the tab bar and stay literal runes on a focused TextField/DateTimeInput; enter/space/esc keep main's button/checkbox/choice/modal/InputSubmitted behavior. - render/composite.go: deleteSurface clears every state map including activeTabs; applyComponents preserves active-tab, focus, modal, and edited-value state across merges, with out-of-range tabs clamping to the first at read time. - render/tabs_test.go: two new tests pin the union — slider-focused vs tab-bar-focused left/right disambiguation, and inputs/modals inside an inactive tab excluded from the focus ring. - README.md / docs/wire-format.md: merged feature descriptions; the 'not yet' lists are now empty (issues #42–#47 all landed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Closes the "Modal renders only its trigger" gap: modals now open on
Enterand close onEsc, with the content child rendered as a bordered in-flow block (a true overlay is out of scope for a string-rendered surface — in-flow expansion is the honest terminal equivalent).How
render/render.go): a Modal joins the ring as a single focusable drawn as its trigger — the trigger subtree is the modal's chrome (like a button's label) and never joins separately. Content focusables join only while the modal is open, matching what is on screen.render/modal.go, new): open modals are tracked as a stack in open order, soEsccloses the innermost of nested modals first.toggleModal/closeTopModalre-derive the ring and keep focus on the modal (soEscreturns focus to the trigger).render/containers.go): closed = cued trigger only (▹idle /▸focused, the TextField glyph-swap convention — chrome stays monochrome). Open = trigger + content in theCardBorderbox with renderCard's width budgeting; the border is kept even in compact mode since it is the only signifier separating modal content from the flow.render/composite.go): open state survivesupdateComponents(pruned only when a component stops being a modal) and is cleared bydeleteSurface.a2tea.go): newHasOpenModalprobe mirroringEditingText— an open modal consumesEscbefore Standalone's esc-quit;Ctrl+Cremains an unconditional quit.Tests
render/modal_test.go(new): closed renders trigger-only; modal is the sole focusable (trigger buttons excluded); focus-cue glyph swap; Enter opens (bordered content, content button joins ring); Enter toggles closed; Esc closes with focus inside content and restores focus to the modal; Esc no-op with nothing open; open state preserved across Apply; open state pruned when the modal is replaced; nested modals close innermost-first.a2tea_modal_test.go(new): Standalone Esc closes the open modal without quitting, then quits once closed; Ctrl+C quits even with an open modal.go build ./... && go vet ./... && go test ./...green locally.Docs:
docs/wire-format.mdandREADME.md"Not yet" lists updated. Thewebsite/docs site is deliberately untouched — deferred to the post-merge docs pass.Fixes #46
🤖 Posted on behalf of
@joestumpby Claude.