mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
* fix(local): subchat mouse selection + hidden plan panel + scroll-extend selection Three bugs in the subagent/swarm drill-in (subchat) view and mouse text selection, all from the same root: newer features (subchat's row-source swap, wheel-scroll) were never integrated into the older selection/footer code. 1) Can't copy anything in the subchat view. transcriptLineAtMouse (and selectedTranscriptText, the copy-time text extraction) always hit-tested against m.transcript (the parent), never m.subchat.childRows, even while the screen was showing the child session. New transcriptHitTestSource() mirrors transcriptView's own branching (nav bar + child rows + full width vs pinned title bar + parent rows + sidebar-aware width) and is now the single source both functions use. 2) Plan panel shown above the composer inside the subchat view. footerView unconditionally rendered the PARENT run's pinned plan panel; only pendingAskUser was gated. Added a !m.subchat.active guard (m.plan belongs to the parent run, not whatever subagent/swarm session is being viewed). 3) Selection didn't extend across a scroll (reported as "stopped at the end of the screen"). transcriptSelection.cursor was only ever updated by a mouseMotion event; a wheel event is a distinct message type that never reached that code, so scrolling while selecting left the selection frozen at whatever line the mouse last physically moved over. New scrollChatExtendingSelection scrolls AND re-evaluates the cursor at the mouse's current position against the post-scroll viewport. Uses a new nearestTranscriptLineAtMouse (falls back to the closest visible selectable line) rather than an exact match, since the recomputed position can land exactly on a blank spacer row between messages after an odd-length scroll shifts text/spacer parity - an exact-match miss there must not silently no-op the extension. Tests: subchat selection resolves the CHILD session's text, not the parent's; the plan panel is absent from the footer during subchat; a selection's cursor bodyY moves (extends) after a wheel-scroll instead of staying pinned. Full gate green (73 pkg, -race). * feat(local): highlight clickable rows on mouse hover Moving the cursor over an interactive row now highlights it (accent underline) before you click, across every clickable surface: - specialist cards and collapse/expand toggle headers in the transcript - AGENTS sidebar rows (swarm members) - PLAN sidebar step rows - permission-prompt options (reuses the EXISTING keyboard-cursor highlight by moving pendingPermission.cursor on hover, instead of new styling) Requires switching the mouse-tracking mode from CellMotion (only reports motion while a button is held) to AllMotion (reports idle cursor movement too) - the existing 15ms mouse-event throttle already covers the added motion events. hover.go: hoverTarget + mouseHover predicate + updateHoverTarget, which reuses the SAME hit-testers and priority order as the existing click handlers so hover and click never disagree about what's clickable. Sidebar hover is resolved by STABLE IDENTITY (sessionID / stepIndex), not a cached line offset: hoveredSidebarLineOffset re-matches the identity against a freshly computed hit list on every render, so a row that's disappeared (a swarm member's linger elapsed, a plan step completed) simply doesn't highlight rather than a coincidentally-matching unrelated row lighting up. Transcript hover stays bodyY-keyed (matching the existing selection-highlight convention) with explicit clearHover() calls wherever bodyY numbering can shift without an intervening mouse motion: wheel-scroll, PgUp/PgDn, window resize, and transcript rebuilds (/clear, /resume, /rewind, /compact via resetFlushFrontier). Adversarial review caught two real bugs before ship, both fixed: - BLOCKER: plan-step hover stored the step's own index (0,1,2...) but the sidebar render treated it as a raw line offset into the rendered rows - every plan-step hover highlighted the wrong row (the AGENTS header/body). Fixed by resolving fresh at render time via the stable-identity redesign above, which also closes the related sidebar-drift issue. - MAJOR: hover wasn't cleared on keyboard PgUp/PgDn scroll. Tests: predicate correctness; hover resolves the right target across all 4 surfaces (specialist card, toggle row, sidebar agent, plan step, permission option) with the correct priority order; hover changes the actual render output (transcript + sidebar); hover clears on wheel-scroll and subchat exit; a regression test proving hoveredSidebarLineOffset self-heals (no highlight) when a hovered row's identity no longer resolves - the exact scenario the blocker fix targets. Full gate green (73 pkg, -race). * fix(local): soften hover color + auto-scroll a drag past the transcript edge 1) Hover highlight was brand-lime (bright, glaring against the near-black surface) and re-paints on every mouse movement. Switched to amber (an existing semantic token, "light orange") and dropped the underline per feedback. 2) Dragging a text selection past the top/bottom edge of the visible transcript now auto-scrolls toward that side and extends the selection to follow, matching the browser/editor "drag past the viewport edge" pattern. transcriptEdgeScrollDelta detects the drag is vertically outside the body rect (but still within its horizontal span, so it isn't just "off to the side" over the sidebar); dragToEdgeScroll scrolls one step and anchors the cursor to whichever line now sits at that edge of the NEW viewport - deliberately not a re-resolve of the drag's physical mouse position, which stays outside the body rect after scrolling (scrolling changes what content occupies a screen row, not the mouse's screen position) and would keep finding nothing. Extracted nearestTranscriptSelectableAt as the shared "closest selectable line to a target bodyY" core, now used by both nearestTranscriptLineAtMouse (target derived from a mouse position that may have landed on a spacer row) and dragToEdgeScroll (target derived directly from the viewport bound, no mouse position involved at all). Tests: dragging past the top edge scrolls toward older content and extends the cursor upward; the symmetric bottom-edge case scrolls toward newer content and extends downward. Full gate green (73 pkg, -race). * fix(local): smooth-glide the drag-to-edge auto-scroll instead of jumping The initial edge-scroll implementation (previous commit) applied a single chatWheelScrollLines (5-line) jump per raw mouse-motion event, which reads as sudden/jerky since motion events during a real drag arrive in bursts, and stops dead the instant the physical mouse holds still even while still parked past the edge (a terminal only reports motion on actual cursor movement). Replaced with a self-scheduling tea.Tick chain (dragEdgeScrollTickMsg, gated by edgeScrollSeq mirroring the existing exitConfirmSeq/copyStatusSeq idiom) that steps a small, fixed increment (dragEdgeScrollStep=1 line) on a short cadence (dragEdgeScrollInterval=70ms) for as long as the drag holds past the edge, independent of whether new raw motion events keep arriving. reducedMotion still gets the original single-bigger-step-per-event behavior (no animated glide). Adversarial review of the new self-perpetuating tick chain (the risk being an orphaned/stuck chain) found one real bug: a keypress mid-drag clears m.transcriptSelection.active directly (the KeyPressMsg handler in model.go) without going through stopEdgeScroll, leaving edgeScrollDelta stale. The in-flight tick itself terminates safely via its own liveness check - the bug was that the STALE nonzero edgeScrollDelta then fooled startEdgeScroll's "already running" fast path on the NEXT same-direction drag into silently doing nothing (no immediate step, no scheduled tick) for that whole hold. Fixed at the press site (transcript_selection.go's mouseLeftPress case): every fresh press now resets glide state via stopEdgeScroll, so a new drag always starts clean regardless of how the previous one ended - a single targeted fix rather than chasing every place transcriptSelection can be cleared. Tests: top/bottom edge glide starts and extends the cursor over several ticks (a single small step can land on a blank spacer row between messages, same as real usage); the chain stops when the drag returns to the body and a stale in-flight tick after that is a safe no-op; the keypress-interrupt regression (reproduces the adversarial review's finding, confirms the fix). Full gate green (73 pkg, -race). * fix(local): soften the brand-lime accent color across the whole UI The accent color (#caff3f) was fully saturated at fairly high lightness (HSL: L 62%, S 100%) - the kind of value that reads as neon/glaring everywhere it filled a block or bold-text surface, not just the one spot originally flagged: the composer's "typed" prompt glyph, the permission popup's focused-option badge, and the text-selection highlight all use it, plus the spinner, the brand chip, tool-card prompts, and more. Rather than patch each of those consumers individually, softened the single palette source (dark theme only; the light theme's accent is a separate, already-darkened value for its own contrast needs, untouched) to L 50% / S 80% (#ade619) - still clearly on-brand lime, and contrast against both black (onAccent text on accent fills) and the near-black panel stays far above WCAG AA (~14:1 and ~13:1, both comfortably over the 4.5:1 floor). Updated TestStreamingFadePaletteMonotonic's pinned RGB assertion (it checked the fade ramp's first bucket equals the literal old accent bytes) to the new values. Full gate green (73 pkg, -race; make lint/go vet clean except an unrelated stray file that isn't part of this branch or git history at all). * fix(local): darken the brand-lime accent further per live feedback Two prior passes (#ade619 at L50/S80, #8db620 at L42/S70) were each tested live against the original #caff3f via raw truecolor escape codes and confirmed too subtle in turn. Settled at L36/S62 (#759523) - a noticeably darker, more olive lime, confirmed against the original side by side. Contrast vs black (onAccent) and the near-black panel stays comfortably above WCAG AA (~6.1:1 and ~5.6:1). Updated the streaming-fade test's pinned RGB assertion to match. * fix: address PR #350 review comments and CI smoke test failures Smoke tests (macOS + Ubuntu): the 4 new edge-glide tests implicitly relied on m.reducedMotion being false, but mouseTestModel() inherits it from defaultReducedMotion(), which forces true whenever colorprofile.NoTTY is detected - true in CI's non-TTY test runners, false in a local interactive dev shell. Set reducedMotion = false explicitly in each of the 4 tests so they're deterministic regardless of environment. Verified by re-running locally under ZERO_REDUCED_MOTION=1 (reproduces the CI condition) - passes. CodeRabbit actionable comment: handleTranscriptSelectionMouse's mouseMotion case gated only on transcriptSelection.active, which stays true through the async copy-command grace window after release (a non-empty selection isn't reset until transcriptCopiedMsg lands) - so a genuine hover motion arriving in that window would be misrouted as a drag update. The suggested fix (gate on msg's own Button field) turned out to be wrong in a way only a full test run caught: TestTranscriptSelectionUpdatesOnGenericMotion is a pre-existing, deliberate test - some terminals don't restate the button on every motion event during a real drag, so trusting the per-event Button field breaks those terminals. Fixed properly instead: added a `dragging` field to transcriptSelectionState, tracking the app's own press/release bracket (true at press, false at release) independent of active and independent of any single event's Button field. mouseMotion now gates on dragging. CodeRabbit nitpicks: routed the two remaining raw `m.hover = hoverTarget{}` subchat-exit assignments through the existing clearHover() helper, so hover invalidation stays in one place. Left the third nitpick (hit-test layout recomputed per hover-motion tick) as a documented follow-up per the reviewer's own "trivial/poor tradeoff" framing - a proper per-frame memoized cache needs correct invalidation on scroll/resize/content-change to avoid reintroducing the exact staleness class of bug already fixed twice in this branch, and isn't worth rushing. New tests: TestTranscriptHoverAfterReleaseDoesNotMoveSelection (the actual bug CodeRabbit found - a post-release hover must not alter the just-copied selection). Full gate green (73 pkg, -race); TestTranscriptSelectionUpdatesOnGenericMotion (the test the first fix attempt broke) verified still passing.
118 lines
4.8 KiB
Go
118 lines
4.8 KiB
Go
package tui
|
|
|
|
import tea "charm.land/bubbletea/v2"
|
|
|
|
// hoverKind discriminates which clickable surface a hoverTarget refers to.
|
|
type hoverKind int
|
|
|
|
const (
|
|
hoverNone hoverKind = iota
|
|
// hoverTranscript: a specialist card or collapse/expand toggle header in the
|
|
// main transcript body (or the subchat child view), identified by bodyY.
|
|
hoverTranscript
|
|
// hoverSidebarAgent: a clickable AGENTS sidebar row, identified by sessionID.
|
|
hoverSidebarAgent
|
|
// hoverPlanStep: a plan step row in the sidebar's PLAN section, identified by
|
|
// step index.
|
|
hoverPlanStep
|
|
)
|
|
|
|
// hoverTarget identifies the single clickable row (if any) currently under the
|
|
// mouse cursor with no button pressed. Exactly one of bodyY/sessionID/stepIndex is
|
|
// meaningful, discriminated by kind.
|
|
//
|
|
// The sidebar fields deliberately store a STABLE IDENTITY (sessionID, stepIndex),
|
|
// not a raw line offset: the AGENTS/PLAN section sizes can change between when a
|
|
// hover is detected and when the sidebar next renders (a swarm member's linger
|
|
// window elapsing, a plan step completing) with no mouse motion in between to
|
|
// re-resolve it. A cached raw offset would then silently point at whatever
|
|
// unrelated row now occupies that slot. hoveredSidebarLineOffset (sidebar.go)
|
|
// re-resolves the CURRENT line offset for this identity fresh on every render, so
|
|
// a row that's gone simply doesn't highlight rather than a wrong one lighting up.
|
|
type hoverTarget struct {
|
|
kind hoverKind
|
|
bodyY int // hoverTranscript
|
|
sessionID string // hoverSidebarAgent
|
|
stepIndex int // hoverPlanStep
|
|
}
|
|
|
|
// mouseHover reports whether msg is a plain cursor-movement event with NO button
|
|
// pressed — a hover, not a drag. Requires AllMotion mouse reporting (see View's
|
|
// wantsMouseCapture branch); CellMotion only reports motion while a button is held,
|
|
// so this predicate would never match under the old mouse mode.
|
|
func mouseHover(msg tea.MouseMsg) bool {
|
|
if _, ok := msg.(tea.MouseMotionMsg); !ok {
|
|
return false
|
|
}
|
|
return mouseEvent(msg).Button == tea.MouseNone
|
|
}
|
|
|
|
// updateHoverTarget resolves what's under the cursor for a hover motion, using
|
|
// the SAME hit-testers and priority order as the click handlers in
|
|
// handleTranscriptSelectionMouse's press case: a sidebar row (agent, then plan
|
|
// step) takes priority since it's outside the chat column, then a clickable
|
|
// transcript line. Clears the hover when nothing clickable is under the cursor.
|
|
func (m model) updateHoverTarget(msg tea.MouseMsg) model {
|
|
if hit, ok := m.sidebarLineAtMouse(msg); ok {
|
|
return m.withHover(hoverTarget{kind: hoverSidebarAgent, sessionID: hit.sessionID})
|
|
}
|
|
if stepIndex, ok := m.planStepAtMouse(msg); ok {
|
|
return m.withHover(hoverTarget{kind: hoverPlanStep, stepIndex: stepIndex})
|
|
}
|
|
if line, ok := m.transcriptLineAtMouse(msg); ok {
|
|
// A permission option reuses its OWN existing keyboard-cursor highlight
|
|
// (see hoverPermissionOption) rather than the m.hover mechanism, so there's
|
|
// nothing further to set here.
|
|
if line.permOption {
|
|
return m.hoverPermissionOption(line.permChoice).withHover(hoverTarget{})
|
|
}
|
|
// Only a card or a collapse/expand toggle header is "clickable" here; plain
|
|
// selectable text (e.g. a user/assistant message) isn't, so hovering over
|
|
// it must not light up as if it were.
|
|
if line.specialistCard || line.toggle {
|
|
return m.withHover(hoverTarget{kind: hoverTranscript, bodyY: line.bodyY})
|
|
}
|
|
}
|
|
return m.withHover(hoverTarget{})
|
|
}
|
|
|
|
// hoverPermissionOption moves the pending permission prompt's cursor to the
|
|
// hovered option, reusing its EXISTING keyboard-navigation highlight (the same
|
|
// one Tab/Shift+Tab/arrow keys move) instead of new styling — the popup already
|
|
// renders whichever option .cursor points at differently. A no-op when no
|
|
// permission prompt is pending or the choice isn't found (defensive; every
|
|
// permOption line's choice always comes from permissionOptions(request) in the
|
|
// first place).
|
|
func (m model) hoverPermissionOption(choice permissionDecision) model {
|
|
if m.pendingPermission == nil {
|
|
return m
|
|
}
|
|
for index, option := range permissionOptions(m.pendingPermission.request) {
|
|
if option.choice == choice {
|
|
m.pendingPermission.cursor = index
|
|
return m
|
|
}
|
|
}
|
|
return m
|
|
}
|
|
|
|
func (m model) withHover(t hoverTarget) model {
|
|
m.hover = t
|
|
return m
|
|
}
|
|
|
|
// clearHover drops any hover highlight. Called wherever the underlying content a
|
|
// hover target refers to may no longer mean the same thing on the next render — a
|
|
// wheel-scroll (the cursor's mapping to a bodyY is only recomputed on the NEXT
|
|
// motion event; without clearing, a stale bodyY could coincidentally match a
|
|
// DIFFERENT row after the viewport shifts, highlighting the wrong thing) or a
|
|
// subchat toggle (bodyY numbering is entirely different between the parent
|
|
// transcript and a child session).
|
|
func (m model) clearHover() model {
|
|
if m.hover.kind == hoverNone {
|
|
return m
|
|
}
|
|
m.hover = hoverTarget{}
|
|
return m
|
|
}
|