Repository navigation
fix(terminal): scroll a phone drag by finger travel, not per touchmove - #963
Conversation
xterm sends herdr one wheel report per wheel event whatever its delta, and the touch handler dispatched one pixel wheel per touchmove, so a quick swipe sent a few reports while a slow drag over the same distance sent one per row. Gather the gesture's travel and send one line-mode wheel per cell height times herdr's default mouse_scroll_lines (3), keeping the rest for the next move. The pixel-to-step arithmetic is shared with the selecting drag's wheel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 39 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
…inger move by cell height The pinch check now drags 300 px once in 2 touchmoves and once in 30 and asserts both send the same count of line-mode wheels, about one per three rows. On the old per-touchmove dispatch this fails. The finger left after a partial pinch release now moves three wheel steps at the 20 px cell, so a broken pinch-stroke guard would send wheels there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r lifts Gathering finger travel into one wheel report per three rows dropped any flick shorter than a step: the remainder reset at each touchstart and touchend never sent it, so 30-40 px nudges at the default font scrolled nothing. On touchend of a one-finger drag (not a pinch, not an adopted grid, not a cancel), send one more report when at least half a step is left (finalStep in lib/wheelSteps.ts). Unit cases for finalStep and a browser case in terminal-pinch-regression: a 0.7-step flick sends one wheel, a 0.3-step flick sends none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pinch's touchStart moved finger 1 back 90 px, which Chrome reports as a one-finger touchmove first; with the drag's leftover that crossed a wheel step at CI's cell height. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
On a phone, a slow drag scrolls herdr's history steadily, but a quick swipe over the same distance moves only a few notches. The text does not follow the finger, and flicking through a long agent transcript takes many repeated drags.
Cause
onTouchMoveinPaneTerminal.tsxdispatched one synthetic pixel-modeWheelEventper touchmove. With mouse reporting on (herdr attach), xterm 5.5 sends exactly one wheel report per wheel event:getLinesScrolledis used only as a zero check, and its_wheelPartialScroll %= 1drops every whole row beyond the first. The wheel-speed replay only covers trusted events, so touch got no compensation. A 40-60 px touchmove (a flick) crossed 2-3 rows but sent one report; a slow drag sent about one per row.Fix
src/lib/wheelSteps.ts:wheelSteps(pixels, stepPixels)returns the whole signed steps and the pixels left over.HERDR_MOUSE_SCROLL_LINES = 3is herdr's defaultui.mouse_scroll_lines.touchPixels, reset on touchstart), divides it by cell height x 3, and dispatches that manyDOM_DELTA_LINEwheels ofdeltaY+-1 (each makes xterm send exactly one report), keeping the rest for the next move. One report then moves herdr about as far as the finger moved.dragWheel) uses the same helper and the samecellHeight()so the two conversions cannot drift.src/AGENTS.mdgeometry row updated.No momentum in this PR: a flick still stops where the finger lifts (
onTouchEndsends at most one leftover step, see the merge-session review below). render-scroll-2 also proposed a decaying momentum phase; that is deferred to a follow-up and this PR closes only the per-touchmove report loss.Tests
scripts/terminal-pinch-regression.ts(browser, runs inui-contexts-regression): at the 13 px baseline it drags 300 px once in 2 touchmoves and once in 30, and asserts both send the same wheel count (about 300 / (3 x cell height)) and that every wheel isDOM_DELTA_LINE. Verified to fail on main'sPaneTerminal.tsx("a 2-move swipe sends line-mode wheels only"). The finger left after a partial pinch release now moves three wheel steps of the 20 px cell (it moved 60 px, under one step, so that negative check could not fail).src/lib/wheelSteps.test.ts(new): five 60 px moves send the same report count as thirty 10 px moves (and 300 x 1 px equals one 300 px move); sign and remainder carry; a reversal pays back gathered travel first; a zero/NaN cell height makes no steps.bun run typecheck: pass.bun run test:unit: 0 fail.bun run build, thenbun run check run bun scripts/ui-contexts-regression.ts: pass, includingcheckTerminalPinch(earlier commit); after the test update,bun run check run bun scripts/terminal-pinch-regression.ts: pass. Its one-finger drag went from 60 px to 150 px: at a 20 px font, 60 px is less than one three-row step, so it would no longer send a wheel. That script only asserts that synthetic wheel events are dispatched; it does not check how far herdr scrolls.scripts/windows-lens-browser-qa.ts(manual QA, not a CI lane) is the only script that drags a finger into real herdr history. It failed twice at an earlier step ("the command's output reaches the mirrored terminal", in the mirror-only part), before the touch-scroll step ran. So the herdr-side scroll distance was not verified end to end here.Notes
mouse_scroll_lineswas changed, one step scrolls that many lines instead of 3, so the text moves proportionally faster or slower than the finger.terminalWheelSpeedstill does not apply to touch (unchanged: the replay is for trusted wheels only).PaneTerminal.tsxelsewhere: textual merge-conflict risk only, no semantic overlap.🤖 Generated with Claude Code
Merge-session review
Fixed (b462b57):
touchendof a one-finger drag (not a pinch, not an adopted grid, not atouchcancel), one more line-mode wheel is sent if at least half a step is left (finalStepinsrc/lib/wheelSteps.ts). Unit cases coverfinalStep.scripts/terminal-pinch-regression.tsnow checks that a 0.7-step flick sends exactly one wheel and a 0.3-step flick sends none. On the previous head this case failed: "a flick of 0.7 step (40 px) sends one wheel". The changelog fragment and thesrc/AGENTS.mdrow now describe the touchend step.Deferred:
Checks:
bun run check fastpasses, andCI=1 bun run check run --build bun scripts/terminal-pinch-regression.tspasses.