mirror of
https://github.com/mark3labs/kit.git
synced 2026-06-14 03:30:26 +00:00
fix(ui): eliminate mouse copy-selection drift during streaming
- Lock viewport scroll while a drag-select is active so highlighted content stays under the cursor (SetItems, appendStreamingChunk, MouseWheelDown all now honor IsMouseDown). - HandleMouseDrag defensively clears autoScroll on every update so a racy re-enable can't shift the row mid-drag. - Recompute scrollback yOffset/viewport height on each mouse event via currentScrollbackBounds() instead of relying on stale values cached during the previous View() pass. - Account for canceling/ctrlCPressedOnce warning rows in distributeHeight and mark layoutDirty when those flags toggle so the height budget and mouse origin stay in sync. - Add ScrollList regression tests covering the three invariants.
This commit is contained in:
+72
-8
@@ -1266,7 +1266,11 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
|||||||
m.scrollList.autoScroll = false
|
m.scrollList.autoScroll = false
|
||||||
case tea.MouseWheelDown:
|
case tea.MouseWheelDown:
|
||||||
m.scrollList.ScrollBy(scrollLines)
|
m.scrollList.ScrollBy(scrollLines)
|
||||||
if m.scrollList.AtBottom() {
|
// Only re-enable auto-scroll when the user is not actively
|
||||||
|
// selecting text. Otherwise a wheel-down during a drag-select
|
||||||
|
// would re-arm GotoBottom on the next stream chunk, shifting
|
||||||
|
// the highlighted row out from under the cursor.
|
||||||
|
if m.scrollList.AtBottom() && !m.scrollList.IsMouseDown() {
|
||||||
m.scrollList.autoScroll = true
|
m.scrollList.autoScroll = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1274,9 +1278,14 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
|||||||
// ── Mouse click selection (crush-style character-level) ──────────────────
|
// ── Mouse click selection (crush-style character-level) ──────────────────
|
||||||
case tea.MouseClickMsg:
|
case tea.MouseClickMsg:
|
||||||
if msg.Button == tea.MouseLeft {
|
if msg.Button == tea.MouseLeft {
|
||||||
// Calculate viewport-relative coordinates.
|
// Compute the scrollback origin from the current frame's layout
|
||||||
viewY := msg.Y - m.scrollbackYOffset
|
// rather than the stale cached value from the previous View().
|
||||||
if viewY >= 0 && viewY < m.scrollList.height {
|
// scrollbackYOffset/scrollList.height are only refreshed inside
|
||||||
|
// View() and lag behind any state change that resized the header
|
||||||
|
// (extension widgets, warning rows, etc.) since the last render.
|
||||||
|
yOff, vpHeight := m.currentScrollbackBounds()
|
||||||
|
viewY := msg.Y - yOff
|
||||||
|
if viewY >= 0 && viewY < vpHeight {
|
||||||
// Clear any previous selection on a new click.
|
// Clear any previous selection on a new click.
|
||||||
// HandleMouseDown will set up new selection state.
|
// HandleMouseDown will set up new selection state.
|
||||||
if m.scrollList.HandleMouseDown(msg.X, viewY) {
|
if m.scrollList.HandleMouseDown(msg.X, viewY) {
|
||||||
@@ -1287,8 +1296,9 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
|||||||
|
|
||||||
// ── Mouse motion/drag for character-level selection ──────────────────────
|
// ── Mouse motion/drag for character-level selection ──────────────────────
|
||||||
case tea.MouseMotionMsg:
|
case tea.MouseMotionMsg:
|
||||||
viewY := msg.Y - m.scrollbackYOffset
|
yOff, vpHeight := m.currentScrollbackBounds()
|
||||||
if viewY >= 0 && viewY < m.scrollList.height {
|
viewY := msg.Y - yOff
|
||||||
|
if viewY >= 0 && viewY < vpHeight {
|
||||||
m.scrollList.HandleMouseDrag(msg.X, viewY)
|
m.scrollList.HandleMouseDrag(msg.X, viewY)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1618,10 +1628,16 @@ func (m *AppModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
|||||||
|
|
||||||
// ── Cancel timer expired ─────────────────────────────────────────────────
|
// ── Cancel timer expired ─────────────────────────────────────────────────
|
||||||
case uicore.CancelTimerExpiredMsg:
|
case uicore.CancelTimerExpiredMsg:
|
||||||
|
if m.canceling {
|
||||||
|
m.layoutDirty = true
|
||||||
|
}
|
||||||
m.canceling = false
|
m.canceling = false
|
||||||
|
|
||||||
// ── Ctrl+C reset timer expired ────────────────────────────────────────────
|
// ── Ctrl+C reset timer expired ────────────────────────────────────────────
|
||||||
case uicore.CtrlCResetMsg:
|
case uicore.CtrlCResetMsg:
|
||||||
|
if m.ctrlCPressedOnce {
|
||||||
|
m.layoutDirty = true
|
||||||
|
}
|
||||||
m.ctrlCPressedOnce = false
|
m.ctrlCPressedOnce = false
|
||||||
|
|
||||||
// ── Input submitted ──────────────────────────────────────────────────────
|
// ── Input submitted ──────────────────────────────────────────────────────
|
||||||
@@ -3763,7 +3779,12 @@ func (m *AppModel) appendStreamingChunk(role, content string) {
|
|||||||
}
|
}
|
||||||
// Auto-scroll to bottom if enabled (iteratr pattern)
|
// Auto-scroll to bottom if enabled (iteratr pattern)
|
||||||
// Don't call SetItems() - the slice reference hasn't changed
|
// Don't call SetItems() - the slice reference hasn't changed
|
||||||
if m.scrollList != nil {
|
//
|
||||||
|
// CRITICAL: never scroll the viewport while the user is actively
|
||||||
|
// selecting text (mouse button held). Doing so shifts the
|
||||||
|
// highlighted content out from under the cursor and produces the
|
||||||
|
// off-by-N-row drift users see when copy-selecting during streaming.
|
||||||
|
if m.scrollList != nil && !m.scrollList.IsMouseDown() {
|
||||||
if m.scrollList.autoScroll {
|
if m.scrollList.autoScroll {
|
||||||
m.scrollList.GotoBottom()
|
m.scrollList.GotoBottom()
|
||||||
} else if m.scrollList.AtBottom() {
|
} else if m.scrollList.AtBottom() {
|
||||||
@@ -3791,6 +3812,36 @@ func (m *AppModel) appendStreamingChunk(role, content string) {
|
|||||||
m.refreshContent()
|
m.refreshContent()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// currentScrollbackBounds returns the live (yOffset, viewportHeight) for the
|
||||||
|
// scrollback region, computed from the current state — not from the cached
|
||||||
|
// values populated inside View().
|
||||||
|
//
|
||||||
|
// scrollbackYOffset and scrollList.height are refreshed once per render, so
|
||||||
|
// any state change that resizes the header (extension widget toggles,
|
||||||
|
// warning rows, queued messages, etc.) leaves the cached values one frame
|
||||||
|
// stale. Mouse click handlers in Update() can then place the cursor on the
|
||||||
|
// wrong line, producing the off-by-N-row drift seen during copy-selection.
|
||||||
|
//
|
||||||
|
// This recomputes the header height by rendering it (cheap — the renderer
|
||||||
|
// returns "" when no extension header is set) and recomputes the viewport
|
||||||
|
// height the same way distributeHeight() does, so both inputs to the
|
||||||
|
// y → (item, line) mapping are always current.
|
||||||
|
func (m *AppModel) currentScrollbackBounds() (yOffset, viewportHeight int) {
|
||||||
|
// Force a fresh layout if anything in Update() marked the state dirty;
|
||||||
|
// otherwise scrollList.height still reflects the previous frame.
|
||||||
|
if m.layoutDirty {
|
||||||
|
m.distributeHeight()
|
||||||
|
m.layoutDirty = false
|
||||||
|
}
|
||||||
|
if headerView := m.renderHeaderFooter(m.getHeader); headerView != "" {
|
||||||
|
yOffset = lipgloss.Height(headerView)
|
||||||
|
}
|
||||||
|
if m.scrollList != nil {
|
||||||
|
viewportHeight = m.scrollList.height
|
||||||
|
}
|
||||||
|
return yOffset, viewportHeight
|
||||||
|
}
|
||||||
|
|
||||||
// distributeHeight recalculates child component heights after a window resize,
|
// distributeHeight recalculates child component heights after a window resize,
|
||||||
// queue change, widget update, or state transition, and propagates the computed
|
// queue change, widget update, or state transition, and propagates the computed
|
||||||
// stream height to the StreamComponent.
|
// stream height to the StreamComponent.
|
||||||
@@ -3863,7 +3914,20 @@ func (m *AppModel) distributeHeight() {
|
|||||||
headerFooterLines += lipgloss.Height(footerView)
|
headerFooterLines += lipgloss.Height(footerView)
|
||||||
}
|
}
|
||||||
|
|
||||||
streamHeight := max(m.height-separatorLines-widgetLines-headerFooterLines-queuedLines-inputLines-statusBarLines, 0)
|
// Account for transient warning rows that View() injects between the
|
||||||
|
// scrollback and the separator. These flags are toggled by ESC/Ctrl+C
|
||||||
|
// handlers; without subtracting them here the joined view exceeds
|
||||||
|
// m.height by one line per active warning and the bottom of the screen
|
||||||
|
// gets silently clipped — which in turn invalidates scrollbackYOffset.
|
||||||
|
var warningLines int
|
||||||
|
if m.canceling {
|
||||||
|
warningLines++
|
||||||
|
}
|
||||||
|
if m.ctrlCPressedOnce {
|
||||||
|
warningLines++
|
||||||
|
}
|
||||||
|
|
||||||
|
streamHeight := max(m.height-separatorLines-widgetLines-headerFooterLines-queuedLines-inputLines-statusBarLines-warningLines, 0)
|
||||||
|
|
||||||
// In alt screen mode, give the calculated height to ScrollList instead of stream.
|
// In alt screen mode, give the calculated height to ScrollList instead of stream.
|
||||||
// The stream component still exists but is embedded as the last item in scrollList.
|
// The stream component still exists but is embedded as the last item in scrollList.
|
||||||
|
|||||||
@@ -60,10 +60,13 @@ func NewScrollList(width, height int) *ScrollList {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// SetItems replaces the items in the scroll list. If auto-scroll is enabled,
|
// SetItems replaces the items in the scroll list. If auto-scroll is enabled,
|
||||||
// the viewport will scroll to the bottom to show the latest content.
|
// the viewport will scroll to the bottom to show the latest content — EXCEPT
|
||||||
|
// when the user is actively selecting text (mouse button held), in which case
|
||||||
|
// the scroll position is locked so the highlighted content stays under the
|
||||||
|
// cursor. The pending bottom-scroll is deferred to MouseUp.
|
||||||
func (s *ScrollList) SetItems(items []MessageItem) {
|
func (s *ScrollList) SetItems(items []MessageItem) {
|
||||||
s.items = items
|
s.items = items
|
||||||
if s.autoScroll {
|
if s.autoScroll && !s.sel.MouseDown {
|
||||||
s.GotoBottom()
|
s.GotoBottom()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -157,6 +160,10 @@ func (s *ScrollList) HandleMouseDown(x, y int) bool {
|
|||||||
// HandleMouseDrag handles mouse motion while button is held.
|
// HandleMouseDrag handles mouse motion while button is held.
|
||||||
// Updates the selection endpoint for character-level precision.
|
// Updates the selection endpoint for character-level precision.
|
||||||
// Returns true if selection was updated.
|
// Returns true if selection was updated.
|
||||||
|
//
|
||||||
|
// Defensively disables auto-scroll on every drag update — even if the
|
||||||
|
// MouseDown handler missed (e.g. click landed in viewport padding), any
|
||||||
|
// active drag means the user is selecting and the viewport must not jump.
|
||||||
func (s *ScrollList) HandleMouseDrag(x, y int) bool {
|
func (s *ScrollList) HandleMouseDrag(x, y int) bool {
|
||||||
if !s.sel.MouseDown {
|
if !s.sel.MouseDown {
|
||||||
return false
|
return false
|
||||||
@@ -171,6 +178,9 @@ func (s *ScrollList) HandleMouseDrag(x, y int) bool {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Hard-lock the viewport while dragging.
|
||||||
|
s.autoScroll = false
|
||||||
|
|
||||||
s.sel.DragItemIdx = itemIdx
|
s.sel.DragItemIdx = itemIdx
|
||||||
s.sel.DragLineIdx = lineIdx
|
s.sel.DragLineIdx = lineIdx
|
||||||
s.sel.DragCol = x
|
s.sel.DragCol = x
|
||||||
@@ -178,6 +188,13 @@ func (s *ScrollList) HandleMouseDrag(x, y int) bool {
|
|||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// IsMouseDown reports whether the user currently has the mouse button held
|
||||||
|
// (i.e. a selection drag is in progress). Used by the parent model to avoid
|
||||||
|
// re-enabling auto-scroll during streaming while the user is selecting.
|
||||||
|
func (s *ScrollList) IsMouseDown() bool {
|
||||||
|
return s.sel.MouseDown
|
||||||
|
}
|
||||||
|
|
||||||
// HandleMouseUp handles mouse button release.
|
// HandleMouseUp handles mouse button release.
|
||||||
// Returns true if there was an active selection.
|
// Returns true if there was an active selection.
|
||||||
func (s *ScrollList) HandleMouseUp() bool {
|
func (s *ScrollList) HandleMouseUp() bool {
|
||||||
|
|||||||
@@ -0,0 +1,132 @@
|
|||||||
|
package ui
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// fakeItem is a deterministic MessageItem for ScrollList tests.
|
||||||
|
type fakeItem struct {
|
||||||
|
id string
|
||||||
|
lines int
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeItem) ID() string { return f.id }
|
||||||
|
func (f *fakeItem) Render(_ int) string {
|
||||||
|
if f.lines <= 0 {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
parts := make([]string, f.lines)
|
||||||
|
for i := range parts {
|
||||||
|
parts[i] = fmt.Sprintf("%s-line-%d", f.id, i)
|
||||||
|
}
|
||||||
|
return strings.Join(parts, "\n")
|
||||||
|
}
|
||||||
|
func (f *fakeItem) Height() int { return f.lines }
|
||||||
|
|
||||||
|
// makeItems builds n fake items of `lines` height each.
|
||||||
|
func makeItems(n, lines int) []MessageItem {
|
||||||
|
out := make([]MessageItem, n)
|
||||||
|
for i := range out {
|
||||||
|
out[i] = &fakeItem{id: fmt.Sprintf("item-%d", i), lines: lines}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestScrollList_MouseDownPreventsAutoScroll verifies the core fix for the
|
||||||
|
// copy-selection drift bug: while the user has the mouse button held
|
||||||
|
// (drag-selecting), incoming content updates must NOT shift the viewport,
|
||||||
|
// because doing so moves the highlighted content out from under the cursor.
|
||||||
|
func TestScrollList_MouseDownPreventsAutoScroll(t *testing.T) {
|
||||||
|
sl := NewScrollList(80, 10)
|
||||||
|
sl.SetItems(makeItems(20, 2)) // 40 lines of content into a 10-line viewport
|
||||||
|
// Capture the auto-scrolled-to-bottom position.
|
||||||
|
startOffsetIdx := sl.offsetIdx
|
||||||
|
startOffsetLine := sl.offsetLine
|
||||||
|
|
||||||
|
// User clicks somewhere in the visible area, starting a drag-select.
|
||||||
|
if !sl.HandleMouseDown(5, 3) {
|
||||||
|
t.Fatalf("HandleMouseDown should accept a click inside the viewport")
|
||||||
|
}
|
||||||
|
if !sl.IsMouseDown() {
|
||||||
|
t.Fatalf("IsMouseDown should be true after HandleMouseDown")
|
||||||
|
}
|
||||||
|
|
||||||
|
// New content arrives. With autoScroll still true, SetItems would
|
||||||
|
// normally call GotoBottom() and shift the viewport. The fix should
|
||||||
|
// suppress that while MouseDown is held.
|
||||||
|
sl.SetItems(makeItems(30, 2)) // 60 lines now
|
||||||
|
if sl.offsetIdx != startOffsetIdx || sl.offsetLine != startOffsetLine {
|
||||||
|
t.Errorf("viewport scrolled during active drag: was (%d,%d), now (%d,%d)",
|
||||||
|
startOffsetIdx, startOffsetLine, sl.offsetIdx, sl.offsetLine)
|
||||||
|
}
|
||||||
|
|
||||||
|
// User releases the mouse — drag is over.
|
||||||
|
sl.HandleMouseUp()
|
||||||
|
if sl.IsMouseDown() {
|
||||||
|
t.Fatalf("IsMouseDown should be false after HandleMouseUp")
|
||||||
|
}
|
||||||
|
|
||||||
|
// After release, a fresh content update should resume auto-scrolling
|
||||||
|
// (move the offset to track the new bottom).
|
||||||
|
afterReleaseIdx := sl.offsetIdx
|
||||||
|
afterReleaseLine := sl.offsetLine
|
||||||
|
sl.SetItems(makeItems(50, 2))
|
||||||
|
if sl.offsetIdx == afterReleaseIdx && sl.offsetLine == afterReleaseLine {
|
||||||
|
t.Errorf("autoscroll did not resume after MouseUp: offset stuck at (%d,%d)",
|
||||||
|
afterReleaseIdx, afterReleaseLine)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestScrollList_DragDisablesAutoScroll verifies that any successful
|
||||||
|
// HandleMouseDrag call clears autoScroll, even when HandleMouseDown didn't
|
||||||
|
// observe it (e.g. a stale wheel-down event set it back to true mid-stream).
|
||||||
|
func TestScrollList_DragDisablesAutoScroll(t *testing.T) {
|
||||||
|
sl := NewScrollList(80, 10)
|
||||||
|
sl.SetItems(makeItems(20, 2))
|
||||||
|
|
||||||
|
// Begin a selection.
|
||||||
|
if !sl.HandleMouseDown(5, 3) {
|
||||||
|
t.Fatalf("HandleMouseDown failed")
|
||||||
|
}
|
||||||
|
// Simulate an external code path that re-enabled autoScroll while
|
||||||
|
// MouseDown is still held (the precise condition that caused drift).
|
||||||
|
sl.autoScroll = true
|
||||||
|
|
||||||
|
// Drag motion should hard-lock the viewport again.
|
||||||
|
if !sl.HandleMouseDrag(10, 4) {
|
||||||
|
t.Fatalf("HandleMouseDrag failed")
|
||||||
|
}
|
||||||
|
if sl.autoScroll {
|
||||||
|
t.Errorf("HandleMouseDrag must clear autoScroll to prevent mid-drag jumps")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestScrollList_SetItemsRespectsMouseDown is the most direct regression
|
||||||
|
// test: even with autoScroll enabled and new content appended at the
|
||||||
|
// bottom, SetItems must not move the viewport while a mouse drag is in
|
||||||
|
// progress. This is what caused the "highlighting shifts by 1+ rows
|
||||||
|
// during streaming" symptom reported by the user.
|
||||||
|
func TestScrollList_SetItemsRespectsMouseDown(t *testing.T) {
|
||||||
|
sl := NewScrollList(80, 5)
|
||||||
|
sl.SetItems(makeItems(10, 2)) // 20 lines into a 5-line viewport
|
||||||
|
// At bottom.
|
||||||
|
preIdx, preLine := sl.offsetIdx, sl.offsetLine
|
||||||
|
|
||||||
|
// Hold mouse down (no actual drag needed).
|
||||||
|
if !sl.HandleMouseDown(0, 0) {
|
||||||
|
t.Fatalf("HandleMouseDown failed")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Append several more items as if streaming. With the bug, each
|
||||||
|
// SetItems would call GotoBottom and shift the offset.
|
||||||
|
for n := 11; n <= 15; n++ {
|
||||||
|
sl.SetItems(makeItems(n, 2))
|
||||||
|
if sl.offsetIdx != preIdx || sl.offsetLine != preLine {
|
||||||
|
t.Fatalf("viewport drifted during streaming with mouse held: "+
|
||||||
|
"start=(%d,%d) now=(%d,%d) after adding item %d",
|
||||||
|
preIdx, preLine, sl.offsetIdx, sl.offsetLine, n)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user