render: dispatch InputSubmitted and ChoiceSelected with Source; re-ground ChoiceSelected to Values []string - #53
Merged
Conversation
Resolve the two standing TODO(a2tea) markers in event/event.go:
- InputSubmitted: Enter on a focused TextField (or DateTimeInput, which
shares the TextField edit path) now emits event.InputSubmitted with
Source{ComponentID, SurfaceID} and the field's current value — the
edited text when the user has typed, else the literal / resolved
binding seed, so display placeholders never leak. Enter on buttons,
modals, and checkboxes is unchanged.
- ChoiceSelected: re-grounded to Values []string per the package doc
(ChoicePicker is natively multi-value) before wiring, and emitted from
togglePickerOption whenever the normalized selection set changes. A
radio-semantics Space on an already-selected single-select option
emits nothing.
Update the event package doc to drop the not-emitted-yet caveats, the
render package/Update docs to describe the new dispatches, and drop the
events line from the README and docs/wire-format.md "not yet" lists.
Fixes #43
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
joestump
commented
Jul 18, 2026
joestump
left a comment
Collaborator
Author
There was a problem hiding this comment.
Adversarial review of the diff against issue #43. Gate re-run locally: go build ./... && go vet ./... && go test ./... -count=1 all green.
What I verified
- State/guard safety.
submitInput(render/render.go:419) andtogglePickerOption(render/inputs.go:116) can only run when the focused component actually exists and is the right kind —focusedIsTextEditable/focusedIsChoicePickergo throughfocusedComponent, which returns a zeroComponentfor an ID an Apply merge removed, so the missing-component path is unreachable and the remaining nil-map reads (s.fieldValues[id]on a nil map) are safe Go.pickerCursorclamping still covers options shrinking across merges. - Key routing. Enter ordering (modal → button → text-editable → checkbox) inserts the new branch without stealing anything: buttons still activate, checkboxes still toggle, Space on a text field still types a space, and the ChoicePicker Space branch now correctly propagates the cmd. Traced every other key case — no regressions.
- ChoiceSelected multi-value correctness.
Valuesis[]stringin option-declaration order; the before/after compare intogglePickerOptionnormalizes the pre-toggle selection with the same option-order walk, so radio-semantics re-select genuinely emits nothing while a real change (including clearing to the empty selection, which emits a non-nil empty slice) always fires. Verified the single-select, multi-select, and undeclared-literal edge paths by hand. - InputSubmitted value shadowing. Edited text wins; unedited falls back to
editSeed, so an unresolved{binding}placeholder submits""and never leaks — matches the rendering path. - Tests assert behavior, not execution. The new tests pin Source (ComponentID + SurfaceID), payload values, option ordering, the no-event radio case (
cmd == nil), the empty-selection event, the edited-vs-literal distinction, and the no-ButtonClickedinvariant.event_test.goshape pin updated forValues []string. - Ticket compliance. Both
TODO(a2tea)markers gone, package doc drops the provisional-status warnings (grep confirms no leftover "not emitted"/"provisional" markers outside historical wire-format prose), README + docs/wire-format.md drop the events line,website/untouched per the working agreement, both events carrySourcelikeButtonClicked.
Blockers
None.
Nits
- render/render.go:414-418 —
submitInputdoc overclaims equivalence with FieldValues. The comment says Value follows "the same shadowing rule as rendering and FieldValues", butgatherFieldValuesskips bindings entirely (resolveDynamicStringreturns nil for non-literals) whilesubmitInputresolves them viaeditSeedagainsts.data. On a surface with an applied data model, Enter on an unedited bound field emitsInputSubmitted.Value = <resolved value>whileFieldValues()omits that key. The event behavior is arguably the more correct of the two (it matches rendering), so no code change requested — but the comment should say "same rule as rendering" and drop the FieldValues claim, or FieldValues should learn the same resolution in a follow-up. - README.md:104-105 — internal contradiction left in a touched file. The "not yet" list still says
CheckBox/ChoicePicker/Slider/DateTimeInput"remain read-only visuals" three lines below a blurb (edited by this PR) describing ChoicePicker emittingChoiceSelectedon selection change. The PR body defers this to the docs pass, which is defensible, but it's a one-line fix in a file this diff already edits. - render/inputs.go:149 — silent normalization without an event. When a picker's literal names undeclared options (e.g. literal
["x","a"], Space on already-selecteda),choiceValuesis rewritten to the normalized["a"]— soFieldValues()changes — whilebefore == outsuppresses the event. The comment documents this as deliberate ("dropped by normalization, not by the user's toggle") and it only bites on producer-error input, so noting for the record only.
Verdict: approve. The nits are documentation-precision issues; the dispatch logic, payload shapes, and tests are correct.
🤖 Posted on behalf of @joestump by Claude.
joestump
added a commit
that referenced
this pull request
Jul 18, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Resolves the two standing
TODO(a2tea)markers inevent/event.go(part of epic #18):InputSubmittedandChoiceSelectedare now actually dispatched by the renderer, both carryingSource{ComponentID, SurfaceID}likeButtonClicked.How
InputSubmitted— Enter on a focused TextField (or DateTimeInput, which shares the TextField edit path) emitsevent.InputSubmitted{Source, ID, Value}via a newSurface.submitInputcmd. Value follows the same shadowing rule as rendering/FieldValues: the edited text when the user has typed, else the literal / resolved-binding seed (editSeed), so an unresolved{binding}placeholder submits""and never leaks. Enter on buttons, modals, and checkboxes is untouched.ChoiceSelected— re-grounded toValues []stringper the package doc's re-grounding note before wiring (ChoicePicker is natively multi-value; changing an emitted type later is worse).togglePickerOptionnow returns atea.Cmd: it compares the normalized (option-declaration-order) selection before and after the toggle and emits the full post-toggle selection only when the set changed — a radio-semantics Space on an already-selected single-select option emits nothing.eventdrops the "not emitted yet / provisional shape" caveats;render's package andUpdatedocs describe the new dispatches. The README package blurb and the README +docs/wire-format.md"not yet" lists drop the events line.Tests
render/textfield_test.go: Enter on an unedited field submits the literal with Source set and noButtonClicked; Enter after typing submits the edited value; Enter on an unresolved bound field submits""(no placeholder leak).render/inputs_test.go: Enter on DateTimeInput emitsInputSubmitted; single-select Space emitsChoiceSelected{Values: [b]}and re-selecting emits nothing; multi-select toggles emit the full selection in option order, down to the empty selection.event/event_test.go: shape pin updated forValues []string.go build ./... && go vet ./... && go test ./...all green.Notes
website/docs are deliberately untouched — deferred to the post-merge docs pass, per the working agreement. The stale "editing beyond TextField is read-only" lines in README/docs/wire-format.md(obsoleted by Editable CheckBox / ChoicePicker / Slider / DateTimeInput (finish #15) #42) are likewise left for that pass; only the events line this ticket owns was dropped.Fixes #43
🤖 Posted on behalf of
@joestumpby Claude.