From 5d2f5f658c2c91a17a96b6f03bbc30ebacd9b4af Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 05:56:53 +0000 Subject: [PATCH 01/11] docs: mobile terminal UX design spec (keyboard viewport, key bar, gestures, select mode) --- .../2026-07-04-mobile-terminal-ux-design.md | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-04-mobile-terminal-ux-design.md diff --git a/docs/superpowers/specs/2026-07-04-mobile-terminal-ux-design.md b/docs/superpowers/specs/2026-07-04-mobile-terminal-ux-design.md new file mode 100644 index 0000000000..4232569e49 --- /dev/null +++ b/docs/superpowers/specs/2026-07-04-mobile-terminal-ux-design.md @@ -0,0 +1,174 @@ +# Mobile Terminal UX — Design + +**Date:** 2026-07-04 +**Status:** Approved (decisions settled interactively with Miguel; he is away during implementation) +**Builds on:** PR #22 (touch→wheel scroll bridge, mobile Copy/Paste/Keyboard controls, OSC 52 clipboard) + +## Problem + +The terminal console (xterm.js hosting the tmux-backed `claude` CLI session, plus +plain shell side terminals) is still close to unusable on phones: + +1. When the on-screen keyboard opens, nothing resizes or scrolls — the keyboard + covers the bottom half of the terminal, including claude's input line. +2. There is no way to select text by touch (xterm.js has no touch selection). +3. There is no way to send Esc, Tab, Shift+Tab, Ctrl combos, or arrow keys — + mobile soft keyboards simply don't have them. + +**Platforms:** iOS Safari (primary, plain web + home-screen PWA), Android Chrome +(secondary). Desktop behavior must remain byte-for-byte unchanged: everything +below is gated on touch capability (`useIsTouchDevice` / `isTouchDevice`, same +gates PR #22 used). + +**Scope:** `XTermInstance` and its satellites in `packages/web-core` — so it +applies to both the CLI main pane and shell side terminals automatically. + +## Decisions (settled with Miguel) + +| Topic | Decision | +|---|---| +| Keyboard handling | Shrink + refit: container tracks `visualViewport`, xterm refits, PTY/tmux resizes; input line always visible | +| Key bar | `esc ⇥ ⇧⇥ ^C ctrl ← ↓ ↑ → ⏎` with sticky/latching ctrl; fixed set, no settings UI | +| Gestures | Long-press + drag = arrow D-pad (3 speed tiers); double-tap = Tab; three-finger tap = paste. **No pinch zoom.** | +| Text selection | Explicit select-mode toggle button in the mobile controls (gestures/scroll bridge pause while active) | +| Font size | A+ / A− stepper buttons in mobile controls, persisted in localStorage | +| Delivery | PR + merge via ship-it light; **no /opt deploy** | + +## Design + +### 1. Keyboard-aware viewport (`useVisualViewportHeight` + layout wiring) + +New hook in `packages/web-core/src/shared/hooks/`: subscribes to +`window.visualViewport` `resize` + `scroll` events (SSR-safe, `null` / +no-op when `visualViewport` is unavailable or the device is not +touch-capable). It exposes the current visible height. + +Wiring: the terminal wrapper (in `XTermInstance`'s outer container's ancestor — +the workspace mobile layout) caps its height to the visual viewport when the +keyboard is open, instead of `h-screen`'s layout-viewport height. The existing +`ResizeObserver` + debounced `fitTerminal` in `XTermInstance` then does the +refit + WS resize automatically — no new resize plumbing. + +Details: + +- `interactive-widget=resizes-content` added to the viewport meta in both + `packages/local-web/index.html` and `packages/remote-web/index.html` — + Android Chrome then resizes the layout viewport natively; the hook remains + the iOS path (and a no-op safety net on Android). +- On keyboard open (viewport shrink) and on terminal focus: + `term.scrollToBottom()` so the prompt is visible immediately. +- Compensate iOS Safari's scroll-the-page-on-focus behavior: when + `visualViewport.offsetTop > 0`, scroll the window back so the app chrome + stays pinned (best-effort; keep it simple and defensive). +- The key bar (below) renders inside this sized container, bottom-anchored, so + it naturally sits directly above the system keyboard. + +### 2. Key bar (`TerminalKeyBar.tsx`) + +Touch-only component (returns `null` off-touch), rendered by `XTermInstance` +under the terminal viewport, always visible on touch devices (no fragile +keyboard-open detection; it doubles as the tap-target to focus the terminal). +Horizontally scrollable single row, 44px-high touch targets: + +`esc` `⇥` `⇧⇥` `^C` `ctrl` `←` `↓` `↑` `→` `⏎` + +Sequences sent through the existing single data pipe (`terminal.input(...)`, +which flows through `onData` → `conn.send`): + +- `esc` → `\x1b`; `⇥` → `\t`; `⇧⇥` → `\x1b[Z` (CSI Z); `^C` → `\x03`; + `⏎` → `\r`. +- Arrows respect application cursor keys mode + (`terminal.modes.applicationCursorKeysMode`): `\x1bOA…` vs `\x1b[A…`. +- Buttons must not steal focus from the terminal (`preventDefault` on + pointer/mouse down) so the system keyboard stays open. + +**Sticky ctrl:** tapping `ctrl` latches it (visually highlighted). The next +single printable character arriving on the data pipe is transformed to its +control code (`char & 0x1f`), then ctrl unlatches. Tapping `ctrl` again while +latched cancels. Implemented as a pure transform function +(`applyStickyCtrl(data, latched) → {out, stillLatched}`) inserted in the +`onData` → `send` path in `XTermInstance`, driven by shared state +(ref + subscription) owned by the key bar. Multi-char bursts (paste, IME +commits) pass through untouched and unlatch — only a single-char chunk is +transformed. Unit-tested. + +### 3. Gesture layer (`terminalTouchGestures.ts`) + +Same architecture as `terminalTouchScroll.ts`: a pure, unit-testable gesture +controller (state machine consuming `{type, touches, timestamp}` events and +emitting actions) plus a thin `install` function that binds real touch events +and executes actions. Coordinated with the existing scroll bridge: + +- **Long-press + drag = D-pad.** Finger down and held ≥ ~350 ms within a small + slop radius enters D-pad mode (a subtle overlay indicator appears). Dragging + past a threshold sends repeated arrow keys in the dominant axis direction; + repeat rate has 3 tiers by drag distance (Termius-style "gears"; roughly + ~110 ms / ~55 ms / ~25 ms per keystroke). Lifting the finger exits. While in + D-pad mode the scroll bridge is suppressed. +- **Movement before the long-press threshold** = scroll, handled by the + existing touch-scroll bridge exactly as today. +- **Double-tap = Tab.** Two taps within ~250 ms and tap-slop distance send + `\t`. Single taps still focus the terminal as before (tap behavior itself is + unchanged — we only add the second-tap interpretation). +- **Three-finger tap = paste.** Same clipboard read path + feedback as the + Paste button (including the availability guards from PR #22). +- Arrow sequences honor application cursor keys mode (shared helper with the + key bar). +- Any multi-touch that isn't a clean three-finger tap keeps today's "ignore + until all fingers lift" rule. + +### 4. Select mode (toggle in `TerminalMobileControls`) + +New toggle button (e.g. text-cursor icon) in the existing mobile controls: + +- **On:** gesture layer + touch-scroll bridge are disabled. Touch drag maps to + xterm buffer selection: translate touch coords → cell coords using the + `.xterm-screen` bounding box and `cols`/`rows`, drive + `terminal.select(col, row, length)` across the drag (line-spanning + selections included). The existing `onSelectionChange` handler already + copies selections to the clipboard; the Copy button also picks them up. + A status flash ("Select mode — drag to select") explains the state. +- **Off (default):** everything behaves as designed above. Toggling off clears + the selection. +- No synthetic mouse events are forwarded to tmux — selection is purely + client-side over the visible buffer, which sidesteps mouse-tracking entirely. +- This is the riskiest item (Miguel: skippable if it breaks things). It is + last in the build order and isolated behind its toggle so it can be dropped + without touching the rest. + +### 5. Font size steppers + +`A−` / `A+` buttons in `TerminalMobileControls`: adjust +`terminal.options.fontSize` within 8–20 (default 12), call `fitTerminal`-style +refit (fit + WS resize), persist to `localStorage` +(`vk-terminal-mobile-font-size`). On touch devices, terminal creation reads the +persisted value; desktop keeps the hardcoded 12. + +## Error handling + +- All new listeners/timers install once per created terminal (PR #22 pattern: + live with the element, die on dispose) and must not leak across reattach. +- Clipboard paths reuse PR #22's guarded helpers (distinct + unavailable/blocked/empty feedback). +- `visualViewport` absent → hook returns null → layout falls back to current + behavior (no crash, desktop unchanged). +- Repeat timers (D-pad) must be cancelled on touchend/touchcancel/dispose. + +## Testing + +- Vitest (run via `npx vitest`, per repo convention) for the pure parts: + gesture controller state machine (long-press vs scroll vs double-tap vs + three-finger disambiguation, speed tiers, cancellation), sticky-ctrl + transform, arrow-sequence selection by cursor mode, touch→cell coordinate + mapping, font-size clamp/persistence logic. +- `pnpm run check` + `pnpm run lint` green. +- Manual smoke on device after merge (Miguel; /opt deploy handled separately + via ship-it, not in this change). + +## Build order + +1. Keyboard-aware viewport + meta tag (issue #1 — biggest pain, standalone). +2. Key bar + sticky ctrl (issue #3). +3. Gesture layer (D-pad, double-tap Tab, three-finger paste). +4. Font steppers. +5. Select mode (riskiest, optional, isolated). From 4b289ad3e20df04d3245a8b75082fdde078e6bc6 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 06:13:08 +0000 Subject: [PATCH 02/11] feat(mobile): size the app to the visual viewport so the keyboard never covers the terminal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit iOS Safari never shrinks the layout viewport when the on-screen keyboard opens, so the fixed inset-0 mobile shell kept its full height and the keyboard sat on top of the terminal's input line. Track window.visualViewport (new useVisualViewportHeight store hook, touch-only, SSR-safe) and drive the mobile shell's height from it; the existing ResizeObserver + fit pipeline then shrinks the PTY/tmux grid so the prompt stays visible. Pin the window scroll back to 0 when iOS pans on focus. Android Chrome gets the declarative fix too: interactive-widget= resizes-content on the viewport meta (local + remote web), which makes the layout viewport itself resize; the hook is a no-op safety net there. Desktop keeps h-screen/grid — the hook returns null off-touch. --- packages/local-web/index.html | 2 +- packages/remote-web/index.html | 2 +- .../ui-new/containers/SharedAppLayout.tsx | 16 +++- .../shared/hooks/useVisualViewportHeight.ts | 76 +++++++++++++++++++ 4 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 packages/web-core/src/shared/hooks/useVisualViewportHeight.ts diff --git a/packages/local-web/index.html b/packages/local-web/index.html index 543f99629b..41e6c16271 100644 --- a/packages/local-web/index.html +++ b/packages/local-web/index.html @@ -7,7 +7,7 @@ - + diff --git a/packages/remote-web/index.html b/packages/remote-web/index.html index 228f40d6df..9d788e5a49 100644 --- a/packages/remote-web/index.html +++ b/packages/remote-web/index.html @@ -2,7 +2,7 @@ - + BetterCoding Remote diff --git a/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx b/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx index e36fe2e955..eca5b75258 100644 --- a/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx +++ b/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx @@ -2,6 +2,7 @@ import { useEffect } from 'react'; import { Outlet } from '@tanstack/react-router'; import { SyncErrorProvider } from '@/shared/providers/SyncErrorProvider'; import { useIsMobile } from '@/shared/hooks/useIsMobile'; +import { useVisualViewportHeight } from '@/shared/hooks/useVisualViewportHeight'; import { useUiPreferencesStore } from '@/shared/stores/useUiPreferencesStore'; import { cn } from '@/shared/lib/utils'; @@ -17,6 +18,7 @@ import { WorkspacesSidebarReopenTag } from '@vibe/ui/components/WorkspacesSideba export function SharedAppLayout() { const currentDestination = useCurrentAppDestination(); const isMobile = useIsMobile(); + const visualViewportHeight = useVisualViewportHeight(); const mobileFontScale = useUiPreferencesStore((s) => s.mobileFontScale); const isLeftSidebarVisible = useUiPreferencesStore( (s) => s.isLeftSidebarVisible @@ -54,9 +56,21 @@ export function SharedAppLayout() { className={cn( 'bg-primary', isMobile - ? 'flex fixed inset-0 pb-[env(safe-area-inset-bottom)]' + ? 'flex fixed inset-x-0 top-0 pb-[env(safe-area-inset-bottom)]' : 'grid grid-rows-[auto_1fr] h-screen' )} + style={ + isMobile + ? { + // Track the VISUAL viewport so the app (and the terminal's + // input line) ends above the on-screen keyboard instead of + // underneath it — iOS never shrinks the layout viewport (see + // useVisualViewportHeight). Falls back to the layout viewport + // when visualViewport is unavailable. + height: visualViewportHeight ?? '100dvh', + } + : undefined + } > {!isMobile && ( <> diff --git a/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts b/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts new file mode 100644 index 0000000000..400fa83842 --- /dev/null +++ b/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts @@ -0,0 +1,76 @@ +import { useSyncExternalStore } from 'react'; + +import { isTouchDevice } from './useIsMobile'; + +/** + * Visible-viewport height tracking for the on-screen keyboard. + * + * iOS Safari never shrinks the layout viewport when the keyboard opens — a + * `fixed inset-0` app keeps its full height and the keyboard just covers the + * bottom half (including the terminal's input line). The only reliable signal + * is `window.visualViewport`, so this store mirrors its height (and pins the + * window scroll back to 0 when iOS pans the page on focus). + * + * Android Chrome resizes the layout viewport itself when the viewport meta + * carries `interactive-widget=resizes-content`; there the visual height equals + * the layout height and applying it is a no-op. + */ + +type Listener = () => void; + +let listeners: Listener[] = []; +let height: number | null = null; +let attached = false; + +function readHeight(): number | null { + const vv = window.visualViewport; + if (!vv) return null; + // Round down so the container never overflows the visible area by a + // sub-pixel (which would put the terminal's last row behind the keyboard). + return Math.floor(vv.height); +} + +function onViewportChange() { + // iOS pans the visual viewport to reveal a focused input near the bottom; + // with the app sized to the visible area there is nothing to reveal, so + // undo the pan to keep the app chrome pinned to the top. + if (window.visualViewport && window.visualViewport.offsetTop > 0) { + window.scrollTo(0, 0); + } + const next = readHeight(); + if (next === height) return; + height = next; + for (const l of [...listeners]) l(); +} + +function subscribe(listener: Listener) { + listeners.push(listener); + const vv = window.visualViewport; + if (vv && !attached) { + attached = true; + height = readHeight(); + vv.addEventListener('resize', onViewportChange); + vv.addEventListener('scroll', onViewportChange); + } + return () => { + listeners = listeners.filter((l) => l !== listener); + // Listeners on visualViewport stay attached for the session — the store is + // module-level and the events are cheap; detaching/reattaching on every + // subscriber churn buys nothing. + }; +} + +function getSnapshot(): number | null { + if (!isTouchDevice()) return null; + if (height === null) height = readHeight(); + return height; +} + +/** + * Current visual viewport height in px on touch devices, or `null` when the + * device isn't touch-capable or `visualViewport` is unavailable — callers fall + * back to their normal (layout-viewport) sizing on `null`. + */ +export function useVisualViewportHeight(): number | null { + return useSyncExternalStore(subscribe, getSnapshot, () => null); +} From 7194e6d1dfb7f43e9b464eed6fd3ab1a24907128 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 06:13:22 +0000 Subject: [PATCH 03/11] feat(mobile): terminal key bar with sticky Ctrl + select toggle + font steppers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Touch-only hotkey row under the terminal (esc ⇥ ⇧⇥ ^C ctrl ← ↓ ↑ → ⏎) — the keys mobile soft keyboards can't type. Arrows follow DECCKM (applicationCursorKeysMode) so they work both at a shell prompt and inside claude/tmux. Buttons preventDefault on pointerdown so the system keyboard stays open while sending. Sticky Ctrl, Termius-style: tapping ctrl latches it (highlighted); the next single typed character is transformed to its control code in the onData→send pipe (pure applyStickyCtrl, unit-tested), then the latch clears. Multi-char bursts (paste/IME) pass through and unlatch. The latch/select flags live in a WeakMap keyed by the Terminal (terminalMobileState): terminals outlive their React components across remount/reattach, so per-mount refs would go stale — this gives the once-per-terminal pipe and the re-mounting React children the same state. TerminalMobileControls gains a select-mode toggle (wired next commit), A−/A+ font steppers (clamped 8–20, persisted, restored on touch devices at terminal creation), and now receives the live terminal + refit callback instead of a ref getter. Focusing the terminal on touch scrolls to the prompt so the keyboard never opens onto stale scrollback. --- .../src/shared/components/TerminalKeyBar.tsx | 120 +++++++++++++++ .../components/TerminalMobileControls.tsx | 140 ++++++++++++++---- .../src/shared/components/XTermInstance.tsx | 87 ++++++++++- .../src/shared/lib/terminalFontSize.test.ts | 31 ++++ .../src/shared/lib/terminalFontSize.ts | 41 +++++ .../shared/lib/terminalKeySequences.test.ts | 79 ++++++++++ .../src/shared/lib/terminalKeySequences.ts | 90 +++++++++++ .../src/shared/lib/terminalMobileState.ts | 75 ++++++++++ 8 files changed, 629 insertions(+), 34 deletions(-) create mode 100644 packages/web-core/src/shared/components/TerminalKeyBar.tsx create mode 100644 packages/web-core/src/shared/lib/terminalFontSize.test.ts create mode 100644 packages/web-core/src/shared/lib/terminalFontSize.ts create mode 100644 packages/web-core/src/shared/lib/terminalKeySequences.test.ts create mode 100644 packages/web-core/src/shared/lib/terminalKeySequences.ts create mode 100644 packages/web-core/src/shared/lib/terminalMobileState.ts diff --git a/packages/web-core/src/shared/components/TerminalKeyBar.tsx b/packages/web-core/src/shared/components/TerminalKeyBar.tsx new file mode 100644 index 0000000000..f3bef5d162 --- /dev/null +++ b/packages/web-core/src/shared/components/TerminalKeyBar.tsx @@ -0,0 +1,120 @@ +import { useCallback, useSyncExternalStore } from 'react'; +import type { Terminal } from '@xterm/xterm'; + +import { cn } from '@/shared/lib/utils'; +import { useIsTouchDevice } from '@/shared/hooks/useIsMobile'; +import { keySequence, type BarKey } from '@/shared/lib/terminalKeySequences'; +import { + getTerminalMobileState, + patchTerminalMobileState, + subscribeTerminalMobileState, +} from '@/shared/lib/terminalMobileState'; + +interface TerminalKeyBarProps { + /** Live terminal (null until mounted) — used for cursor-mode + ctrl latch state. */ + terminal: Terminal | null; + /** Send raw bytes to the PTY (bypasses the sticky-Ctrl transform on typed input). */ + onSendKey: (data: string) => void; +} + +const KEYS: ReadonlyArray<{ key: BarKey; label: string; aria: string }> = [ + { key: 'esc', label: 'esc', aria: 'Escape' }, + { key: 'tab', label: '⇥', aria: 'Tab' }, + { key: 'shift-tab', label: '⇧⇥', aria: 'Shift+Tab' }, + { key: 'ctrl-c', label: '^C', aria: 'Control+C (interrupt)' }, + // 'ctrl' is rendered between ^C and the arrows (special latching button). + { key: 'left', label: '←', aria: 'Arrow left' }, + { key: 'down', label: '↓', aria: 'Arrow down' }, + { key: 'up', label: '↑', aria: 'Arrow up' }, + { key: 'right', label: '→', aria: 'Arrow right' }, + { key: 'enter', label: '⏎', aria: 'Enter' }, +]; + +const KEY_CLASS = + 'flex items-center justify-center min-w-11 h-9 px-2 rounded-md shrink-0 ' + + 'bg-secondary border text-sm text-low active:bg-primary active:text-normal ' + + 'transition-colors select-none'; + +/** + * Termius-style hotkey row for keys mobile soft keyboards can't type: + * Esc / Tab / Shift+Tab / ^C / sticky Ctrl / arrows / Enter. + * + * Touch-only (renders null otherwise) and always visible on touch devices — + * it sits at the bottom of the terminal pane, which the visual-viewport + * sizing keeps directly above the on-screen keyboard while typing. + * + * Buttons preventDefault on pointerdown so a tap never steals focus from + * xterm's textarea — the system keyboard stays open while sending keys. + */ +export function TerminalKeyBar({ terminal, onSendKey }: TerminalKeyBarProps) { + const isTouch = useIsTouchDevice(); + + const subscribe = useCallback( + (cb: () => void) => + terminal ? subscribeTerminalMobileState(terminal, cb) : () => {}, + [terminal] + ); + const ctrlLatched = useSyncExternalStore( + subscribe, + () => (terminal ? getTerminalMobileState(terminal).ctrlLatched : false), + () => false + ); + + if (!isTouch) return null; + + const keepFocus = (e: { preventDefault: () => void }) => e.preventDefault(); + + const sendKey = (key: BarKey) => { + const appCursor = terminal?.modes.applicationCursorKeysMode ?? false; + onSendKey(keySequence(key, appCursor)); + terminal?.scrollToBottom(); + }; + + const toggleCtrl = () => { + if (!terminal) return; + patchTerminalMobileState(terminal, { + ctrlLatched: !getTerminalMobileState(terminal).ctrlLatched, + }); + }; + + const renderKey = (key: BarKey, label: string, aria: string) => ( + + ); + + return ( +
+ {KEYS.slice(0, 4).map(({ key, label, aria }) => + renderKey(key, label, aria) + )} + + {KEYS.slice(4).map(({ key, label, aria }) => renderKey(key, label, aria))} +
+ ); +} diff --git a/packages/web-core/src/shared/components/TerminalMobileControls.tsx b/packages/web-core/src/shared/components/TerminalMobileControls.tsx index 8a5ffa2f7c..8f93221106 100644 --- a/packages/web-core/src/shared/components/TerminalMobileControls.tsx +++ b/packages/web-core/src/shared/components/TerminalMobileControls.tsx @@ -1,8 +1,15 @@ -import { useEffect, useRef, useState } from 'react'; +import { + useCallback, + useEffect, + useRef, + useState, + useSyncExternalStore, +} from 'react'; import type { Terminal } from '@xterm/xterm'; import { CopyIcon, ClipboardTextIcon, + CursorTextIcon, KeyboardIcon, CaretLeftIcon, CaretRightIcon, @@ -11,10 +18,22 @@ import { import { cn } from '@/shared/lib/utils'; import { useIsTouchDevice } from '@/shared/hooks/useIsMobile'; import { extractViewportText } from '@/shared/lib/terminalViewportText'; +import { + clampTerminalFontSize, + saveMobileTerminalFontSize, + TERMINAL_DEFAULT_FONT_SIZE, +} from '@/shared/lib/terminalFontSize'; +import { + getTerminalMobileState, + patchTerminalMobileState, + subscribeTerminalMobileState, +} from '@/shared/lib/terminalMobileState'; interface TerminalMobileControlsProps { - /** Live terminal accessor — refs don't trigger renders, so read on demand. */ - getTerminal: () => Terminal | null; + /** Live terminal (null until the mount effect registers it). */ + terminal: Terminal | null; + /** Re-fit the grid + report the new size to the PTY (font-size steppers). */ + refit: () => void; } const STATUS_MS = 1600; @@ -24,23 +43,36 @@ const BUTTON_CLASS = '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 - * renders nothing unless the device is touch-capable. + * Touch-only Copy / Paste / Select / font-size / 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). + * 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, + terminal, + refit, }: TerminalMobileControlsProps) { const isTouch = useIsTouchDevice(); const [expanded, setExpanded] = useState(true); const [status, setStatus] = useState(null); const statusTimer = useRef | null>(null); + const subscribe = useCallback( + (cb: () => void) => + terminal ? subscribeTerminalMobileState(terminal, cb) : () => {}, + [terminal] + ); + const selectMode = useSyncExternalStore( + subscribe, + () => (terminal ? getTerminalMobileState(terminal).selectMode : false), + () => false + ); + useEffect( () => () => { if (statusTimer.current) clearTimeout(statusTimer.current); @@ -57,12 +89,11 @@ export function TerminalMobileControls({ }; const handleKeyboard = () => { - getTerminal()?.focus(); + terminal?.focus(); }; const handlePaste = async () => { - const term = getTerminal(); - if (!term) return; + if (!terminal) 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) { @@ -75,7 +106,7 @@ export function TerminalMobileControls({ flash('Clipboard empty'); return; } - term.paste(text); + terminal.paste(text); flash('Pasted'); } catch { flash('Paste blocked'); @@ -83,8 +114,7 @@ export function TerminalMobileControls({ }; const handleCopy = async () => { - const term = getTerminal(); - if (!term) return; + if (!terminal) return; // Guard the write API up front so we never flash "Copied" without copying. if (!navigator.clipboard?.writeText) { flash('Copy unavailable'); @@ -92,12 +122,12 @@ export function TerminalMobileControls({ } let text: string; let label: string; - if (term.hasSelection()) { - text = term.getSelection(); + if (terminal.hasSelection()) { + text = terminal.getSelection(); label = 'Copied selection'; } else { - const buf = term.buffer.active; - text = extractViewportText(buf, buf.viewportY, term.rows); + const buf = terminal.buffer.active; + text = extractViewportText(buf, buf.viewportY, terminal.rows); label = 'Copied screen'; } if (!text) { @@ -112,6 +142,28 @@ export function TerminalMobileControls({ } }; + const handleSelectMode = () => { + if (!terminal) return; + const next = !getTerminalMobileState(terminal).selectMode; + patchTerminalMobileState(terminal, { selectMode: next }); + if (!next) terminal.clearSelection(); + flash(next ? 'Select mode — drag to select' : 'Select mode off'); + }; + + const stepFontSize = (delta: number) => { + if (!terminal) return; + const current = terminal.options.fontSize ?? TERMINAL_DEFAULT_FONT_SIZE; + const next = clampTerminalFontSize(current + delta); + if (next === current) { + flash(`Font ${current}px (limit)`); + return; + } + terminal.options.fontSize = next; + saveMobileTerminalFontSize(next); + refit(); + flash(`Font ${next}px`); + }; + const actions = [ { label: 'Copy from terminal', Icon: CopyIcon, onClick: handleCopy }, { @@ -146,18 +198,50 @@ export function TerminalMobileControls({ {status} )} - {expanded && - actions.map(({ label, Icon, onClick }) => ( + {expanded && ( + <> + {actions.map(({ label, Icon, onClick }) => ( + + ))} + - ))} + + + )} ); @@ -97,9 +119,7 @@ export function TerminalKeyBar({ terminal, onSendKey }: TerminalKeyBarProps) { role="toolbar" aria-label="Terminal keys" > - {KEYS.slice(0, 4).map(({ key, label, aria }) => - renderKey(key, label, aria) - )} + {KEYS.slice(0, 4).map(renderKey)} - {KEYS.slice(4).map(({ key, label, aria }) => renderKey(key, label, aria))} + {KEYS.slice(4).map(renderKey)} ); } From ae26581ecd9b5e81bbd97b858192810c3cc50122 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 06:34:47 +0000 Subject: [PATCH 06/11] fix(mobile): keep inset-0 fallback; height override only when visualViewport is live Avoids depending on dvh support: with no visualViewport (or no touch) the class alone reproduces the exact pre-existing fixed inset-0 behavior; the explicit height + bottom:auto only kick in when the hook has a real value. --- .../ui-new/containers/SharedAppLayout.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx b/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx index eca5b75258..325466d47d 100644 --- a/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx +++ b/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx @@ -56,19 +56,18 @@ export function SharedAppLayout() { className={cn( 'bg-primary', isMobile - ? 'flex fixed inset-x-0 top-0 pb-[env(safe-area-inset-bottom)]' + ? 'flex fixed inset-0 pb-[env(safe-area-inset-bottom)]' : 'grid grid-rows-[auto_1fr] h-screen' )} style={ - isMobile - ? { - // Track the VISUAL viewport so the app (and the terminal's - // input line) ends above the on-screen keyboard instead of - // underneath it — iOS never shrinks the layout viewport (see - // useVisualViewportHeight). Falls back to the layout viewport - // when visualViewport is unavailable. - height: visualViewportHeight ?? '100dvh', - } + // Track the VISUAL viewport so the app (and the terminal's input + // line) ends above the on-screen keyboard instead of underneath it — + // iOS never shrinks the layout viewport (see useVisualViewportHeight). + // The explicit height wins over the class's `bottom: 0`; when the + // hook has no value (no touch / no visualViewport) the class alone + // keeps the exact pre-existing inset-0 behavior. + isMobile && visualViewportHeight !== null + ? { height: visualViewportHeight, bottom: 'auto' } : undefined } > From 9ea29b136a4f7ccca1c708b9125d1b4681771d25 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 06:46:40 +0000 Subject: [PATCH 07/11] fix(mobile): apply council round-1 + security review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Sticky Ctrl now acts on KEYSTROKES only: an onKey-set flag marks the next onData chunk as keyboard-originated; pastes / IME commits / terminal query replies (DSR/DA) are never transformed and never consume the latch. Key-bar taps consume the latch (one-keystroke contract). toCtrlChar is ASCII-only — String.toUpperCase would map ß→SS→^S (XOFF freeze). - Three-finger paste: same guarded clipboard path + explicit status feedback as the Paste button (flash channel through the terminal mobile state) — no more silent clipboard read; gesture+selection installers gated on isTouchDevice() so non-touch sessions carry zero new listeners. - Select mode copies ONCE on release; the per-change auto-copy is suppressed while select mode is active (a drag no longer hammers the clipboard with dozens of intermediate writes / destroys the user's clipboard on touchstart). - D-pad promotion no longer depends on setTimeout: a starved timer promotes synchronously inside onTouchMove (before the slop check), and the gesture layer is attached BEFORE the scroll bridge so same-event suppression holds. - cancelActiveTerminalGesture(terminal): React unmount aborts an in-flight D-pad (repeat timer + dpadActive suppression can't outlive detach). - visualViewport store: listeners only attach on touch devices; the scrollTo(0,0) pin is touch-gated (desktop pinch-zoom is never fought). 64 unit tests green (new: timer starvation, cancel, non-ASCII ctrl). --- .../src/shared/components/TerminalKeyBar.tsx | 5 + .../components/TerminalMobileControls.tsx | 19 ++- .../src/shared/components/XTermInstance.tsx | 113 ++++++++++++------ .../shared/hooks/useVisualViewportHeight.ts | 8 +- .../shared/lib/terminalKeySequences.test.ts | 9 ++ .../src/shared/lib/terminalKeySequences.ts | 5 +- .../src/shared/lib/terminalMobileState.ts | 42 +++++-- .../shared/lib/terminalTouchGestures.test.ts | 37 ++++++ .../src/shared/lib/terminalTouchGestures.ts | 52 ++++++-- .../src/shared/lib/terminalTouchSelection.ts | 16 +++ 10 files changed, 244 insertions(+), 62 deletions(-) diff --git a/packages/web-core/src/shared/components/TerminalKeyBar.tsx b/packages/web-core/src/shared/components/TerminalKeyBar.tsx index 7523c0fdd2..b3231645c6 100644 --- a/packages/web-core/src/shared/components/TerminalKeyBar.tsx +++ b/packages/web-core/src/shared/components/TerminalKeyBar.tsx @@ -85,6 +85,11 @@ export function TerminalKeyBar({ terminal, onSendKey }: TerminalKeyBarProps) { const sendKey = (key: BarKey) => { const appCursor = terminal?.modes.applicationCursorKeysMode ?? false; onSendKey(keySequence(key, appCursor)); + // A bar tap counts as the latch's "one keystroke" — otherwise ctrl, + // arrow, then a typed letter would surprise-fire a control combo. + if (terminal && getTerminalMobileState(terminal).ctrlLatched) { + patchTerminalMobileState(terminal, { ctrlLatched: false }); + } terminal?.scrollToBottom(); }; diff --git a/packages/web-core/src/shared/components/TerminalMobileControls.tsx b/packages/web-core/src/shared/components/TerminalMobileControls.tsx index 8f93221106..1c24ade5ee 100644 --- a/packages/web-core/src/shared/components/TerminalMobileControls.tsx +++ b/packages/web-core/src/shared/components/TerminalMobileControls.tsx @@ -72,6 +72,11 @@ export function TerminalMobileControls({ () => (terminal ? getTerminalMobileState(terminal).selectMode : false), () => false ); + const installerFlash = useSyncExternalStore( + subscribe, + () => (terminal ? getTerminalMobileState(terminal).flash : null), + () => null + ); useEffect( () => () => { @@ -80,13 +85,19 @@ export function TerminalMobileControls({ [] ); - if (!isTouch) return null; - - const flash = (msg: string) => { + const flash = useCallback((msg: string) => { setStatus(msg); if (statusTimer.current) clearTimeout(statusTimer.current); statusTimer.current = setTimeout(() => setStatus(null), STATUS_MS); - }; + }, []); + + // Surface one-shot status messages emitted by the DOM-level installers + // (e.g. the three-finger paste gesture) in the same flash pill. + useEffect(() => { + if (installerFlash) flash(installerFlash.message); + }, [installerFlash, flash]); + + if (!isTouch) return null; const handleKeyboard = () => { terminal?.focus(); diff --git a/packages/web-core/src/shared/components/XTermInstance.tsx b/packages/web-core/src/shared/components/XTermInstance.tsx index 09b7440e51..1fe53ad490 100644 --- a/packages/web-core/src/shared/components/XTermInstance.tsx +++ b/packages/web-core/src/shared/components/XTermInstance.tsx @@ -10,13 +10,17 @@ import { } from '@/shared/lib/terminalTheme'; import { buildTerminalWsUrl } from '@/shared/lib/terminalWsUrl'; import { installTerminalTouchScroll } from '@/shared/lib/terminalTouchScroll'; -import { installTerminalTouchGestures } from '@/shared/lib/terminalTouchGestures'; +import { + cancelActiveTerminalGesture, + installTerminalTouchGestures, +} from '@/shared/lib/terminalTouchGestures'; import { installTerminalTouchSelection } from '@/shared/lib/terminalTouchSelection'; import { applyStickyCtrl, keySequence, } from '@/shared/lib/terminalKeySequences'; import { + flashTerminalMobileStatus, getTerminalMobileState, patchTerminalMobileState, } from '@/shared/lib/terminalMobileState'; @@ -168,13 +172,21 @@ export function XTermInstance({ registerTerminalInstance(tabId, terminal, fitAddon); + // Sticky Ctrl (mobile key bar) must act on KEYSTROKES only. onData also + // carries pastes and terminal query replies (DSR/DA), which must neither + // be transformed into control codes nor silently consume an armed latch. + // onKey fires right before the onData chunk a keystroke produces, so it + // marks the next chunk as keyboard-originated. + let nextChunkFromKeyboard = false; + terminal.onKey(() => { + nextChunkFromKeyboard = true; + }); terminal.onData((data) => { + const fromKeyboard = nextChunkFromKeyboard; + nextChunkFromKeyboard = false; const conn = getTerminalConnection(tabId); if (!conn) return; - // Sticky Ctrl (mobile key bar): a latched ctrl turns the next single - // typed character into its control code. The latch always clears on - // the next chunk — multi-char bursts (paste/IME) pass through as-is. - if (getTerminalMobileState(terminal).ctrlLatched) { + if (fromKeyboard && getTerminalMobileState(terminal).ctrlLatched) { patchTerminalMobileState(terminal, { ctrlLatched: false }); conn.send(applyStickyCtrl(data).out); return; @@ -188,6 +200,11 @@ export function XTermInstance({ // terminal and live exactly as long as it does (the listener dies with // the element on dispose, the selection hook with the terminal). terminal.onSelectionChange(() => { + // Touch select mode fires a selection change per touchmove — copying + // here would hammer the clipboard with intermediate selections (and + // destroy whatever the user had copied the moment a drag starts). + // That mode copies exactly once on release (terminalTouchSelection). + if (getTerminalMobileState(terminal).selectMode) return; const text = terminal.getSelection(); if (text) { void navigator.clipboard?.writeText(text).catch(() => { @@ -218,46 +235,62 @@ export function XTermInstance({ } }); + // Touch gesture layer (long-press D-pad → arrows, double-tap → Tab, + // three-finger tap → paste) and select-mode drag selection. Same + // lifetime rule as the scroll bridge below: attach once, dies with the + // element. Gated on touch capability so a non-touch session carries + // zero new listeners — and the paste gesture cannot exist there. + // Attached BEFORE the scroll bridge on purpose: listener order is + // attach order, so when a starved long-press promotes to D-pad inside + // a touchmove, the suppression flag is already set by the time the + // scroll bridge sees that same event. + if (isTouchDevice()) { + const t = terminal; + const sendRaw = (data: string) => + getTerminalConnection(tabId)?.send(data); + installTerminalTouchGestures(t, { + sendArrow: (dir) => + sendRaw(keySequence(dir, t.modes.applicationCursorKeysMode)), + sendTab: () => sendRaw('\t'), + paste: () => { + // Same guarded clipboard path + explicit feedback as the Paste + // button — a silent clipboard read would be a pastejacking aid. + if (!navigator.clipboard?.readText) { + flashTerminalMobileStatus(t, 'Paste unavailable'); + return; + } + void navigator.clipboard + .readText() + .then((text) => { + if (!text) { + flashTerminalMobileStatus(t, 'Clipboard empty'); + return; + } + t.paste(text); + flashTerminalMobileStatus(t, 'Pasted'); + }) + .catch(() => { + flashTerminalMobileStatus(t, 'Paste blocked'); + }); + }, + }); + installTerminalTouchSelection(t); + + // Keyboard-open ergonomics: focusing the terminal on a touch device + // pops the system keyboard — jump to the prompt so it isn't hidden + // in scrollback while the viewport shrinks around it. + t.textarea?.addEventListener('focus', () => t.scrollToBottom()); + } + // 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. + // (Kept unconditional like PR #22 — inert without touch events — but + // attached AFTER the gesture layer; see the ordering note above.) installTerminalTouchScroll(terminal); - - // Touch gesture layer (long-press D-pad → arrows, double-tap → Tab, - // three-finger tap → paste) and select-mode drag selection. Same - // lifetime rule as the scroll bridge: attach once, dies with the - // element. All are inert without touch input, so no capability gate. - const t = terminal; - const sendRaw = (data: string) => - getTerminalConnection(tabId)?.send(data); - installTerminalTouchGestures(t, { - sendArrow: (dir) => - sendRaw(keySequence(dir, t.modes.applicationCursorKeysMode)), - sendTab: () => sendRaw('\t'), - paste: () => { - if (!navigator.clipboard?.readText) return; - void navigator.clipboard - .readText() - .then((text) => { - if (text) t.paste(text); - }) - .catch(() => { - // Paste permission denied — the Paste button reports errors; - // the gesture stays silent. - }); - }, - }); - installTerminalTouchSelection(t); - - // Keyboard-open ergonomics: focusing the terminal on a touch device - // pops the system keyboard — jump to the prompt so it isn't hidden - // in scrollback while the viewport shrinks around it. - t.textarea?.addEventListener('focus', () => { - if (isTouchDevice()) t.scrollToBottom(); - }); } terminalRef.current = terminal; @@ -286,6 +319,10 @@ export function XTermInstance({ } return () => { + // A D-pad gesture running right now would never see its touchend once + // the element leaves the DOM — stop its repeat timer and release the + // scroll-bridge suppression before detaching. + cancelActiveTerminalGesture(terminal); if (terminal.element && terminal.element.parentNode) { terminal.element.parentNode.removeChild(terminal.element); } diff --git a/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts b/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts index 400fa83842..97739b243a 100644 --- a/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts +++ b/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts @@ -33,7 +33,9 @@ function readHeight(): number | null { function onViewportChange() { // iOS pans the visual viewport to reveal a focused input near the bottom; // with the app sized to the visible area there is nothing to reveal, so - // undo the pan to keep the app chrome pinned to the top. + // undo the pan to keep the app chrome pinned to the top. Touch-only: on a + // desktop, offsetTop > 0 means the USER pinch-zoomed — never fight that. + if (!isTouchDevice()) return; if (window.visualViewport && window.visualViewport.offsetTop > 0) { window.scrollTo(0, 0); } @@ -46,7 +48,9 @@ function onViewportChange() { function subscribe(listener: Listener) { listeners.push(listener); const vv = window.visualViewport; - if (vv && !attached) { + // Never attach on non-touch devices — the hook always snapshots null there, + // so listening would only risk desktop side effects for no consumer. + if (vv && !attached && isTouchDevice()) { attached = true; height = readHeight(); vv.addEventListener('resize', onViewportChange); diff --git a/packages/web-core/src/shared/lib/terminalKeySequences.test.ts b/packages/web-core/src/shared/lib/terminalKeySequences.test.ts index 1e286efbf6..53d604e6ee 100644 --- a/packages/web-core/src/shared/lib/terminalKeySequences.test.ts +++ b/packages/web-core/src/shared/lib/terminalKeySequences.test.ts @@ -77,3 +77,12 @@ describe('applyStickyCtrl', () => { }); }); }); + +describe('toCtrlChar — non-ASCII safety (council round 1)', () => { + it('never maps non-ASCII letters (toUpperCase would turn ß into SS → ^S)', () => { + expect(toCtrlChar('ß')).toBeNull(); + expect(toCtrlChar('é')).toBeNull(); + expect(toCtrlChar('ñ')).toBeNull(); + expect(toCtrlChar('～')).toBeNull(); + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalKeySequences.ts b/packages/web-core/src/shared/lib/terminalKeySequences.ts index b1892fdcd4..a645f34716 100644 --- a/packages/web-core/src/shared/lib/terminalKeySequences.ts +++ b/packages/web-core/src/shared/lib/terminalKeySequences.ts @@ -62,7 +62,10 @@ export function toCtrlChar(ch: string): string | null { if (ch.length !== 1) return null; if (ch === ' ') return '\x00'; if (ch === '?') return '\x7f'; - const code = ch.toUpperCase().charCodeAt(0); + // ASCII-only: String.toUpperCase would map ß→SS (→ ^S, XOFF freeze!) and + // other locale surprises, so uppercase by code point arithmetic instead. + let code = ch.charCodeAt(0); + if (code >= 0x61 && code <= 0x7a) code -= 0x20; // a-z → A-Z // @ A-Z [ \ ] ^ _ → 0x00-0x1f if (code >= 0x40 && code <= 0x5f) return String.fromCharCode(code & 0x1f); return null; diff --git a/packages/web-core/src/shared/lib/terminalMobileState.ts b/packages/web-core/src/shared/lib/terminalMobileState.ts index a174cf8981..138ab1b4f6 100644 --- a/packages/web-core/src/shared/lib/terminalMobileState.ts +++ b/packages/web-core/src/shared/lib/terminalMobileState.ts @@ -18,6 +18,24 @@ export interface TerminalMobileState { selectMode: boolean; /** A long-press D-pad gesture is running; the scroll bridge stands down. */ dpadActive: boolean; + /** + * One-shot status message from a DOM-level installer (e.g. the paste + * gesture) for the React controls to flash. The nonce makes every emission + * a distinct value so repeated identical messages still notify. + */ + flash: { nonce: number; message: string } | null; +} + +let flashNonce = 0; + +/** Emit a one-shot status flash for this terminal's mobile controls. */ +export function flashTerminalMobileStatus( + terminal: Terminal, + message: string +): void { + patchTerminalMobileState(terminal, { + flash: { nonce: ++flashNonce, message }, + }); } interface Entry { @@ -31,7 +49,12 @@ function entryFor(terminal: Terminal): Entry { let entry = entries.get(terminal); if (!entry) { entry = { - state: { ctrlLatched: false, selectMode: false, dpadActive: false }, + state: { + ctrlLatched: false, + selectMode: false, + dpadActive: false, + flash: null, + }, listeners: new Set(), }; entries.set(terminal, entry); @@ -50,18 +73,17 @@ export function patchTerminalMobileState( patch: Partial ): void { const entry = entryFor(terminal); - let changed = false; - for (const key of Object.keys(patch) as (keyof TerminalMobileState)[]) { - const value = patch[key]; - if (value !== undefined && entry.state[key] !== value) { - entry.state[key] = value; - changed = true; - } - } + // Drop explicit-undefined keys so a sloppy caller can't blank a field. + const defined = Object.fromEntries( + Object.entries(patch).filter(([, value]) => value !== undefined) + ) as Partial; + const changed = (Object.keys(defined) as (keyof TerminalMobileState)[]).some( + (key) => entry.state[key] !== defined[key] + ); if (!changed) return; // Snapshot object identity changes only on real transitions so React's // useSyncExternalStore consumers don't re-render on no-op patches. - entry.state = { ...entry.state }; + entry.state = { ...entry.state, ...defined }; for (const listener of [...entry.listeners]) listener(); } diff --git a/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts b/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts index 3a5845f42d..bde16b1635 100644 --- a/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts +++ b/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts @@ -244,3 +244,40 @@ describe('select mode (disabled layer)', () => { expect(dpad).toEqual([]); }); }); + +describe('timer starvation + cancellation (council round 1)', () => { + it('promotes to D-pad inside onTouchMove when the timer never fired', () => { + const { ctrl, arrows, dpad } = harness(); + ctrl.onTouchStart(p(100, 100), 0); + // No onTimer call at all — simulates a starved main thread. First move + // arrives well past the long-press threshold. + const r = ctrl.onTouchMove( + p(100, 100 + DPAD_DEAD_ZONE_PX + 10), + LONG_PRESS_MS + 200 + ); + expect(r.prevent).toBe(true); + expect(dpad[0]).toEqual({ active: true, dir: null }); + expect(arrows).toEqual(['down']); + }); + + it('still hands an early move to the scroll bridge', () => { + const { ctrl, dpad } = harness(); + ctrl.onTouchStart(p(100, 100), 0); + const r = ctrl.onTouchMove(p(100, 160), LONG_PRESS_MS - 50); + expect(r.prevent).toBe(false); + expect(dpad).toEqual([]); + }); + + it('cancel() releases an active D-pad and stops repeats', () => { + const { ctrl, dpad } = harness(); + ctrl.onTouchStart(p(100, 100), 0); + runTimersUntil(ctrl, LONG_PRESS_MS); + ctrl.onTouchMove(p(100, 150), LONG_PRESS_MS + 10); + ctrl.cancel(); + expect(dpad.at(-1)).toEqual({ active: false, dir: null }); + expect(ctrl.nextTimerAt()).toBeNull(); + // A stray touchend after cancel is a no-op. + ctrl.onTouchEnd(0, LONG_PRESS_MS + 100); + expect(dpad.at(-1)).toEqual({ active: false, dir: null }); + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalTouchGestures.ts b/packages/web-core/src/shared/lib/terminalTouchGestures.ts index 713d5a9ce3..9fb82dbbae 100644 --- a/packages/web-core/src/shared/lib/terminalTouchGestures.ts +++ b/packages/web-core/src/shared/lib/terminalTouchGestures.ts @@ -122,14 +122,25 @@ export function createTouchGestureController(deps: GestureDeps) { onTouchMove(p: GesturePoint, now: number): GestureMoveResult { if (phase === 'pressed') { - const moved = - Math.abs(p.clientX - startX) > TAP_SLOP_PX || - Math.abs(p.clientY - startY) > TAP_SLOP_PX; - if (moved) { - // Moving before the long-press delay = scroll; the bridge owns it. - phase = 'ignored'; + if (now - pressAt >= LONG_PRESS_MS) { + // The promotion timer can be starved by a busy main thread (TUI + // redraw, keyboard resize). The finger DID dwell long enough, so + // promote here — before the slop check — or the first delayed + // move would hand a held press to the scroll bridge. + phase = 'dpad'; + dir = null; + deps.setDpad(true, null); + // fall through to D-pad handling of this same move + } else { + const moved = + Math.abs(p.clientX - startX) > TAP_SLOP_PX || + Math.abs(p.clientY - startY) > TAP_SLOP_PX; + if (moved) { + // Moving before the long-press delay = scroll; the bridge owns it. + phase = 'ignored'; + } + return { prevent: false }; } - return { prevent: false }; } if (phase !== 'dpad') return { prevent: false }; @@ -210,6 +221,18 @@ export function createTouchGestureController(deps: GestureDeps) { if (phase === 'dpad' && dir) return nextRepeatAt; return null; }, + + /** + * Abort whatever is in flight (React detach mid-gesture: touchend may + * never arrive once the element leaves the DOM). Releases the D-pad — + * clearing the scroll-bridge suppression — and stops key repeats. + */ + cancel(): void { + exitDpad(); + phase = 'idle'; + maxTouches = 0; + hasLastTap = false; + }, }; } @@ -219,6 +242,15 @@ export interface InstallGestureOptions { paste: () => void; } +// Live gesture cancellers, one per terminal. React unmount cleanup calls +// cancelActiveTerminalGesture so a D-pad running at detach time can't keep +// its repeat timer (and the scroll-bridge suppression) alive. +const activeGestureCancels = new WeakMap void>(); + +export function cancelActiveTerminalGesture(terminal: Terminal): void { + activeGestureCancels.get(terminal)?.(); +} + /** * Bind the gesture controller to a live terminal element. Like the scroll * bridge: attach once per created terminal; listeners live and die with the @@ -312,8 +344,14 @@ export function installTerminalTouchGestures( el.addEventListener('touchend', onEnd, { passive: true }); el.addEventListener('touchcancel', onEnd, { passive: true }); + activeGestureCancels.set(terminal, () => { + controller.cancel(); + reschedule(); // nextTimerAt() is now null → clears the pending timer + }); + return () => { if (timer) clearTimeout(timer); + activeGestureCancels.delete(terminal); el.removeEventListener('touchstart', onStart); el.removeEventListener('touchmove', onMove); el.removeEventListener('touchend', onEnd); diff --git a/packages/web-core/src/shared/lib/terminalTouchSelection.ts b/packages/web-core/src/shared/lib/terminalTouchSelection.ts index d4850d60c3..e21bfefb92 100644 --- a/packages/web-core/src/shared/lib/terminalTouchSelection.ts +++ b/packages/web-core/src/shared/lib/terminalTouchSelection.ts @@ -108,6 +108,22 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { }; const onEnd = () => { + // Copy exactly once, on release — the per-change auto-copy is suppressed + // in select mode so a drag doesn't overwrite the clipboard dozens of + // times with intermediate selections. + if ( + anchor !== null && + getTerminalMobileState(terminal).selectMode && + terminal.hasSelection() + ) { + const text = terminal.getSelection(); + if (text) { + void navigator.clipboard?.writeText(text).catch(() => { + // Clipboard can be blocked; the selection itself still stands and + // the Copy button reports errors explicitly. + }); + } + } anchor = null; }; From 99065ff519ef0b5758528039ce3a90396cd5df39 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 07:04:38 +0000 Subject: [PATCH 08/11] refactor(mobile): apply simplify pass + code-review round-1 fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctness (from 6 finder angles + codex + security cross-check): - Sticky Ctrl provenance now uses a paste marker instead of an onKey flag: Android IME (Gboard) commits text via composition with NO key event, so the flag left the latch dead on the PR's own target platform. New rule: a single-char onData chunk transforms unless a paste is in flight (pasteTextIntoTerminal brackets every app paste path with a WeakSet marker; xterm's paste() emits synchronously). Query replies (DSR/DA) are multi-char and pass through; gesture arrows/Tab and key-bar taps consume the latch (one-keystroke contract everywhere). - Gesture controller: coalesced multi-finger touchstart (all fingers in one event) now initializes pressAt/start coords and respects the select-mode gate — three-finger tap actually fires on iOS and can't leak through select mode; a travelling three-finger gesture (iPadOS system swipes) no longer counts as a tap; touchcancel is a cancel, never a tap/paste; adapter uses event timestamps + performance.now() so a busy main thread can't misread a late-delivered swipe as a dwell and NTP steps can't stall D-pad repeats; cancel() wipes double-tap history. - Select mode: capture-phase listeners + stopPropagation keep xterm's own viewport touch scrolling (plain shells) from scrolling under a selection drag; touchToCell returns null on degenerate rects instead of feeding NaN to terminal.select(); copy-on-release goes through writeClipboardViaBridge (VSCode-iframe fallback); terminal.select() only runs when the target cell changed. - Viewport store: pinch-zoom (visualViewport.scale > 1) now reports null — the app keeps its full-size layout and the scroll pin never fights a zoom pan; notifications are rAF-coalesced; keyboard-height override now applies on BOTH layout branches so touch tablets (>767px) get it too. - Unmount resets selectMode/ctrlLatched: modes no longer silently survive a pane close/reopen against the registry-persisted terminal. - Key bar: role=group (toolbar ARIA contract needs roving focus); D-pad overlay uses +/plain arrows (the fancy glyphs tofu on Android). Simplification (4 cleanup angles): - pasteIntoTerminal: one guarded, feedback-carrying paste implementation behind the Paste button, the three-finger gesture, and right-click paste. - installTerminalTouchLayers encodes the gestures→selection→scroll attach order as code instead of a prose comment in the component. - Flash channel is now an event subscription (no retained flash state — a stale 'Pasted' pill can't replay on remount), useTerminalMobileState dedupes the subscription boilerplate, KEYS is one array with ctrl in place (no slice(0,4) magic), focus guards live on the container, isTouchDevice is memoized, gesture timer reschedules only on deadline change, lastTap is one nullable object. 75 unit tests green (new: terminalMobileState store/flash contract, coalesced multi-touch, swipe-not-tap, cancel semantics, degenerate rects). --- .../src/shared/components/TerminalKeyBar.tsx | 89 +++++----- .../components/TerminalMobileControls.tsx | 53 ++---- .../src/shared/components/XTermInstance.tsx | 129 +++++--------- .../ui-new/containers/SharedAppLayout.tsx | 11 +- .../web-core/src/shared/hooks/useIsMobile.ts | 15 +- .../shared/hooks/useVisualViewportHeight.ts | 32 +++- .../shared/lib/terminalMobileState.test.ts | 79 +++++++++ .../src/shared/lib/terminalMobileState.ts | 87 +++++---- .../web-core/src/shared/lib/terminalPaste.ts | 52 ++++++ .../shared/lib/terminalTouchGestures.test.ts | 38 ++++ .../src/shared/lib/terminalTouchGestures.ts | 167 ++++++++++++------ .../src/shared/lib/terminalTouchLayers.ts | 67 +++++++ .../shared/lib/terminalTouchSelection.test.ts | 10 ++ .../src/shared/lib/terminalTouchSelection.ts | 89 +++++++--- 14 files changed, 615 insertions(+), 303 deletions(-) create mode 100644 packages/web-core/src/shared/lib/terminalMobileState.test.ts create mode 100644 packages/web-core/src/shared/lib/terminalPaste.ts create mode 100644 packages/web-core/src/shared/lib/terminalTouchLayers.ts diff --git a/packages/web-core/src/shared/components/TerminalKeyBar.tsx b/packages/web-core/src/shared/components/TerminalKeyBar.tsx index b3231645c6..6a16ff8685 100644 --- a/packages/web-core/src/shared/components/TerminalKeyBar.tsx +++ b/packages/web-core/src/shared/components/TerminalKeyBar.tsx @@ -1,4 +1,3 @@ -import { useCallback, useSyncExternalStore } from 'react'; import type { Terminal } from '@xterm/xterm'; import type { Icon } from '@phosphor-icons/react'; import { @@ -17,7 +16,7 @@ import { keySequence, type BarKey } from '@/shared/lib/terminalKeySequences'; import { getTerminalMobileState, patchTerminalMobileState, - subscribeTerminalMobileState, + useTerminalMobileState, } from '@/shared/lib/terminalMobileState'; interface TerminalKeyBarProps { @@ -29,9 +28,10 @@ interface TerminalKeyBarProps { // Icons instead of key-glyph characters (⇥ ⇧⇥ ⏎): those codepoints are // missing from Android/Linux system fonts and render as tofu boxes; the -// phosphor icons render identically everywhere. +// phosphor icons render identically everywhere. 'ctrl' is the latching +// modifier button, rendered specially at its position in this list. const KEYS: ReadonlyArray<{ - key: BarKey; + key: BarKey | 'ctrl'; label?: string; Icon?: Icon; aria: string; @@ -40,7 +40,11 @@ const KEYS: ReadonlyArray<{ { key: 'tab', Icon: ArrowLineRightIcon, aria: 'Tab' }, { key: 'shift-tab', Icon: ArrowLineLeftIcon, aria: 'Shift+Tab' }, { key: 'ctrl-c', label: '^C', aria: 'Control+C (interrupt)' }, - // 'ctrl' is rendered between ^C and the arrows (special latching button). + { + key: 'ctrl', + label: 'ctrl', + aria: 'Control (sticky — next letter sends the combo)', + }, { key: 'left', Icon: ArrowLeftIcon, aria: 'Arrow left' }, { key: 'down', Icon: ArrowDownIcon, aria: 'Arrow down' }, { key: 'up', Icon: ArrowUpIcon, aria: 'Arrow up' }, @@ -66,17 +70,7 @@ const KEY_CLASS = */ export function TerminalKeyBar({ terminal, onSendKey }: TerminalKeyBarProps) { const isTouch = useIsTouchDevice(); - - const subscribe = useCallback( - (cb: () => void) => - terminal ? subscribeTerminalMobileState(terminal, cb) : () => {}, - [terminal] - ); - const ctrlLatched = useSyncExternalStore( - subscribe, - () => (terminal ? getTerminalMobileState(terminal).ctrlLatched : false), - () => false - ); + const { ctrlLatched } = useTerminalMobileState(terminal); if (!isTouch) return null; @@ -100,46 +94,43 @@ export function TerminalKeyBar({ terminal, onSendKey }: TerminalKeyBarProps) { }); }; - const renderKey = ({ key, label, Icon, aria }: (typeof KEYS)[number]) => ( - - ); - - return ( -
- {KEYS.slice(0, 4).map(renderKey)} + const renderKey = ({ key, label, Icon, aria }: (typeof KEYS)[number]) => { + const isCtrl = key === 'ctrl'; + return ( - {KEYS.slice(4).map(renderKey)} + ); + }; + + return ( + // preventDefault on pointer/mouse down at the toolbar level (events + // bubble) so no tap steals focus from xterm's textarea — the system + // keyboard stays open while sending keys. + // role="group", not "toolbar": the ARIA toolbar contract requires roving + // tabindex + arrow-key traversal, and arrow keys here are CONTENT. +
+ {KEYS.map(renderKey)}
); } diff --git a/packages/web-core/src/shared/components/TerminalMobileControls.tsx b/packages/web-core/src/shared/components/TerminalMobileControls.tsx index 1c24ade5ee..9457adc703 100644 --- a/packages/web-core/src/shared/components/TerminalMobileControls.tsx +++ b/packages/web-core/src/shared/components/TerminalMobileControls.tsx @@ -1,10 +1,4 @@ -import { - useCallback, - useEffect, - useRef, - useState, - useSyncExternalStore, -} from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import type { Terminal } from '@xterm/xterm'; import { CopyIcon, @@ -26,8 +20,10 @@ import { import { getTerminalMobileState, patchTerminalMobileState, - subscribeTerminalMobileState, + subscribeTerminalMobileFlash, + useTerminalMobileState, } from '@/shared/lib/terminalMobileState'; +import { pasteIntoTerminal } from '@/shared/lib/terminalPaste'; interface TerminalMobileControlsProps { /** Live terminal (null until the mount effect registers it). */ @@ -62,21 +58,7 @@ export function TerminalMobileControls({ const [status, setStatus] = useState(null); const statusTimer = useRef | null>(null); - const subscribe = useCallback( - (cb: () => void) => - terminal ? subscribeTerminalMobileState(terminal, cb) : () => {}, - [terminal] - ); - const selectMode = useSyncExternalStore( - subscribe, - () => (terminal ? getTerminalMobileState(terminal).selectMode : false), - () => false - ); - const installerFlash = useSyncExternalStore( - subscribe, - () => (terminal ? getTerminalMobileState(terminal).flash : null), - () => null - ); + const { selectMode } = useTerminalMobileState(terminal); useEffect( () => () => { @@ -94,8 +76,9 @@ export function TerminalMobileControls({ // Surface one-shot status messages emitted by the DOM-level installers // (e.g. the three-finger paste gesture) in the same flash pill. useEffect(() => { - if (installerFlash) flash(installerFlash.message); - }, [installerFlash, flash]); + if (!terminal) return; + return subscribeTerminalMobileFlash(terminal, flash); + }, [terminal, flash]); if (!isTouch) return null; @@ -103,25 +86,9 @@ export function TerminalMobileControls({ terminal?.focus(); }; - const handlePaste = async () => { + const handlePaste = () => { if (!terminal) 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(); - if (!text) { - flash('Clipboard empty'); - return; - } - terminal.paste(text); - flash('Pasted'); - } catch { - flash('Paste blocked'); - } + void pasteIntoTerminal(terminal, flash); }; const handleCopy = async () => { diff --git a/packages/web-core/src/shared/components/XTermInstance.tsx b/packages/web-core/src/shared/components/XTermInstance.tsx index 1fe53ad490..67fa9d80aa 100644 --- a/packages/web-core/src/shared/components/XTermInstance.tsx +++ b/packages/web-core/src/shared/components/XTermInstance.tsx @@ -9,22 +9,21 @@ import { getTerminalTheme, } from '@/shared/lib/terminalTheme'; import { buildTerminalWsUrl } from '@/shared/lib/terminalWsUrl'; -import { installTerminalTouchScroll } from '@/shared/lib/terminalTouchScroll'; +import { cancelActiveTerminalGesture } from '@/shared/lib/terminalTouchGestures'; +import { installTerminalTouchLayers } from '@/shared/lib/terminalTouchLayers'; +import { applyStickyCtrl } from '@/shared/lib/terminalKeySequences'; import { - cancelActiveTerminalGesture, - installTerminalTouchGestures, -} from '@/shared/lib/terminalTouchGestures'; -import { installTerminalTouchSelection } from '@/shared/lib/terminalTouchSelection'; + isTerminalPasting, + pasteTextIntoTerminal, +} from '@/shared/lib/terminalPaste'; import { - applyStickyCtrl, - keySequence, -} from '@/shared/lib/terminalKeySequences'; -import { - flashTerminalMobileStatus, getTerminalMobileState, patchTerminalMobileState, } from '@/shared/lib/terminalMobileState'; -import { loadMobileTerminalFontSize } from '@/shared/lib/terminalFontSize'; +import { + loadMobileTerminalFontSize, + TERMINAL_DEFAULT_FONT_SIZE, +} from '@/shared/lib/terminalFontSize'; import { isTouchDevice } from '@/shared/hooks/useIsMobile'; import { useTerminal } from '@/shared/hooks/useTerminal'; import { TerminalMobileControls } from './TerminalMobileControls'; @@ -124,7 +123,9 @@ export function XTermInstance({ cursorBlink: true, // Touch devices restore the persisted A−/A+ stepper choice; desktop // keeps the fixed default. - fontSize: isTouchDevice() ? loadMobileTerminalFontSize() : 12, + fontSize: isTouchDevice() + ? loadMobileTerminalFontSize() + : TERMINAL_DEFAULT_FONT_SIZE, fontFamily: '"IBM Plex Mono", monospace', theme: getTerminalTheme(), }); @@ -172,21 +173,24 @@ export function XTermInstance({ registerTerminalInstance(tabId, terminal, fitAddon); - // Sticky Ctrl (mobile key bar) must act on KEYSTROKES only. onData also - // carries pastes and terminal query replies (DSR/DA), which must neither - // be transformed into control codes nor silently consume an armed latch. - // onKey fires right before the onData chunk a keystroke produces, so it - // marks the next chunk as keyboard-originated. - let nextChunkFromKeyboard = false; - terminal.onKey(() => { - nextChunkFromKeyboard = true; - }); + // Sticky Ctrl (mobile key bar): a latched ctrl turns the next single + // TYPED character into its control code. Provenance rules: pastes are + // bracketed by the isTerminalPasting marker (all app paste paths go + // through pasteTextIntoTerminal; xterm's paste() emits synchronously) — + // clipboard text is never transformed. Terminal query replies (DSR/DA) + // are always multi-char and pass through untouched. Everything else + // that arrives as a single character is a keystroke — including Android + // IME commits, which xterm delivers via composition WITHOUT a key + // event, so an onKey-based flag would leave the latch dead on Gboard. + // The latch stays armed until a keystroke consumes it. terminal.onData((data) => { - const fromKeyboard = nextChunkFromKeyboard; - nextChunkFromKeyboard = false; const conn = getTerminalConnection(tabId); if (!conn) return; - if (fromKeyboard && getTerminalMobileState(terminal).ctrlLatched) { + if ( + data.length === 1 && + !isTerminalPasting(terminal) && + getTerminalMobileState(terminal).ctrlLatched + ) { patchTerminalMobileState(terminal, { ctrlLatched: false }); conn.send(applyStickyCtrl(data).out); return; @@ -226,7 +230,8 @@ export function XTermInstance({ void navigator.clipboard ?.readText() .then((text) => { - if (text) terminal.paste(text); + // Marker-carrying paste — never sticky-Ctrl-transformed. + if (text) pasteTextIntoTerminal(terminal, text); }) .catch(() => { // Paste permission denied — Ctrl+V still works via the @@ -235,62 +240,14 @@ export function XTermInstance({ } }); - // Touch gesture layer (long-press D-pad → arrows, double-tap → Tab, - // three-finger tap → paste) and select-mode drag selection. Same - // lifetime rule as the scroll bridge below: attach once, dies with the - // element. Gated on touch capability so a non-touch session carries - // zero new listeners — and the paste gesture cannot exist there. - // Attached BEFORE the scroll bridge on purpose: listener order is - // attach order, so when a starved long-press promotes to D-pad inside - // a touchmove, the suppression flag is already set by the time the - // scroll bridge sees that same event. - if (isTouchDevice()) { - const t = terminal; - const sendRaw = (data: string) => - getTerminalConnection(tabId)?.send(data); - installTerminalTouchGestures(t, { - sendArrow: (dir) => - sendRaw(keySequence(dir, t.modes.applicationCursorKeysMode)), - sendTab: () => sendRaw('\t'), - paste: () => { - // Same guarded clipboard path + explicit feedback as the Paste - // button — a silent clipboard read would be a pastejacking aid. - if (!navigator.clipboard?.readText) { - flashTerminalMobileStatus(t, 'Paste unavailable'); - return; - } - void navigator.clipboard - .readText() - .then((text) => { - if (!text) { - flashTerminalMobileStatus(t, 'Clipboard empty'); - return; - } - t.paste(text); - flashTerminalMobileStatus(t, 'Pasted'); - }) - .catch(() => { - flashTerminalMobileStatus(t, 'Paste blocked'); - }); - }, - }); - installTerminalTouchSelection(t); - - // Keyboard-open ergonomics: focusing the terminal on a touch device - // pops the system keyboard — jump to the prompt so it isn't hidden - // in scrollback while the viewport shrinks around it. - t.textarea?.addEventListener('focus', () => t.scrollToBottom()); - } - - // 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. - // (Kept unconditional like PR #22 — inert without touch events — but - // attached AFTER the gesture layer; see the ordering note above.) - installTerminalTouchScroll(terminal); + // All touch layers (gestures → selection → scroll bridge) in the one + // valid order — see terminalTouchLayers for the ordering invariant and + // lifetime rules. Attached once per created terminal; NOT removed in + // the mount cleanup below — the listeners live and die with the + // element on terminal.dispose(), like the handlers above. + installTerminalTouchLayers(terminal, (data) => + getTerminalConnection(tabId)?.send(data) + ); } terminalRef.current = terminal; @@ -321,8 +278,16 @@ export function XTermInstance({ return () => { // A D-pad gesture running right now would never see its touchend once // the element leaves the DOM — stop its repeat timer and release the - // scroll-bridge suppression before detaching. + // scroll-bridge suppression before detaching. Modes also reset: a + // select mode or armed ctrl latch silently surviving a pane + // close/reopen (the terminal persists in the registry) would leave + // swipes selecting instead of scrolling, or fire a control code + // minutes later, with no visible cue at remount. cancelActiveTerminalGesture(terminal); + patchTerminalMobileState(terminal, { + selectMode: false, + ctrlLatched: false, + }); if (terminal.element && terminal.element.parentNode) { terminal.element.parentNode.removeChild(terminal.element); } diff --git a/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx b/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx index 325466d47d..2f811f53a2 100644 --- a/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx +++ b/packages/web-core/src/shared/components/ui-new/containers/SharedAppLayout.tsx @@ -63,10 +63,13 @@ export function SharedAppLayout() { // Track the VISUAL viewport so the app (and the terminal's input // line) ends above the on-screen keyboard instead of underneath it — // iOS never shrinks the layout viewport (see useVisualViewportHeight). - // The explicit height wins over the class's `bottom: 0`; when the - // hook has no value (no touch / no visualViewport) the class alone - // keeps the exact pre-existing inset-0 behavior. - isMobile && visualViewportHeight !== null + // Applied on BOTH layout branches: the hook is non-null only on + // touch devices, which includes tablets wide enough for the desktop + // layout (their on-screen keyboard covers the terminal all the + // same). The explicit height wins over `bottom: 0` / `h-screen`; + // when the hook has no value (no touch / no visualViewport / + // pinch-zoomed) the classes alone keep the pre-existing behavior. + visualViewportHeight !== null ? { height: visualViewportHeight, bottom: 'auto' } : undefined } diff --git a/packages/web-core/src/shared/hooks/useIsMobile.ts b/packages/web-core/src/shared/hooks/useIsMobile.ts index 460ed6e20e..e77740b7ee 100644 --- a/packages/web-core/src/shared/hooks/useIsMobile.ts +++ b/packages/web-core/src/shared/hooks/useIsMobile.ts @@ -62,12 +62,19 @@ export function useIsRealMobile(): boolean { * iPadOS with a desktop UA and hybrid touch laptops. Capability doesn't change * within a session, so it is computed once. */ +let touchDeviceCache: boolean | null = null; + 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; + // Capability is session-constant, and this now runs on hot paths (store + // snapshots, per-viewport-event) — evaluate the media query once. + if (touchDeviceCache === null) { + const coarsePointer = + typeof window.matchMedia === 'function' && + window.matchMedia('(pointer: coarse)').matches; + touchDeviceCache = coarsePointer || navigator.maxTouchPoints > 0; + } + return touchDeviceCache; } // No-op subscribe: touch capability doesn't change within a session, so the diff --git a/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts b/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts index 97739b243a..2f698982d4 100644 --- a/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts +++ b/packages/web-core/src/shared/hooks/useVisualViewportHeight.ts @@ -14,35 +14,51 @@ import { isTouchDevice } from './useIsMobile'; * Android Chrome resizes the layout viewport itself when the viewport meta * carries `interactive-widget=resizes-content`; there the visual height equals * the layout height and applying it is a no-op. + * + * Pinch-zoom is NOT the keyboard: when `visualViewport.scale > 1` the height + * shrink and the offset pan are the user zooming, so the store reports null + * (layout falls back to its full-size classes) and never fights the pan. */ type Listener = () => void; +/** Above this scale the viewport change is user pinch-zoom, not a keyboard. */ +const ZOOM_SCALE_EPSILON = 1.01; + let listeners: Listener[] = []; let height: number | null = null; let attached = false; +let notifyFrame: number | null = null; function readHeight(): number | null { const vv = window.visualViewport; if (!vv) return null; + if (vv.scale > ZOOM_SCALE_EPSILON) return null; // Round down so the container never overflows the visible area by a // sub-pixel (which would put the terminal's last row behind the keyboard). return Math.floor(vv.height); } function onViewportChange() { + const vv = window.visualViewport; + if (!vv) return; // iOS pans the visual viewport to reveal a focused input near the bottom; // with the app sized to the visible area there is nothing to reveal, so - // undo the pan to keep the app chrome pinned to the top. Touch-only: on a - // desktop, offsetTop > 0 means the USER pinch-zoomed — never fight that. - if (!isTouchDevice()) return; - if (window.visualViewport && window.visualViewport.offsetTop > 0) { + // undo the pan to keep the app chrome pinned to the top. Never at zoom — + // an offset while zoomed is the user panning around their pinch-zoom. + if (vv.offsetTop > 0 && vv.scale <= ZOOM_SCALE_EPSILON) { window.scrollTo(0, 0); } const next = readHeight(); if (next === height) return; height = next; - for (const l of [...listeners]) l(); + // Coalesce the keyboard-animation event burst to one notify per frame — + // this store re-renders the app shell, so per-event renders are real cost. + if (notifyFrame !== null) return; + notifyFrame = requestAnimationFrame(() => { + notifyFrame = null; + for (const l of [...listeners]) l(); + }); } function subscribe(listener: Listener) { @@ -50,6 +66,7 @@ function subscribe(listener: Listener) { const vv = window.visualViewport; // Never attach on non-touch devices — the hook always snapshots null there, // so listening would only risk desktop side effects for no consumer. + // (Touch capability is session-constant; see isTouchDevice.) if (vv && !attached && isTouchDevice()) { attached = true; height = readHeight(); @@ -72,8 +89,9 @@ function getSnapshot(): number | null { /** * Current visual viewport height in px on touch devices, or `null` when the - * device isn't touch-capable or `visualViewport` is unavailable — callers fall - * back to their normal (layout-viewport) sizing on `null`. + * device isn't touch-capable, `visualViewport` is unavailable, or the user is + * pinch-zoomed — callers fall back to their normal (layout-viewport) sizing + * on `null`. */ export function useVisualViewportHeight(): number | null { return useSyncExternalStore(subscribe, getSnapshot, () => null); diff --git a/packages/web-core/src/shared/lib/terminalMobileState.test.ts b/packages/web-core/src/shared/lib/terminalMobileState.test.ts new file mode 100644 index 0000000000..1f97681011 --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalMobileState.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { Terminal } from '@xterm/xterm'; + +import { + flashTerminalMobileStatus, + getTerminalMobileState, + patchTerminalMobileState, + subscribeTerminalMobileFlash, + subscribeTerminalMobileState, +} from './terminalMobileState'; + +// The store only uses the terminal as a WeakMap key. +const term = () => ({}) as Terminal; + +describe('terminal mobile state store', () => { + it('starts with everything off', () => { + expect(getTerminalMobileState(term())).toEqual({ + ctrlLatched: false, + selectMode: false, + dpadActive: false, + }); + }); + + it('patches change the snapshot identity only on real transitions', () => { + const t = term(); + const before = getTerminalMobileState(t); + patchTerminalMobileState(t, { ctrlLatched: false }); // no-op + expect(getTerminalMobileState(t)).toBe(before); + patchTerminalMobileState(t, { ctrlLatched: true }); + const after = getTerminalMobileState(t); + expect(after).not.toBe(before); + expect(after.ctrlLatched).toBe(true); + }); + + it('notifies subscribers on transitions and not on no-ops', () => { + const t = term(); + const cb = vi.fn(); + const unsubscribe = subscribeTerminalMobileState(t, cb); + patchTerminalMobileState(t, { selectMode: false }); // no-op + expect(cb).not.toHaveBeenCalled(); + patchTerminalMobileState(t, { selectMode: true }); + expect(cb).toHaveBeenCalledTimes(1); + unsubscribe(); + patchTerminalMobileState(t, { selectMode: false }); + expect(cb).toHaveBeenCalledTimes(1); + }); + + it('isolates state between terminals', () => { + const a = term(); + const b = term(); + patchTerminalMobileState(a, { dpadActive: true }); + expect(getTerminalMobileState(a).dpadActive).toBe(true); + expect(getTerminalMobileState(b).dpadActive).toBe(false); + }); +}); + +describe('flash channel (events, not state)', () => { + it('delivers messages to live listeners only — nothing is retained', () => { + const t = term(); + const early = vi.fn(); + flashTerminalMobileStatus(t, 'lost'); // no listener yet — dropped + const unsubscribe = subscribeTerminalMobileFlash(t, early); + expect(early).not.toHaveBeenCalled(); // no replay of 'lost' + flashTerminalMobileStatus(t, 'Pasted'); + expect(early).toHaveBeenCalledWith('Pasted'); + unsubscribe(); + flashTerminalMobileStatus(t, 'after'); + expect(early).toHaveBeenCalledTimes(1); + }); + + it('repeated identical messages each notify', () => { + const t = term(); + const cb = vi.fn(); + subscribeTerminalMobileFlash(t, cb); + flashTerminalMobileStatus(t, 'Pasted'); + flashTerminalMobileStatus(t, 'Pasted'); + expect(cb).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalMobileState.ts b/packages/web-core/src/shared/lib/terminalMobileState.ts index 138ab1b4f6..2fb8badd52 100644 --- a/packages/web-core/src/shared/lib/terminalMobileState.ts +++ b/packages/web-core/src/shared/lib/terminalMobileState.ts @@ -1,3 +1,4 @@ +import { useCallback, useSyncExternalStore } from 'react'; import type { Terminal } from '@xterm/xterm'; /** @@ -18,29 +19,19 @@ export interface TerminalMobileState { selectMode: boolean; /** A long-press D-pad gesture is running; the scroll bridge stands down. */ dpadActive: boolean; - /** - * One-shot status message from a DOM-level installer (e.g. the paste - * gesture) for the React controls to flash. The nonce makes every emission - * a distinct value so repeated identical messages still notify. - */ - flash: { nonce: number; message: string } | null; } -let flashNonce = 0; - -/** Emit a one-shot status flash for this terminal's mobile controls. */ -export function flashTerminalMobileStatus( - terminal: Terminal, - message: string -): void { - patchTerminalMobileState(terminal, { - flash: { nonce: ++flashNonce, message }, - }); -} +const EMPTY_STATE: TerminalMobileState = { + ctrlLatched: false, + selectMode: false, + dpadActive: false, +}; interface Entry { state: TerminalMobileState; listeners: Set<() => void>; + /** One-shot status listeners (flash pill) — events, not retained state. */ + flashListeners: Set<(message: string) => void>; } const entries = new WeakMap(); @@ -49,13 +40,9 @@ function entryFor(terminal: Terminal): Entry { let entry = entries.get(terminal); if (!entry) { entry = { - state: { - ctrlLatched: false, - selectMode: false, - dpadActive: false, - flash: null, - }, + state: { ...EMPTY_STATE }, listeners: new Set(), + flashListeners: new Set(), }; entries.set(terminal, entry); } @@ -73,17 +60,13 @@ export function patchTerminalMobileState( patch: Partial ): void { const entry = entryFor(terminal); - // Drop explicit-undefined keys so a sloppy caller can't blank a field. - const defined = Object.fromEntries( - Object.entries(patch).filter(([, value]) => value !== undefined) - ) as Partial; - const changed = (Object.keys(defined) as (keyof TerminalMobileState)[]).some( - (key) => entry.state[key] !== defined[key] + const changed = (Object.keys(patch) as (keyof TerminalMobileState)[]).some( + (key) => patch[key] !== undefined && entry.state[key] !== patch[key] ); if (!changed) return; // Snapshot object identity changes only on real transitions so React's // useSyncExternalStore consumers don't re-render on no-op patches. - entry.state = { ...entry.state, ...defined }; + entry.state = { ...entry.state, ...patch }; for (const listener of [...entry.listeners]) listener(); } @@ -95,3 +78,47 @@ export function subscribeTerminalMobileState( entry.listeners.add(listener); return () => entry.listeners.delete(listener); } + +/** + * One-shot status message from a DOM-level installer (e.g. the paste gesture) + * to whichever mobile controls are currently mounted. Deliberately an EVENT, + * not a state field: retained flash state would replay a stale "Pasted" pill + * on every remount against a long-lived terminal. + */ +export function flashTerminalMobileStatus( + terminal: Terminal, + message: string +): void { + for (const listener of [...entryFor(terminal).flashListeners]) { + listener(message); + } +} + +export function subscribeTerminalMobileFlash( + terminal: Terminal, + listener: (message: string) => void +): () => void { + const entry = entryFor(terminal); + entry.flashListeners.add(listener); + return () => entry.flashListeners.delete(listener); +} + +/** + * React view of a terminal's mobile state. Snapshot identity only changes on + * real transitions (see patchTerminalMobileState), so consumers can safely + * destructure per-field without extra memoization. + */ +export function useTerminalMobileState( + terminal: Terminal | null +): Readonly { + const subscribe = useCallback( + (cb: () => void) => + terminal ? subscribeTerminalMobileState(terminal, cb) : () => {}, + [terminal] + ); + return useSyncExternalStore( + subscribe, + () => (terminal ? getTerminalMobileState(terminal) : EMPTY_STATE), + () => EMPTY_STATE + ); +} diff --git a/packages/web-core/src/shared/lib/terminalPaste.ts b/packages/web-core/src/shared/lib/terminalPaste.ts new file mode 100644 index 0000000000..5ae0087b51 --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalPaste.ts @@ -0,0 +1,52 @@ +import type { Terminal } from '@xterm/xterm'; + +// Terminals with a paste currently flowing through their onData pipe. +// xterm's paste() emits data synchronously, so the marker brackets exactly +// the paste's chunks — the sticky-Ctrl transform checks it to keep clipboard +// text from ever being turned into control codes (a 1-char unbracketed paste +// is indistinguishable from a keystroke by shape alone). +const pasting = new WeakSet(); + +export function isTerminalPasting(terminal: Terminal): boolean { + return pasting.has(terminal); +} + +/** The ONLY way app code should paste into a terminal — carries the marker. */ +export function pasteTextIntoTerminal(terminal: Terminal, text: string): void { + pasting.add(terminal); + try { + terminal.paste(text); + } finally { + pasting.delete(terminal); + } +} + +/** + * Guarded clipboard→terminal paste with explicit user feedback — the single + * implementation behind both the Paste button and the three-finger gesture. + * A silent clipboard read would be a pastejacking aid, so every outcome + * reports: unavailable (no Clipboard API — insecure context/WebView), empty, + * blocked (permission denied), or pasted. + */ +export async function pasteIntoTerminal( + terminal: Terminal, + notify: (message: string) => void +): Promise { + // Optional chaining alone would resolve to undefined on insecure contexts + // and look like an empty clipboard — report "unavailable" distinctly. + if (!navigator.clipboard?.readText) { + notify('Paste unavailable'); + return; + } + try { + const text = await navigator.clipboard.readText(); + if (!text) { + notify('Clipboard empty'); + return; + } + pasteTextIntoTerminal(terminal, text); + notify('Pasted'); + } catch { + notify('Paste blocked'); + } +} diff --git a/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts b/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts index bde16b1635..044d380986 100644 --- a/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts +++ b/packages/web-core/src/shared/lib/terminalTouchGestures.test.ts @@ -281,3 +281,41 @@ describe('timer starvation + cancellation (council round 1)', () => { expect(dpad.at(-1)).toEqual({ active: false, dir: null }); }); }); + +describe('coalesced multi-touch + cancellation semantics (review round 1)', () => { + it('three-finger tap works when all fingers land in ONE touchstart', () => { + const { ctrl, events } = harness(); + // Previous gesture long ago — pressAt must not leak from it. + ctrl.onTouchStart(p(50, 50), 0); + ctrl.onTouchEnd(0, 30); + ctrl.onTouchStart(p(100, 100, 3), 10_000); + ctrl.onTouchEnd(0, 10_080); + expect(events).toEqual(['paste']); + }); + + it('coalesced multi-start respects the select-mode gate', () => { + const { ctrl, events } = harness({ enabled: false }); + ctrl.onTouchStart(p(100, 100, 3), 0); + ctrl.onTouchEnd(0, 80); + expect(events).toEqual([]); + }); + + it('a travelling three-finger gesture (system swipe) does not paste', () => { + const { ctrl, events } = harness(); + ctrl.onTouchStart(p(100, 100, 3), 0); + ctrl.onTouchMove(p(100 + 3 * TAP_SLOP_PX, 100, 3), 40); + ctrl.onTouchEnd(0, 90); + expect(events).toEqual([]); + }); + + it('cancel() wipes double-tap history — a cancelled tap is not tap #1', () => { + const { ctrl, events } = harness(); + ctrl.onTouchStart(p(100, 100), 0); + ctrl.onTouchEnd(0, 30); // clean tap #1 + ctrl.onTouchStart(p(100, 100), 100); + ctrl.cancel(); // browser claimed the second touch + ctrl.onTouchStart(p(100, 100), 200); + ctrl.onTouchEnd(0, 230); + expect(events).toEqual([]); // cancelled sequence must not complete a pair + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalTouchGestures.ts b/packages/web-core/src/shared/lib/terminalTouchGestures.ts index 9fb82dbbae..ddb85962d1 100644 --- a/packages/web-core/src/shared/lib/terminalTouchGestures.ts +++ b/packages/web-core/src/shared/lib/terminalTouchGestures.ts @@ -21,7 +21,10 @@ import { * * `createTouchGestureController` is pure and time-injected (every method takes * `now`; scheduling is pull-based via `nextTimerAt`) — the unit-tested seam. - * `installTerminalTouchGestures` binds real events and one timer to it. + * `installTerminalTouchGestures` binds real events and one timer to it, using + * event timestamps (not handler-delivery time) so a busy main thread can't + * misclassify a fast swipe as a dwell, and the monotonic clock so NTP steps + * can't stall repeats mid-gesture. */ export const LONG_PRESS_MS = 350; @@ -84,21 +87,26 @@ export function createTouchGestureController(deps: GestureDeps) { let startY = 0; let pressAt = 0; let maxTouches = 0; + /** A multi-touch gesture travelled beyond slop — it's a swipe, not a tap. */ + let multiMoved = false; // D-pad repeat state. let dir: ArrowKey | null = null; let interval = DPAD_TIERS[0].interval; let nextRepeatAt = 0; // Double-tap tracking. - let lastTapAt = 0; - let lastTapX = 0; - let lastTapY = 0; - let hasLastTap = false; + let lastTap: { at: number; x: number; y: number } | null = null; const exitDpad = () => { if (phase === 'dpad') deps.setDpad(false, null); dir = null; }; + const promoteToDpad = () => { + phase = 'dpad'; + dir = null; + deps.setDpad(true, null); + }; + return { onTouchStart(p: GesturePoint, now: number): void { if (p.touches === 1) { @@ -111,31 +119,56 @@ export function createTouchGestureController(deps: GestureDeps) { startY = p.clientY; pressAt = now; maxTouches = 1; + multiMoved = false; + return; + } + if (phase === 'idle') { + // The whole gesture arrived as one multi-finger touchstart (fingers + // landing within the same frame — the normal case for a deliberate + // three-finger tap). Initialize here or `pressAt` would be stale + // from the previous gesture and the select-mode gate skipped. + if (!deps.isEnabled()) { + phase = 'ignored'; + return; + } + phase = 'multi'; + startX = p.clientX; + startY = p.clientY; + pressAt = now; + maxTouches = p.touches; + multiMoved = false; return; } - // Additional finger landed. A D-pad in progress is cancelled; otherwise - // start tracking a potential three-finger tap. + // Additional finger landed mid-gesture. A D-pad in progress is + // cancelled; otherwise keep tracking a potential three-finger tap. exitDpad(); maxTouches = Math.max(maxTouches, p.touches); phase = phase === 'ignored' ? 'ignored' : 'multi'; }, onTouchMove(p: GesturePoint, now: number): GestureMoveResult { + const movedPastSlop = + Math.abs(p.clientX - startX) > TAP_SLOP_PX || + Math.abs(p.clientY - startY) > TAP_SLOP_PX; + + if (phase === 'multi') { + // A travelling multi-touch is a swipe/pinch (e.g. iPadOS three-finger + // system gestures) — it must never count as a three-finger TAP. + if (movedPastSlop) multiMoved = true; + return { prevent: false }; + } + if (phase === 'pressed') { if (now - pressAt >= LONG_PRESS_MS) { // The promotion timer can be starved by a busy main thread (TUI - // redraw, keyboard resize). The finger DID dwell long enough, so - // promote here — before the slop check — or the first delayed - // move would hand a held press to the scroll bridge. - phase = 'dpad'; - dir = null; - deps.setDpad(true, null); + // redraw, keyboard resize). The finger DID dwell long enough + // (`now` is the event's own timestamp, so a late-DELIVERED fast + // swipe doesn't land here) — promote before the slop check, or + // this move would hand a held press to the scroll bridge. + promoteToDpad(); // fall through to D-pad handling of this same move } else { - const moved = - Math.abs(p.clientX - startX) > TAP_SLOP_PX || - Math.abs(p.clientY - startY) > TAP_SLOP_PX; - if (moved) { + if (movedPastSlop) { // Moving before the long-press delay = scroll; the bridge owns it. phase = 'ignored'; } @@ -165,48 +198,50 @@ export function createTouchGestureController(deps: GestureDeps) { onTouchEnd(remainingTouches: number, now: number): void { if (remainingTouches > 0) return; // wait for the last finger const wasPhase = phase; - const wasMulti = maxTouches > 1; + const touchCount = maxTouches; + const wasMultiMoved = multiMoved; exitDpad(); phase = 'idle'; - const touchCount = maxTouches; maxTouches = 0; + multiMoved = false; if (wasPhase === 'ignored' || wasPhase === 'dpad') { - hasLastTap = false; + lastTap = null; return; } - if (wasMulti) { - if (touchCount === 3 && now - pressAt <= MULTI_TAP_MS) deps.paste(); - hasLastTap = false; + if (touchCount > 1) { + if ( + touchCount === 3 && + !wasMultiMoved && + now - pressAt <= MULTI_TAP_MS + ) { + deps.paste(); + } + lastTap = null; return; } if (wasPhase !== 'pressed' || now - pressAt >= LONG_PRESS_MS) { - hasLastTap = false; + lastTap = null; return; } // A clean quick tap. Second one in time + place = Tab. if ( - hasLastTap && - now - lastTapAt <= DOUBLE_TAP_MS && - Math.abs(startX - lastTapX) <= 2 * TAP_SLOP_PX && - Math.abs(startY - lastTapY) <= 2 * TAP_SLOP_PX + lastTap && + now - lastTap.at <= DOUBLE_TAP_MS && + Math.abs(startX - lastTap.x) <= 2 * TAP_SLOP_PX && + Math.abs(startY - lastTap.y) <= 2 * TAP_SLOP_PX ) { deps.sendTab(); - hasLastTap = false; // a triple tap is not two Tabs + lastTap = null; // a triple tap is not two Tabs return; } - hasLastTap = true; - lastTapAt = now; - lastTapX = startX; - lastTapY = startY; + lastTap = { at: now, x: startX, y: startY }; }, /** Run due work: long-press promotion and D-pad key repeats. */ onTimer(now: number): void { if (phase === 'pressed' && now - pressAt >= LONG_PRESS_MS) { - phase = 'dpad'; - dir = null; - deps.setDpad(true, null); + promoteToDpad(); return; } if (phase === 'dpad' && dir && now >= nextRepeatAt) { @@ -223,15 +258,18 @@ export function createTouchGestureController(deps: GestureDeps) { }, /** - * Abort whatever is in flight (React detach mid-gesture: touchend may - * never arrive once the element leaves the DOM). Releases the D-pad — - * clearing the scroll-bridge suppression — and stops key repeats. + * Abort whatever is in flight (touchcancel, or React detach mid-gesture: + * touchend may never arrive once the element leaves the DOM). Releases + * the D-pad — clearing the scroll-bridge suppression — and stops key + * repeats. Also the touchcancel path: a cancelled sequence must never + * count as a tap or three-finger paste. */ cancel(): void { exitDpad(); phase = 'idle'; maxTouches = 0; - hasLastTap = false; + multiMoved = false; + lastTap = null; }, }; } @@ -265,6 +303,8 @@ export function installTerminalTouchGestures( if (!el) return () => {}; // Minimal D-pad overlay: a centered badge showing the active direction. + // Plain arrows/plus only — fancier key-glyph codepoints render as tofu on + // Android/Linux (same reason the key bar uses phosphor icons). const overlay = document.createElement('div'); overlay.className = 'xterm-dpad-overlay'; overlay.style.cssText = @@ -272,17 +312,18 @@ export function installTerminalTouchGestures( 'z-index:20;display:none;padding:10px 14px;border-radius:9999px;' + 'background:rgba(0,0,0,0.55);color:#fff;font-size:20px;line-height:1;' + 'pointer-events:none;user-select:none;'; - overlay.textContent = '✛'; + overlay.textContent = '+'; el.appendChild(overlay); const DIR_GLYPH: Record = { - up: '▲', - down: '▼', - left: '◀', - right: '▶', + up: '↑', + down: '↓', + left: '←', + right: '→', }; let timer: ReturnType | null = null; + let timerTargetAt: number | null = null; const controller = createTouchGestureController({ isEnabled: () => !getTerminalMobileState(terminal).selectMode, @@ -292,23 +333,26 @@ export function installTerminalTouchGestures( setDpad: (active, dirNow) => { patchTerminalMobileState(terminal, { dpadActive: active }); overlay.style.display = active ? 'block' : 'none'; - overlay.textContent = dirNow ? DIR_GLYPH[dirNow] : '✛'; + overlay.textContent = dirNow ? DIR_GLYPH[dirNow] : '+'; }, }); const reschedule = () => { + const at = controller.nextTimerAt(); + if (at === timerTargetAt) return; // same deadline — keep the timer if (timer) { clearTimeout(timer); timer = null; } - const at = controller.nextTimerAt(); + timerTargetAt = at; if (at === null) return; timer = setTimeout( () => { - controller.onTimer(Date.now()); + timerTargetAt = null; + controller.onTimer(performance.now()); reschedule(); }, - Math.max(0, at - Date.now()) + Math.max(0, at - performance.now()) ); }; @@ -321,13 +365,16 @@ export function installTerminalTouchGestures( }; }; + // e.timeStamp shares performance.now()'s monotonic origin and carries the + // moment the touch actually happened — not when a busy main thread got + // around to delivering it. const onStart = (e: TouchEvent) => { - controller.onTouchStart(toPoint(e), Date.now()); + controller.onTouchStart(toPoint(e), e.timeStamp); reschedule(); }; const onMove = (e: TouchEvent) => { if ( - controller.onTouchMove(toPoint(e), Date.now()).prevent && + controller.onTouchMove(toPoint(e), e.timeStamp).prevent && e.cancelable ) { e.preventDefault(); @@ -335,19 +382,23 @@ export function installTerminalTouchGestures( reschedule(); }; const onEnd = (e: TouchEvent) => { - controller.onTouchEnd(e.touches.length, Date.now()); + controller.onTouchEnd(e.touches.length, e.timeStamp); + reschedule(); + }; + const onCancel = () => { + // A cancelled sequence (system gesture, palm rejection, browser + // interruption) must not fire tap/paste actions — and may never deliver + // another event for the remaining fingers, so don't wait for them. + controller.cancel(); reschedule(); }; el.addEventListener('touchstart', onStart, { passive: true }); el.addEventListener('touchmove', onMove, { passive: false }); el.addEventListener('touchend', onEnd, { passive: true }); - el.addEventListener('touchcancel', onEnd, { passive: true }); + el.addEventListener('touchcancel', onCancel, { passive: true }); - activeGestureCancels.set(terminal, () => { - controller.cancel(); - reschedule(); // nextTimerAt() is now null → clears the pending timer - }); + activeGestureCancels.set(terminal, onCancel); return () => { if (timer) clearTimeout(timer); @@ -355,7 +406,7 @@ export function installTerminalTouchGestures( el.removeEventListener('touchstart', onStart); el.removeEventListener('touchmove', onMove); el.removeEventListener('touchend', onEnd); - el.removeEventListener('touchcancel', onEnd); + el.removeEventListener('touchcancel', onCancel); overlay.remove(); }; } diff --git a/packages/web-core/src/shared/lib/terminalTouchLayers.ts b/packages/web-core/src/shared/lib/terminalTouchLayers.ts new file mode 100644 index 0000000000..eb8d86c8cf --- /dev/null +++ b/packages/web-core/src/shared/lib/terminalTouchLayers.ts @@ -0,0 +1,67 @@ +import type { Terminal } from '@xterm/xterm'; + +import { isTouchDevice } from '@/shared/hooks/useIsMobile'; +import { keySequence } from './terminalKeySequences'; +import { + flashTerminalMobileStatus, + getTerminalMobileState, + patchTerminalMobileState, +} from './terminalMobileState'; +import { pasteIntoTerminal } from './terminalPaste'; +import { installTerminalTouchGestures } from './terminalTouchGestures'; +import { installTerminalTouchScroll } from './terminalTouchScroll'; +import { installTerminalTouchSelection } from './terminalTouchSelection'; + +/** + * Install every touch layer for a freshly created terminal, in the one order + * that is correct — this function exists to make the ordering invariant + * impossible to violate from a component: + * + * 1. gestures, 2. selection, 3. scroll bridge (LAST) + * + * Listener order is attach order, so when a starved long-press promotes to + * D-pad inside a touchmove, the suppression flag is already set by the time + * the scroll bridge sees that same event. Attach the bridge first and every + * starved promotion leaks wheel steps to the PTY. + * + * Lifetime: attach once per created terminal; listeners live and die with + * `terminal.element` (an in-flight D-pad is additionally cancellable via + * cancelActiveTerminalGesture on React detach). Gestures/selection are gated + * on touch capability so non-touch sessions carry zero new listeners; the + * scroll bridge stays unconditional exactly as PR #22 shipped it (inert + * without touch events). + */ +export function installTerminalTouchLayers( + terminal: Terminal, + sendRaw: (data: string) => void +): void { + if (isTouchDevice()) { + // A gesture-sent key counts as the ctrl latch's "one keystroke", same as + // a key-bar tap — otherwise ctrl, D-pad arrow, then a typed letter would + // surprise-fire a control combo. + const sendKey = (data: string) => { + sendRaw(data); + if (getTerminalMobileState(terminal).ctrlLatched) { + patchTerminalMobileState(terminal, { ctrlLatched: false }); + } + }; + installTerminalTouchGestures(terminal, { + sendArrow: (dir) => + sendKey(keySequence(dir, terminal.modes.applicationCursorKeysMode)), + sendTab: () => sendKey('\t'), + paste: () => + void pasteIntoTerminal(terminal, (message) => + flashTerminalMobileStatus(terminal, message) + ), + }); + installTerminalTouchSelection(terminal); + + // Keyboard-open ergonomics: focusing the terminal on a touch device pops + // the system keyboard — jump to the prompt so it isn't hidden in + // scrollback while the viewport shrinks around it. + terminal.textarea?.addEventListener('focus', () => + terminal.scrollToBottom() + ); + } + installTerminalTouchScroll(terminal); +} diff --git a/packages/web-core/src/shared/lib/terminalTouchSelection.test.ts b/packages/web-core/src/shared/lib/terminalTouchSelection.test.ts index 4e2ae27929..16891d126f 100644 --- a/packages/web-core/src/shared/lib/terminalTouchSelection.test.ts +++ b/packages/web-core/src/shared/lib/terminalTouchSelection.test.ts @@ -50,3 +50,13 @@ describe('linearSelection', () => { ).toEqual({ col: 4, row: 2, length: 1 }); }); }); + +describe('touchToCell — degenerate rects (review round 1)', () => { + it('returns null instead of NaN cells for zero-sized rects', () => { + const zero = { left: 0, top: 0, width: 0, height: 0 }; + expect(touchToCell(0, 0, zero, cols, rows)).toBeNull(); + expect(touchToCell(10, 10, { ...rect, width: 0 }, cols, rows)).toBeNull(); + expect(touchToCell(10, 10, { ...rect, height: 0 }, cols, rows)).toBeNull(); + expect(touchToCell(10, 10, rect, 0, rows)).toBeNull(); + }); +}); diff --git a/packages/web-core/src/shared/lib/terminalTouchSelection.ts b/packages/web-core/src/shared/lib/terminalTouchSelection.ts index e21bfefb92..432b855165 100644 --- a/packages/web-core/src/shared/lib/terminalTouchSelection.ts +++ b/packages/web-core/src/shared/lib/terminalTouchSelection.ts @@ -1,5 +1,6 @@ import type { Terminal } from '@xterm/xterm'; +import { writeClipboardViaBridge } from './clipboard'; import { getTerminalMobileState } from './terminalMobileState'; /** @@ -13,7 +14,14 @@ import { getTerminalMobileState } from './terminalMobileState'; * * Active only while `selectMode` is on (toggled from TerminalMobileControls); * the gesture layer and scroll bridge stand down for the same flag. The - * existing `onSelectionChange` handler auto-copies the result. + * finished selection is copied exactly once on release — the per-change + * auto-copy in XTermInstance is suppressed during select mode so a drag + * doesn't hammer the clipboard with intermediate selections. + * + * Listeners are CAPTURE-phase and stop propagation while select mode is on: + * xterm's own viewport touch handlers (which scroll local scrollback when + * mouse tracking is off — plain shell terminals) live on child elements and + * would otherwise scroll the buffer underneath the selection drag. */ export interface CellPoint { @@ -21,14 +29,21 @@ export interface CellPoint { row: number; } -/** Map a touch point to a 0-based cell within the screen rect (clamped). */ +/** + * Map a touch point to a 0-based cell within the screen rect (clamped). + * Returns null for a degenerate rect (hidden/zero-sized pane mid-layout) — + * dividing by it would produce NaN cells and corrupt xterm's selection. + */ export function touchToCell( clientX: number, clientY: number, rect: { left: number; top: number; width: number; height: number }, cols: number, rows: number -): CellPoint { +): CellPoint | null { + if (rect.width <= 0 || rect.height <= 0 || cols <= 0 || rows <= 0) { + return null; + } const cellW = rect.width / cols; const cellH = rect.height / rows; const col = Math.floor((clientX - rect.left) / cellW); @@ -71,8 +86,12 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { (el.querySelector('.xterm-screen') as HTMLElement | null) ?? el; let anchor: CellPoint | null = null; + let lastSel: { col: number; row: number; length: number } | null = null; - const cellFromTouch = (t: { clientX: number; clientY: number }) => { + const cellFromTouch = (t: { + clientX: number; + clientY: number; + }): CellPoint | null => { const rect = screen.getBoundingClientRect(); const cell = touchToCell( t.clientX, @@ -81,6 +100,7 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { terminal.cols, terminal.rows ); + if (!cell) return null; // Anchor rows to the ABSOLUTE buffer position at gesture time so the // selection targets what the user sees even with scrollback offset. return { col: cell.col, row: cell.row + terminal.buffer.active.viewportY }; @@ -88,54 +108,71 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { const onStart = (e: TouchEvent) => { if (!getTerminalMobileState(terminal).selectMode) return; + // Select mode owns the touch: keep it from xterm's own viewport touch + // scrolling (child listeners) and from browser default scrolling. + e.stopPropagation(); + if (e.cancelable) e.preventDefault(); if (e.touches.length !== 1) { anchor = null; return; } anchor = cellFromTouch(e.touches[0]); + lastSel = null; terminal.clearSelection(); - // Don't let the tap refocus/scroll — in select mode the finger selects. - if (e.cancelable) e.preventDefault(); }; const onMove = (e: TouchEvent) => { - if (!anchor || !getTerminalMobileState(terminal).selectMode) return; - if (e.touches.length !== 1) return; + if (!getTerminalMobileState(terminal).selectMode) return; + e.stopPropagation(); + if (e.cancelable) e.preventDefault(); + if (!anchor || e.touches.length !== 1) return; const focus = cellFromTouch(e.touches[0]); + if (!focus) return; const sel = linearSelection(anchor, focus, terminal.cols); + // Only touch xterm's selection service when the target cell changed — + // touchmove fires at 60-120Hz, cells change far less often. + if ( + lastSel && + lastSel.col === sel.col && + lastSel.row === sel.row && + lastSel.length === sel.length + ) { + return; + } + lastSel = sel; terminal.select(sel.col, sel.row, sel.length); - if (e.cancelable) e.preventDefault(); }; const onEnd = () => { - // Copy exactly once, on release — the per-change auto-copy is suppressed - // in select mode so a drag doesn't overwrite the clipboard dozens of - // times with intermediate selections. + // Copy exactly once, on release. Bridge-aware helper: in the VSCode + // iframe navigator.clipboard rejects and the parent handles the copy. if ( anchor !== null && getTerminalMobileState(terminal).selectMode && terminal.hasSelection() ) { const text = terminal.getSelection(); - if (text) { - void navigator.clipboard?.writeText(text).catch(() => { - // Clipboard can be blocked; the selection itself still stands and - // the Copy button reports errors explicitly. - }); - } + if (text) void writeClipboardViaBridge(text); } anchor = null; + lastSel = null; }; - el.addEventListener('touchstart', onStart, { passive: false }); - el.addEventListener('touchmove', onMove, { passive: false }); - el.addEventListener('touchend', onEnd, { passive: true }); - el.addEventListener('touchcancel', onEnd, { passive: true }); + el.addEventListener('touchstart', onStart, { + passive: false, + capture: true, + }); + el.addEventListener('touchmove', onMove, { passive: false, capture: true }); + el.addEventListener('touchend', onEnd, { passive: true, capture: true }); + el.addEventListener('touchcancel', onEnd, { + passive: true, + capture: true, + }); return () => { - el.removeEventListener('touchstart', onStart); - el.removeEventListener('touchmove', onMove); - el.removeEventListener('touchend', onEnd); - el.removeEventListener('touchcancel', onEnd); + el.removeEventListener('touchstart', onStart, { capture: true }); + el.removeEventListener('touchmove', onMove, { capture: true }); + el.removeEventListener('touchend', onEnd, { capture: true }); + el.removeEventListener('touchcancel', onEnd, { capture: true }); }; } From fdcb05ed319cb9f5ce44ec5d9fbb11c20ce989aa Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 07:12:19 +0000 Subject: [PATCH 09/11] fix(mobile): council round-2 security fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Select-mode copy uses navigator.clipboard directly instead of writeClipboardViaBridge: the bridge's fallback posts the text to window.parent with targetOrigin '*', and a terminal selection can hold secrets — an untrusted framing page must never receive them. Failure is surfaced via the flash pill; the selection stays so the Copy button can retry. (Matches every other terminal copy path, which also use the direct clipboard.) - Touch devices intercept the textarea's native paste event (OS paste callout, hardware Cmd+V) and route it through pasteTextIntoTerminal so the sticky-Ctrl provenance marker covers ALL paste paths — a 1-char clipboard while Ctrl is latched can no longer become a control code. Desktop keeps xterm's native paste handler untouched. - Select-mode touchcancel clears the drag WITHOUT copying: a cancelled sequence must not clobber the clipboard with a partial selection. Deliberately unchanged: multi-char chunks (IME commits) pass through without consuming the latch — consuming there would let invisible terminal query replies (DSR/DA, also multi-char) eat an armed latch, the exact round-1 bug; the latch is always visible on the highlighted ctrl key. --- .../src/shared/lib/terminalTouchLayers.ts | 13 +++++++- .../src/shared/lib/terminalTouchSelection.ts | 32 +++++++++++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/packages/web-core/src/shared/lib/terminalTouchLayers.ts b/packages/web-core/src/shared/lib/terminalTouchLayers.ts index eb8d86c8cf..48cc09d1cc 100644 --- a/packages/web-core/src/shared/lib/terminalTouchLayers.ts +++ b/packages/web-core/src/shared/lib/terminalTouchLayers.ts @@ -7,7 +7,7 @@ import { getTerminalMobileState, patchTerminalMobileState, } from './terminalMobileState'; -import { pasteIntoTerminal } from './terminalPaste'; +import { pasteIntoTerminal, pasteTextIntoTerminal } from './terminalPaste'; import { installTerminalTouchGestures } from './terminalTouchGestures'; import { installTerminalTouchScroll } from './terminalTouchScroll'; import { installTerminalTouchSelection } from './terminalTouchSelection'; @@ -62,6 +62,17 @@ export function installTerminalTouchLayers( terminal.textarea?.addEventListener('focus', () => terminal.scrollToBottom() ); + + // Route the OS paste callout / hardware Cmd+V through the provenance + // marker too — xterm's internal textarea paste handler would otherwise + // emit onData unmarked, and a 1-char clipboard while Ctrl is latched + // would be synthesized into a control code. Touch-only: the latch can + // only be armed here, and desktop keeps xterm's native paste path. + terminal.textarea?.addEventListener('paste', (e) => { + const text = e.clipboardData?.getData('text'); + e.preventDefault(); + if (text) pasteTextIntoTerminal(terminal, text); + }); } installTerminalTouchScroll(terminal); } diff --git a/packages/web-core/src/shared/lib/terminalTouchSelection.ts b/packages/web-core/src/shared/lib/terminalTouchSelection.ts index 432b855165..b125ea36a1 100644 --- a/packages/web-core/src/shared/lib/terminalTouchSelection.ts +++ b/packages/web-core/src/shared/lib/terminalTouchSelection.ts @@ -1,7 +1,9 @@ import type { Terminal } from '@xterm/xterm'; -import { writeClipboardViaBridge } from './clipboard'; -import { getTerminalMobileState } from './terminalMobileState'; +import { + flashTerminalMobileStatus, + getTerminalMobileState, +} from './terminalMobileState'; /** * Touch drag-selection for select mode. @@ -144,27 +146,43 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { }; const onEnd = () => { - // Copy exactly once, on release. Bridge-aware helper: in the VSCode - // iframe navigator.clipboard rejects and the parent handles the copy. + // Copy exactly once, on release. Direct clipboard only — deliberately + // NOT writeClipboardViaBridge: its fallback posts the text to + // window.parent with targetOrigin '*', and terminal selections can hold + // secrets; an untrusted framing page must never receive them. Failure + // is surfaced instead (the selection stays — the Copy button retries). if ( anchor !== null && getTerminalMobileState(terminal).selectMode && terminal.hasSelection() ) { const text = terminal.getSelection(); - if (text) void writeClipboardViaBridge(text); + if (text && navigator.clipboard?.writeText) { + navigator.clipboard.writeText(text).then( + () => flashTerminalMobileStatus(terminal, 'Copied selection'), + () => flashTerminalMobileStatus(terminal, 'Copy blocked') + ); + } } anchor = null; lastSel = null; }; + const onCancel = () => { + // A cancelled sequence (system gesture, palm rejection) fires nothing — + // clobbering the clipboard with a partial selection would be worse than + // no copy. The selection stays visible; Copy can still grab it. + anchor = null; + lastSel = null; + }; + el.addEventListener('touchstart', onStart, { passive: false, capture: true, }); el.addEventListener('touchmove', onMove, { passive: false, capture: true }); el.addEventListener('touchend', onEnd, { passive: true, capture: true }); - el.addEventListener('touchcancel', onEnd, { + el.addEventListener('touchcancel', onCancel, { passive: true, capture: true, }); @@ -173,6 +191,6 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { el.removeEventListener('touchstart', onStart, { capture: true }); el.removeEventListener('touchmove', onMove, { capture: true }); el.removeEventListener('touchend', onEnd, { capture: true }); - el.removeEventListener('touchcancel', onEnd, { capture: true }); + el.removeEventListener('touchcancel', onCancel, { capture: true }); }; } From 60e413a05e88c7ae9e022f18a1d3221c859c5c32 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 07:15:58 +0000 Subject: [PATCH 10/11] =?UTF-8?q?fix(mobile):=20CodeRabbit=20=E2=80=94=20s?= =?UTF-8?q?tray=20fingers=20must=20not=20corrupt=20a=20select-mode=20drag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A palm touch mid-drag no longer discards the anchor (the drag resumes when the stray finger lifts), and the copy-on-release waits for the LAST finger — a stray finger lifting first can't end the drag and copy a partial selection while the primary finger is still down. Mirrors the gesture controller's remaining-touches rule. --- packages/web-core/src/shared/lib/terminalTouchSelection.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/web-core/src/shared/lib/terminalTouchSelection.ts b/packages/web-core/src/shared/lib/terminalTouchSelection.ts index b125ea36a1..24897bcd1a 100644 --- a/packages/web-core/src/shared/lib/terminalTouchSelection.ts +++ b/packages/web-core/src/shared/lib/terminalTouchSelection.ts @@ -115,7 +115,9 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { e.stopPropagation(); if (e.cancelable) e.preventDefault(); if (e.touches.length !== 1) { - anchor = null; + // Either a fresh multi-finger gesture (anchor is already null) or a + // stray extra finger mid-drag (palm contact) — keep the anchor and + // ignore it; the drag resumes when the stray finger lifts. return; } anchor = cellFromTouch(e.touches[0]); @@ -145,7 +147,8 @@ export function installTerminalTouchSelection(terminal: Terminal): () => void { terminal.select(sel.col, sel.row, sel.length); }; - const onEnd = () => { + const onEnd = (e: TouchEvent) => { + if (e.touches.length > 0) return; // wait for the last finger // Copy exactly once, on release. Direct clipboard only — deliberately // NOT writeClipboardViaBridge: its fallback posts the text to // window.parent with targetOrigin '*', and terminal selections can hold From 15c160b91103465cca0735137d78d64dc042f6c5 Mon Sep 17 00:00:00 2001 From: Miguel Rasero Date: Sat, 4 Jul 2026 07:24:15 +0000 Subject: [PATCH 11/11] fix(mobile): paste interception must beat xterm's own textarea handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit: the previous listener registered on the textarea AFTER xterm's (attached at open()), so both ran — preventDefault can't stop an earlier listener on the same target, double-pasting every OS-callout/Cmd+V paste on touch devices. Ancestor capture-phase listener + stopPropagation runs before the target and keeps the event from xterm's handler entirely. --- .../src/shared/lib/terminalTouchLayers.ts | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/web-core/src/shared/lib/terminalTouchLayers.ts b/packages/web-core/src/shared/lib/terminalTouchLayers.ts index 48cc09d1cc..7691426004 100644 --- a/packages/web-core/src/shared/lib/terminalTouchLayers.ts +++ b/packages/web-core/src/shared/lib/terminalTouchLayers.ts @@ -66,13 +66,23 @@ export function installTerminalTouchLayers( // Route the OS paste callout / hardware Cmd+V through the provenance // marker too — xterm's internal textarea paste handler would otherwise // emit onData unmarked, and a 1-char clipboard while Ctrl is latched - // would be synthesized into a control code. Touch-only: the latch can - // only be armed here, and desktop keeps xterm's native paste path. - terminal.textarea?.addEventListener('paste', (e) => { - const text = e.clipboardData?.getData('text'); - e.preventDefault(); - if (text) pasteTextIntoTerminal(terminal, text); - }); + // would be synthesized into a control code. Capture-phase on the ROOT + // element: at the textarea itself listeners run in registration order + // and xterm's (registered at open()) would fire first — both handlers + // running would paste twice. Ancestor capture runs before the target, + // and stopPropagation keeps the event from xterm's handler entirely. + // Touch-only: the latch can only be armed here, and desktop keeps + // xterm's native paste path. + terminal.element?.addEventListener( + 'paste', + (e) => { + const text = e.clipboardData?.getData('text'); + e.preventDefault(); + e.stopPropagation(); + if (text) pasteTextIntoTerminal(terminal, text); + }, + { capture: true } + ); } installTerminalTouchScroll(terminal); }