From 202a41364208b6e5217f4ef66c5f5ed4e2e2a9d6 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 27 Jun 2026 09:05:41 +0000 Subject: [PATCH 1/3] fix(workspaces): mobile touch scroll + copy/paste in the terminal console MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console (xterm.js, CLI/tmux `claude`) was unusable on touch devices. xterm's own touch-scroll handlers bail out when the application enables mouse tracking — exactly what tmux/`claude` do — so swipes did nothing, while copy and paste were mouse-only (drag-select + right-click). Desktop was unaffected and must stay byte-for-byte unchanged. - Touch→wheel bridge (terminalTouchScroll.ts): translate a single-finger vertical swipe into the SAME WheelEvents xterm already handles for a desktop mouse wheel — forwarded to claude/tmux under mouse tracking, native scrollback otherwise. deltaMode:1 + screen-clamped coords sidestep xterm's private scroll internals; the gesture logic is a pure, unit-tested controller. Bridge only acts when mouse tracking is on, so xterm's native touch scroll still drives shell side terminals. - Touch-only Copy / Paste / Keyboard controls (TerminalMobileControls.tsx), gated on pointer-coarse / maxTouchPoints, collapsible, with explicit action feedback. Copy uses the selection or the visible screen (respecting wrapped lines). - Desktop untouched: touch events never fire for mouse users, the controls render null off-touch, and no wheel/selection/contextmenu/OSC52 path changes. --- .../components/TerminalMobileControls.tsx | 193 +++++++++++++++++ .../src/shared/components/XTermInstance.tsx | 13 +- .../src/shared/hooks/useIsTouchDevice.ts | 23 ++ .../shared/lib/terminalTouchScroll.test.ts | 135 ++++++++++++ .../src/shared/lib/terminalTouchScroll.ts | 202 ++++++++++++++++++ .../shared/lib/terminalViewportText.test.ts | 54 +++++ .../src/shared/lib/terminalViewportText.ts | 29 +++ 7 files changed, 648 insertions(+), 1 deletion(-) create mode 100644 packages/web-core/src/shared/components/TerminalMobileControls.tsx create mode 100644 packages/web-core/src/shared/hooks/useIsTouchDevice.ts create mode 100644 packages/web-core/src/shared/lib/terminalTouchScroll.test.ts create mode 100644 packages/web-core/src/shared/lib/terminalTouchScroll.ts create mode 100644 packages/web-core/src/shared/lib/terminalViewportText.test.ts create mode 100644 packages/web-core/src/shared/lib/terminalViewportText.ts diff --git a/packages/web-core/src/shared/components/TerminalMobileControls.tsx b/packages/web-core/src/shared/components/TerminalMobileControls.tsx new file mode 100644 index 0000000000..069f918840 --- /dev/null +++ b/packages/web-core/src/shared/components/TerminalMobileControls.tsx @@ -0,0 +1,193 @@ +import { useEffect, useRef, useState } from 'react'; +import type { Terminal } from '@xterm/xterm'; +import { + CopyIcon, + ClipboardTextIcon, + KeyboardIcon, + CaretLeftIcon, + CaretRightIcon, +} from '@phosphor-icons/react'; + +import { cn } from '@/shared/lib/utils'; +import { useIsTouchDevice } from '@/shared/hooks/useIsTouchDevice'; +import { extractViewportText } from '@/shared/lib/terminalViewportText'; + +interface TerminalMobileControlsProps { + /** Live terminal accessor — refs don't trigger renders, so read on demand. */ + getTerminal: () => Terminal | null; +} + +const STATUS_MS = 1600; + +/** + * Touch-only Copy / Paste / Keyboard affordances for the terminal. Desktop keeps + * its mouse/keyboard flow (drag-select, right-click, Ctrl/Cmd+V) untouched — this + * renders nothing unless the device is touch-capable. + * + * Mounted as a sibling of (NOT inside) the xterm element so taps never reach + * xterm's focus/selection handling. Collapsible and pinned top-right so it can't + * cover claude's bottom input. Every action gives explicit feedback (mobile + * clipboard calls fail silently otherwise). + */ +export function TerminalMobileControls({ + getTerminal, +}: TerminalMobileControlsProps) { + const isTouch = useIsTouchDevice(); + const [expanded, setExpanded] = useState(true); + const [status, setStatus] = useState(null); + const statusTimer = useRef | null>(null); + + useEffect( + () => () => { + if (statusTimer.current) clearTimeout(statusTimer.current); + }, + [] + ); + + if (!isTouch) return null; + + const flash = (msg: string) => { + setStatus(msg); + if (statusTimer.current) clearTimeout(statusTimer.current); + statusTimer.current = setTimeout(() => setStatus(null), STATUS_MS); + }; + + const handleKeyboard = () => { + getTerminal()?.focus(); + }; + + const handlePaste = async () => { + const term = getTerminal(); + if (!term) return; + try { + const text = await navigator.clipboard?.readText(); + if (!text) { + flash('Clipboard empty'); + return; + } + term.paste(text); + flash('Pasted'); + } catch { + flash('Paste blocked'); + } + }; + + const handleCopy = async () => { + const term = getTerminal(); + if (!term) return; + let text: string; + let label: string; + if (term.hasSelection()) { + text = term.getSelection(); + label = 'Copied selection'; + } else { + const buf = term.buffer.active; + text = extractViewportText(buf, buf.viewportY, term.rows); + label = 'Copied screen'; + } + if (!text) { + flash('Nothing to copy'); + return; + } + try { + await navigator.clipboard?.writeText(text); + flash(label); + } catch { + flash('Copy blocked'); + } + }; + + // Belt-and-suspenders: keep taps on the controls from reaching the terminal. + const stop = (e: { stopPropagation: () => void }) => e.stopPropagation(); + + const button = + 'flex items-center justify-center size-11 rounded-md bg-secondary border ' + + 'text-low hover:text-normal active:bg-primary transition-colors'; + + return ( +
+ {status && ( + + {status} + + )} + {expanded && ( + <> + + + + + )} + +
+ ); +} diff --git a/packages/web-core/src/shared/components/XTermInstance.tsx b/packages/web-core/src/shared/components/XTermInstance.tsx index 85bad639db..231e5945c2 100644 --- a/packages/web-core/src/shared/components/XTermInstance.tsx +++ b/packages/web-core/src/shared/components/XTermInstance.tsx @@ -9,7 +9,9 @@ import { getTerminalTheme, } from '@/shared/lib/terminalTheme'; import { buildTerminalWsUrl } from '@/shared/lib/terminalWsUrl'; +import { installTerminalTouchScroll } from '@/shared/lib/terminalTouchScroll'; import { useTerminal } from '@/shared/hooks/useTerminal'; +import { TerminalMobileControls } from './TerminalMobileControls'; interface XTermInstanceProps { tabId: string; @@ -188,6 +190,14 @@ export function XTermInstance({ }); } }); + + // Mobile/touch: bridge vertical swipes to the SAME wheel events xterm + // already handles for a desktop mouse wheel (see terminalTouchScroll). + // Attached once on the freshly created element and intentionally NOT + // removed in the mount cleanup below — like the listeners above it lives + // with the element and tears down on terminal.dispose(). Removing it per + // unmount would leave reattached terminals without mobile scrolling. + installTerminalTouchScroll(terminal); } terminalRef.current = terminal; @@ -258,10 +268,11 @@ export function XTermInstance({ // theme.
+ terminalRef.current} />
); } diff --git a/packages/web-core/src/shared/hooks/useIsTouchDevice.ts b/packages/web-core/src/shared/hooks/useIsTouchDevice.ts new file mode 100644 index 0000000000..7b9bb631a2 --- /dev/null +++ b/packages/web-core/src/shared/hooks/useIsTouchDevice.ts @@ -0,0 +1,23 @@ +import { useState } from 'react'; + +/** + * Capability-based touch detection (coarse pointer OR touch points). + * + * Unlike `useIsRealMobile()` (user-agent based), this also covers iPadOS with a + * desktop UA and hybrid touch laptops — the population that needs the on-screen + * terminal controls. Device capability does not change within a session, so it + * is computed once. + */ +export function isTouchDevice(): boolean { + if (typeof window === 'undefined') return false; + const coarsePointer = + typeof window.matchMedia === 'function' && + window.matchMedia('(pointer: coarse)').matches; + return coarsePointer || navigator.maxTouchPoints > 0; +} + +/** React hook version — stable, computed once (no re-render on resize). */ +export function useIsTouchDevice(): boolean { + const [touch] = useState(isTouchDevice); + return touch; +} diff --git a/packages/web-core/src/shared/lib/terminalTouchScroll.test.ts b/packages/web-core/src/shared/lib/terminalTouchScroll.test.ts new file mode 100644 index 0000000000..f1ba155818 --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalTouchScroll.test.ts @@ -0,0 +1,135 @@ +import { describe, it, expect } from 'vitest'; + +import { + createTouchScrollController, + decideAxis, + clampToRect, + WHEEL_STEP_PX, + AXIS_LOCK_THRESHOLD_PX, +} from './terminalTouchScroll'; + +function controllerWith(mode: string) { + const wheels: Array<1 | -1> = []; + const ctrl = createTouchScrollController({ + getMouseTrackingMode: () => mode, + dispatchWheel: (dir) => wheels.push(dir), + }); + return { ctrl, wheels }; +} + +describe('decideAxis', () => { + it('stays undecided until movement passes the lock threshold', () => { + expect(decideAxis(2, 3)).toBe('undecided'); + }); + it('locks vertical when vertical travel dominates', () => { + expect(decideAxis(2, AXIS_LOCK_THRESHOLD_PX + 5)).toBe('vertical'); + }); + it('locks ignore when horizontal travel dominates', () => { + expect(decideAxis(AXIS_LOCK_THRESHOLD_PX + 5, 2)).toBe('ignore'); + }); +}); + +describe('clampToRect', () => { + it('pulls a point outside the rect back inside its bounds', () => { + const rect = { left: 10, top: 20, right: 110, bottom: 220 }; + expect(clampToRect(0, 0, rect)).toEqual({ x: 11, y: 21 }); + expect(clampToRect(999, 999, rect)).toEqual({ x: 109, y: 219 }); + }); + it('leaves an interior point unchanged', () => { + const rect = { left: 0, top: 0, right: 100, bottom: 100 }; + expect(clampToRect(50, 60, rect)).toEqual({ x: 50, y: 60 }); + }); +}); + +describe('createTouchScrollController (mouse tracking active)', () => { + it('translates an upward swipe into floor(travel / step) scroll-down wheels', () => { + const { ctrl, wheels } = controllerWith('vt200'); + ctrl.onTouchStart({ touches: 1, clientX: 100, clientY: 300 }); + // Cross the axis threshold, then swipe up a total of 3 full steps. + ctrl.onTouchMove({ + touches: 1, + clientX: 100, + clientY: 300 - (AXIS_LOCK_THRESHOLD_PX + 1), + }); + const r = ctrl.onTouchMove({ + touches: 1, + clientX: 100, + clientY: 300 - 3 * WHEEL_STEP_PX, + }); + expect(wheels).toEqual([1, 1, 1]); // finger up => scroll down + expect(r.prevent).toBe(true); + }); + + it('translates a downward swipe into scroll-up wheels', () => { + const { ctrl, wheels } = controllerWith('any'); + ctrl.onTouchStart({ touches: 1, clientX: 0, clientY: 0 }); + ctrl.onTouchMove({ + touches: 1, + clientX: 0, + clientY: AXIS_LOCK_THRESHOLD_PX + 1, + }); + ctrl.onTouchMove({ touches: 1, clientX: 0, clientY: 2 * WHEEL_STEP_PX }); + expect(wheels).toEqual([-1, -1]); // finger down => scroll up (history) + }); + + it('prevents default for a committed vertical gesture even before a full step', () => { + const { ctrl, wheels } = controllerWith('vt200'); + ctrl.onTouchStart({ touches: 1, clientX: 0, clientY: 0 }); + const r = ctrl.onTouchMove({ + touches: 1, + clientX: 0, + clientY: -(AXIS_LOCK_THRESHOLD_PX + 1), + }); + expect(r.prevent).toBe(true); + expect(wheels.length).toBe(0); // not a full step yet, but gesture is owned + }); +}); + +describe('createTouchScrollController (mouse tracking inactive)', () => { + it('never synthesizes wheels and never prevents — xterm native touch scrolls', () => { + const { ctrl, wheels } = controllerWith('none'); + ctrl.onTouchStart({ touches: 1, clientX: 0, clientY: 0 }); + const r = ctrl.onTouchMove({ touches: 1, clientX: 0, clientY: -100 }); + expect(wheels.length).toBe(0); + expect(r.prevent).toBe(false); + }); +}); + +describe('createTouchScrollController (gesture guards)', () => { + it('ignores multi-touch (pinch) gestures', () => { + const { ctrl, wheels } = controllerWith('vt200'); + ctrl.onTouchStart({ touches: 2, clientX: 0, clientY: 0 }); + const r = ctrl.onTouchMove({ touches: 2, clientX: 0, clientY: -100 }); + expect(wheels.length).toBe(0); + expect(r.prevent).toBe(false); + }); + + it('ignores a horizontal swipe', () => { + const { ctrl, wheels } = controllerWith('vt200'); + ctrl.onTouchStart({ touches: 1, clientX: 0, clientY: 0 }); + const r = ctrl.onTouchMove({ + touches: 1, + clientX: AXIS_LOCK_THRESHOLD_PX + 50, + clientY: 1, + }); + expect(wheels.length).toBe(0); + expect(r.prevent).toBe(false); + }); + + it('resets between gestures so a new swipe starts clean', () => { + const { ctrl, wheels } = controllerWith('vt200'); + ctrl.onTouchStart({ touches: 1, clientX: 0, clientY: 0 }); + // Finger down one full step => one scroll-up (-1) wheel. + ctrl.onTouchMove({ touches: 1, clientX: 0, clientY: WHEEL_STEP_PX }); + ctrl.onTouchEnd(); + // A fresh horizontal gesture must not inherit the previous vertical lock. + ctrl.onTouchStart({ touches: 1, clientX: 0, clientY: 0 }); + const r = ctrl.onTouchMove({ + touches: 1, + clientX: AXIS_LOCK_THRESHOLD_PX + 50, + clientY: 0, + }); + expect(r.prevent).toBe(false); + expect(wheels).toEqual([-1]); // only the first gesture's single step + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalTouchScroll.ts b/packages/web-core/src/shared/lib/terminalTouchScroll.ts new file mode 100644 index 0000000000..c26260abd0 --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalTouchScroll.ts @@ -0,0 +1,202 @@ +import type { Terminal } from '@xterm/xterm'; + +/** + * Touch → wheel scroll bridge for xterm.js 5.5. + * + * Why this exists: xterm binds its own touch-scroll handlers to `terminal.element`, + * but they early-return when the application has enabled mouse tracking + * (`coreMouseService.areMouseEventsActive`). In the CLI pane, tmux/`claude` turn + * mouse tracking on, so a finger swipe does nothing while the desktop mouse wheel + * still scrolls. We close the gap by translating a vertical swipe into the SAME + * synthetic `WheelEvent`s xterm already handles for a real mouse wheel: + * - mouse tracking active → xterm forwards the wheel to the app (tmux/claude scrolls) + * - mouse tracking inactive → we stay out of the way so xterm's native touch + * handler scrolls local scrollback (no double-scroll). + * + * xterm 5.5 coupling — RE-VALIDATE ON UPGRADE (pinned by a Playwright contract + * check, see tmp ship-it spec): the wheel listener is on `terminal.element`; + * `deltaMode: 1` (DOM_DELTA_LINE) + `deltaY: ±1` bypasses xterm's private + * pixel→row accumulator and deterministically scrolls exactly one line; the + * pointer coords must land inside `.xterm-screen` or xterm drops the report. + * + * The DOM-free `createTouchScrollController` is the unit-tested seam; + * `installTerminalTouchScroll` is a thin adapter that wires real touch events to it. + */ + +export type Axis = 'undecided' | 'vertical' | 'ignore'; + +export interface Rect { + left: number; + top: number; + right: number; + bottom: number; +} + +/** Accumulated finger travel (px) per dispatched line-wheel. */ +export const WHEEL_STEP_PX = 24; +/** Finger travel (px) before the gesture locks to an axis. */ +export const AXIS_LOCK_THRESHOLD_PX = 8; + +export function decideAxis( + dx: number, + dy: number, + threshold = AXIS_LOCK_THRESHOLD_PX +): Axis { + if (Math.abs(dx) < threshold && Math.abs(dy) < threshold) return 'undecided'; + // Ties go to "ignore" so an ambiguous diagonal doesn't hijack horizontal intent. + return Math.abs(dy) > Math.abs(dx) ? 'vertical' : 'ignore'; +} + +export function clampToRect( + x: number, + y: number, + rect: Rect +): { x: number; y: number } { + return { + x: Math.min(Math.max(x, rect.left + 1), rect.right - 1), + y: Math.min(Math.max(y, rect.top + 1), rect.bottom - 1), + }; +} + +export interface TouchPoint { + touches: number; + clientX: number; + clientY: number; +} + +export interface TouchScrollDeps { + /** Current xterm mouse-tracking mode; the bridge only acts when !== 'none'. */ + getMouseTrackingMode: () => string; + /** Dispatch one line-wheel: +1 = scroll down (toward newer), -1 = scroll up. */ + dispatchWheel: (direction: 1 | -1, clientX: number, clientY: number) => void; +} + +export interface TouchMoveResult { + /** Whether the caller should `preventDefault()` (we own this gesture). */ + prevent: boolean; +} + +/** + * Pure, DOM-free gesture controller — the testable seam. + */ +export function createTouchScrollController(deps: TouchScrollDeps) { + let axis: Axis = 'undecided'; + let startX = 0; + let startY = 0; + let lastY = 0; + let accumulated = 0; + + return { + onTouchStart(p: TouchPoint): void { + if (p.touches !== 1) { + axis = 'ignore'; // pinch / multi-touch — leave it to the browser + return; + } + axis = 'undecided'; + startX = p.clientX; + startY = p.clientY; + lastY = p.clientY; + accumulated = 0; + }, + + onTouchMove(p: TouchPoint): TouchMoveResult { + if (p.touches !== 1 || axis === 'ignore') return { prevent: false }; + + if (axis === 'undecided') { + axis = decideAxis(p.clientX - startX, p.clientY - startY); + if (axis !== 'vertical') return { prevent: false }; + } + + // Only bridge when xterm's own touch scroll is disabled (mouse tracking on). + // Otherwise let xterm scroll its scrollback natively — avoids double-scroll. + if (deps.getMouseTrackingMode() === 'none') return { prevent: false }; + + // Natural scrolling: finger up (clientY decreases) reveals newer content + // (scroll down, +1); finger down reveals history (scroll up, -1). + accumulated += lastY - p.clientY; + lastY = p.clientY; + + while (accumulated >= WHEEL_STEP_PX) { + deps.dispatchWheel(1, p.clientX, p.clientY); + accumulated -= WHEEL_STEP_PX; + } + while (accumulated <= -WHEEL_STEP_PX) { + deps.dispatchWheel(-1, p.clientX, p.clientY); + accumulated += WHEEL_STEP_PX; + } + + // We've committed to the vertical gesture: always prevent page scroll/rubber-band, + // even on sub-step moves that didn't dispatch a wheel yet. + return { prevent: true }; + }, + + onTouchEnd(): void { + axis = 'undecided'; + accumulated = 0; + }, + }; +} + +/** + * Wire the controller to a live xterm terminal's element. Attach ONCE per created + * terminal (in the creation branch) and do NOT remove on React unmount — the + * listeners live on `terminal.element` and tear down with it on + * `terminal.dispose()`, mirroring the existing `contextmenu`/selection handlers. + * Returns a disposer for tests and explicit teardown. + */ +export function installTerminalTouchScroll(terminal: Terminal): () => void { + const el = terminal.element; + if (!el) return () => {}; + + const getScreenRect = (): Rect => { + const screen = el.querySelector('.xterm-screen') as HTMLElement | null; + const r = (screen ?? el).getBoundingClientRect(); + return { left: r.left, top: r.top, right: r.right, bottom: r.bottom }; + }; + + const controller = createTouchScrollController({ + getMouseTrackingMode: () => terminal.modes.mouseTrackingMode, + dispatchWheel: (direction, clientX, clientY) => { + const { x, y } = clampToRect(clientX, clientY, getScreenRect()); + el.dispatchEvent( + new WheelEvent('wheel', { + deltaY: direction, + deltaMode: 1, // DOM_DELTA_LINE — bypasses xterm's private px→row accumulator + bubbles: true, + cancelable: true, + clientX: x, + clientY: y, + }) + ); + }, + }); + + const toPoint = (e: TouchEvent): TouchPoint => { + const t = e.touches[0] ?? e.changedTouches[0]; + return { + touches: e.touches.length, + clientX: t?.clientX ?? 0, + clientY: t?.clientY ?? 0, + }; + }; + + const onStart = (e: TouchEvent) => controller.onTouchStart(toPoint(e)); + const onMove = (e: TouchEvent) => { + if (controller.onTouchMove(toPoint(e)).prevent && e.cancelable) { + e.preventDefault(); + } + }; + const onEnd = () => controller.onTouchEnd(); + + el.addEventListener('touchstart', onStart, { passive: true }); + el.addEventListener('touchmove', onMove, { passive: false }); + el.addEventListener('touchend', onEnd, { passive: true }); + el.addEventListener('touchcancel', onEnd, { passive: true }); + + return () => { + el.removeEventListener('touchstart', onStart); + el.removeEventListener('touchmove', onMove); + el.removeEventListener('touchend', onEnd); + el.removeEventListener('touchcancel', onEnd); + }; +} diff --git a/packages/web-core/src/shared/lib/terminalViewportText.test.ts b/packages/web-core/src/shared/lib/terminalViewportText.test.ts new file mode 100644 index 0000000000..ab6341cd5b --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalViewportText.test.ts @@ -0,0 +1,54 @@ +import { describe, it, expect } from 'vitest'; + +import { extractViewportText } from './terminalViewportText'; + +type FakeLine = { + isWrapped: boolean; + translateToString: (trimRight?: boolean) => string; +}; + +function line(text: string, isWrapped = false): FakeLine { + return { + isWrapped, + translateToString: (trimRight?: boolean) => + trimRight ? text.replace(/\s+$/, '') : text, + }; +} + +function buffer(lines: Array) { + return { getLine: (i: number) => lines[i] } as never; +} + +describe('extractViewportText', () => { + it('joins separate logical lines with newlines', () => { + const buf = buffer([line('foo'), line('bar')]); + expect(extractViewportText(buf, 0, 2)).toBe('foo\nbar'); + }); + + it('keeps a wrapped line whole (no newline at the wrap point)', () => { + // Row 1 is a continuation of row 0 (isWrapped === true). + const buf = buffer([line('AAAA'), line('BBBB', true)]); + expect(extractViewportText(buf, 0, 2)).toBe('AAAABBBB'); + }); + + it('preserves trailing spaces within a wrapped segment but trims the tail row', () => { + const buf = buffer([line('one '), line('two', true), line('three')]); + // "one " keeps its space (continued), "two" then ends the logical line. + expect(extractViewportText(buf, 0, 3)).toBe('one two\nthree'); + }); + + it('trims trailing blank rows', () => { + const buf = buffer([line('x'), line(' '), line('')]); + expect(extractViewportText(buf, 0, 3)).toBe('x'); + }); + + it('tolerates missing rows without throwing', () => { + const buf = buffer([line('a'), undefined, line('b')]); + expect(extractViewportText(buf, 0, 3)).toBe('a\nb'); + }); + + it('honors the viewport offset', () => { + const buf = buffer([line('hidden'), line('shown')]); + expect(extractViewportText(buf, 1, 1)).toBe('shown'); + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalViewportText.ts b/packages/web-core/src/shared/lib/terminalViewportText.ts new file mode 100644 index 0000000000..d51beceec9 --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalViewportText.ts @@ -0,0 +1,29 @@ +import type { IBuffer } from '@xterm/xterm'; + +/** + * Reconstruct the text of the visible viewport, preserving logical lines across + * xterm's hard-wrapped rows. + * + * `IBufferLine.translateToString()` does not account for wrapping, so naively + * joining every visual row with "\n" corrupts wrapped output. xterm marks a row + * as a wrap-continuation of the previous one via `IBufferLine.isWrapped`, so we + * only insert a newline when the NEXT row is not a continuation of this one. + * + * Tolerates missing/blank lines, wide characters, and the alternate buffer. + */ +export function extractViewportText( + buffer: Pick, + viewportY: number, + rows: number +): string { + const parts: string[] = []; + for (let i = 0; i < rows; i++) { + const line = buffer.getLine(viewportY + i); + if (!line) continue; + const continued = buffer.getLine(viewportY + i + 1)?.isWrapped ?? false; + // Don't trim a wrapped row — its content runs straight into the next row. + parts.push(line.translateToString(!continued)); + if (!continued) parts.push('\n'); + } + return parts.join('').replace(/\n+$/, ''); +} From 50d2d508325988eaf6504bcefce3706d32d4bdbf Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 27 Jun 2026 09:24:05 +0000 Subject: [PATCH 2/3] fix(workspaces): harden terminal mobile controls + touch-scroll (review) Apply post-implementation review findings (codex + CodeRabbit + 4 cleanup passes): - Clipboard guards: explicitly check navigator.clipboard.readText/writeText before use. Optional chaining short-circuits to undefined on insecure contexts/WebViews, which made Copy flash "Copied" without copying and Paste say "empty" when blocked. Now reports "unavailable" distinctly. - Multi-touch: keep a gesture ignored until ALL fingers lift. A partial pinch release no longer lets the remaining finger resume bridged scrolling from stale state. Added a regression test. - Hot path: resolve .xterm-screen once at install and memoize the clamped point per touchmove instead of querySelector + getBoundingClientRect per dispatched wheel. - Cohesion: move isTouchDevice/useIsTouchDevice into useIsMobile.ts beside the viewport/UA detectors; drive the action buttons from a small array; hoist the button class to module scope; correct a comment that referenced an uncommitted contract test. --- .../components/TerminalMobileControls.tsx | 81 +++++++++---------- .../web-core/src/shared/hooks/useIsMobile.ts | 22 +++++ .../src/shared/hooks/useIsTouchDevice.ts | 23 ------ .../shared/lib/terminalTouchScroll.test.ts | 21 +++++ .../src/shared/lib/terminalTouchScroll.ts | 56 ++++++++----- 5 files changed, 118 insertions(+), 85 deletions(-) delete mode 100644 packages/web-core/src/shared/hooks/useIsTouchDevice.ts diff --git a/packages/web-core/src/shared/components/TerminalMobileControls.tsx b/packages/web-core/src/shared/components/TerminalMobileControls.tsx index 069f918840..8a5ffa2f7c 100644 --- a/packages/web-core/src/shared/components/TerminalMobileControls.tsx +++ b/packages/web-core/src/shared/components/TerminalMobileControls.tsx @@ -9,7 +9,7 @@ import { } from '@phosphor-icons/react'; import { cn } from '@/shared/lib/utils'; -import { useIsTouchDevice } from '@/shared/hooks/useIsTouchDevice'; +import { useIsTouchDevice } from '@/shared/hooks/useIsMobile'; import { extractViewportText } from '@/shared/lib/terminalViewportText'; interface TerminalMobileControlsProps { @@ -19,6 +19,10 @@ interface TerminalMobileControlsProps { const STATUS_MS = 1600; +const BUTTON_CLASS = + 'flex items-center justify-center size-11 rounded-md bg-secondary border ' + + 'text-low hover:text-normal active:bg-primary transition-colors'; + /** * Touch-only Copy / Paste / Keyboard affordances for the terminal. Desktop keeps * its mouse/keyboard flow (drag-select, right-click, Ctrl/Cmd+V) untouched — this @@ -59,8 +63,14 @@ export function TerminalMobileControls({ const handlePaste = async () => { const term = getTerminal(); if (!term) return; + // Insecure contexts / some WebViews have no Clipboard API at all — optional + // chaining would otherwise resolve to undefined and look like "empty". + if (!navigator.clipboard?.readText) { + flash('Paste unavailable'); + return; + } try { - const text = await navigator.clipboard?.readText(); + const text = await navigator.clipboard.readText(); if (!text) { flash('Clipboard empty'); return; @@ -75,6 +85,11 @@ export function TerminalMobileControls({ const handleCopy = async () => { const term = getTerminal(); if (!term) return; + // Guard the write API up front so we never flash "Copied" without copying. + if (!navigator.clipboard?.writeText) { + flash('Copy unavailable'); + return; + } let text: string; let label: string; if (term.hasSelection()) { @@ -90,20 +105,26 @@ export function TerminalMobileControls({ return; } try { - await navigator.clipboard?.writeText(text); + await navigator.clipboard.writeText(text); flash(label); } catch { flash('Copy blocked'); } }; + const actions = [ + { label: 'Copy from terminal', Icon: CopyIcon, onClick: handleCopy }, + { + label: 'Paste into terminal', + Icon: ClipboardTextIcon, + onClick: handlePaste, + }, + { label: 'Show keyboard', Icon: KeyboardIcon, onClick: handleKeyboard }, + ]; + // Belt-and-suspenders: keep taps on the controls from reaching the terminal. const stop = (e: { stopPropagation: () => void }) => e.stopPropagation(); - const button = - 'flex items-center justify-center size-11 rounded-md bg-secondary border ' + - 'text-low hover:text-normal active:bg-primary transition-colors'; - return (
)} - {expanded && ( - <> + {expanded && + actions.map(({ label, Icon, onClick }) => ( - - - - )} + ))}