fix(mobile): keep the keyboard reachable when the viewport is scrolled up

Addresses the review on #244.

BLOCKING (item 1). selectSession() ends with scrollToLastNonEmptyLine(), which
parks the viewport above the bottom for any session taller than the screen, so
after a tab switch every tap classified as 'history' — touchstart ran
preventDefault() + blur, and touchend's early return skipped focus. Both routes
to focus closed on one gesture, the same mechanism as #173.

Suppressing the mouse REPORT while scrolled up is right and is kept; suppressing
FOCUS is not. touchstart now only preventDefaults 'content' taps (a scrolled-up
viewport sends nothing, so there is no compatibility click worth cancelling), and
the 'history' branch focuses instead of blurring.

Verified against the maintainer's own test, which was already on master and red:
`keeps the terminal input focusable after a tab switch parks the viewport
off-bottom` fails without this change and passes with it.

Item 2: dropped both `terminal-action-pending` guards. The class exists nowhere
in the repo, so both branches were permanently false and the comment promised
coverage that did not exist.

Item 3: removed the `Working` literals. Live claude 2.1.226 prints
"Cooked for 2m 6s" with a different bullet and a randomised verb, so they were
dead code. The status row is matched by its affordance ("esc to interrupt")
instead, which is what makes it actionable. The affordance regex is also
tightened to require a key or gesture name, so prose like "click here to open
the file" no longer dismisses the keyboard.

Item 4: removed _shouldForwardTouchScrollToApp and its test. It was never called,
and wiring it as written would have restricted forwarding to claude only,
dropping gemini from the path #205 established — a behaviour change this PR has
no reason to make.

Smaller items: the touchstart classification is cached and reused for the
touchend of the same gesture (keyed on exact coordinates, so a moved finger
re-classifies), removing two of the three full-viewport scans per gesture; the
duplicated touchLastX assignment is gone; and the no-touch bail-out returns null
rather than claiming 'history'.

test/mobile/keyboard.test.ts: 51 tests, 5 failed | 46 passed — the same 5
pre-existing failures as master, unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
lior
2026-08-10 13:32:11 +03:00
co-authored by Claude Opus 5
parent 623fedf5b7
commit 3b85001fed
3 changed files with 147 additions and 101 deletions
+53 -28
View File
@@ -653,6 +653,7 @@ Object.assign(CodemanApp.prototype, {
let didScroll = false; // track whether touchmove fired (tap vs scroll)
let touchStartY = 0;
let tapStartedWithTerminalFocus = false;
let tapStartIntentCache = null;
const TAP_THRESHOLD = 8; // px — ignore micro-drift to distinguish tap from scroll
container.addEventListener(
'touchstart',
@@ -666,11 +667,24 @@ Object.assign(CodemanApp.prototype, {
isTouching = true;
didScroll = false;
tapStartedWithTerminalFocus = this._isMobileTerminalInputFocused();
// Classifying scans the whole viewport with translateToString, and
// this runs at the start of EVERY gesture including scroll drags.
// Cache the result for the touchend of this same gesture rather than
// recomputing it; the cache is keyed on the exact start coordinates
// so a finger that moved re-classifies at its real position.
const touchStartIntent = this._classifyMobileTerminalTap(touchLastX, touchLastY);
if (touchStartIntent !== 'input') {
tapStartIntentCache = { x: touchLastX, y: touchLastY, intent: touchStartIntent };
if (touchStartIntent === 'content') {
// Cancel xterm/browser focus before the compatibility click can
// open the OS keyboard. Content taps are re-emitted as SGR on
// touchend; history taps deliberately remain inert.
// touchend.
//
// 'history' is deliberately NOT included. A scrolled-up viewport
// sends nothing, so there is no compatibility click worth
// cancelling — and preventDefault() here, paired with touchend's
// early return, closes both routes to focus at once. Since
// selectSession() ends with scrollToLastNonEmptyLine(), that made
// the keyboard unreachable after every tab switch.
ev.preventDefault();
this._blurMobileTerminalInput();
}
@@ -700,7 +714,6 @@ Object.assign(CodemanApp.prototype, {
// fling, so a jittery tap would both position the cursor AND scroll.
if (!didScroll) return;
ev.preventDefault();
touchLastX = ev.touches[0].clientX;
const delta = touchLastY - touchY; // positive = scroll down
pixelAccum += delta;
velocity = delta * 1.2;
@@ -737,7 +750,13 @@ Object.assign(CodemanApp.prototype, {
const touch = ev.changedTouches && ev.changedTouches[0];
if (touch) {
this._suppressTrustedTapMouseEvents();
this._handleMobileTerminalTap(touch, tapStartedWithTerminalFocus);
const cached =
tapStartIntentCache &&
tapStartIntentCache.x === touch.clientX &&
tapStartIntentCache.y === touch.clientY
? tapStartIntentCache.intent
: null;
this._handleMobileTerminalTap(touch, tapStartedWithTerminalFocus, cached);
}
}
tapStartedWithTerminalFocus = false;
@@ -3371,9 +3390,6 @@ Object.assign(CodemanApp.prototype, {
const mouseTrackingOn = !!mouseMode && mouseMode !== 'none';
if (!mouseTrackingOn && !this._sessionUsesServerMouseStrip()) return 'input';
// Permission/elicitation prompts own the full live terminal until answered.
if (document.body?.classList?.contains('terminal-action-pending')) return 'content';
const buffer = this.terminal.buffer?.active;
if (!buffer?.getLine) return 'input';
@@ -3421,10 +3437,13 @@ Object.assign(CodemanApp.prototype, {
let logicalLineEnd = tappedRow;
while (logicalLineEnd + 1 < rows && wrappedRows[logicalLineEnd + 1]) logicalLineEnd++;
const tappedLine = lines.slice(logicalLineStart, logicalLineEnd + 1).join('');
if (
mode === 'claude' &&
/^\s*[•·]\s*Working\b.*(?:background|esc to interrupt)/i.test(tappedLine)
) {
// Claude's status row is TUI-owned: tapping it opens the teammate view, so it
// must not be treated as a keyboard target. Match the AFFORDANCE, not the
// wording — the bullet and verb are both unstable (claude 2.1.226 prints
// "✻ Cooked for 2m 6s", "✻ Baked for 9m 47s"; earlier builds printed
// "• Working …"), while "esc to interrupt" / "background" are what make the
// row actionable in the first place.
if (mode === 'claude' && /\b(?:esc to interrupt|background)\b/i.test(tappedLine)) {
return 'content';
}
if (menuSelectionVisible) return 'content';
@@ -3471,8 +3490,6 @@ Object.assign(CodemanApp.prototype, {
* otherwise focus xterm, so focus has to be restored explicitly.
*/
_isActionableMobileTerminalTap(clientX, clientY) {
if (document.body?.classList?.contains('terminal-action-pending')) return true;
const pos = this._clientPointToCell(clientX, clientY);
const buffer = this.terminal?.buffer?.active;
if (!pos || !buffer?.getLine) return false;
@@ -3502,14 +3519,22 @@ Object.assign(CodemanApp.prototype, {
// The hint sits on its own row, so a readback's TITLE row — the one a
// finger actually lands on — carries no affordance text itself. Look at the
// adjacent row too, which is how these blocks are laid out in practice.
// Keyed on the ACTION VERB, and deliberately not on prose verbs. A CLI hint
// names a key or a gesture ("ctrl+r to expand", "tap to collapse",
// "esc to interrupt"); "click here to open the file" is transcript content
// and must keep the keyboard, so `click` and bare `here` are excluded.
// The hint may sit mid-line — Claude's status row is
// "✻ Cooked for 2m 6s · esc to interrupt" — so this is not anchored.
const affordance =
/\b(?:ctrl\+\w+|tap|click|enter|esc)\b[^.]{0,24}\bto\s+(?:expand|collapse|view|open|interrupt|see)\b/i;
/\b(?:ctrl\+\w+|shift\+\w+|esc|enter|tab|tap)\s+to\s+(?:expand|collapse|view|open|interrupt|see)\b/i;
const blockStart = Math.max(0, logicalLineStart - 1);
const blockEnd = Math.min(rows - 1, logicalLineEnd + 1);
for (let row = blockStart; row <= blockEnd; row++) {
if (affordance.test(lines[row])) return true;
}
if (/^\s*[•·]\s*Working\b/i.test(tappedLine)) return true;
// A Claude status row ("✻ Cooked for 2m 6s · esc to interrupt") is caught by
// the affordance above; there is deliberately no verb literal here, because
// the verb is randomised per build.
const hasMenuPrompt = lines.some((line) => /^\s*[❯›]\s+\d+[.)]\s/.test(line));
const hasMenuChoice = lines.some((line) => /^\s+\d+[.)]\s/.test(line));
@@ -3526,11 +3551,20 @@ Object.assign(CodemanApp.prototype, {
}
},
_handleMobileTerminalTap(touch, startedWithTerminalFocus) {
if (!touch || !this.terminal) return 'history';
const intent = this._classifyMobileTerminalTap(touch.clientX, touch.clientY);
_handleMobileTerminalTap(touch, startedWithTerminalFocus, cachedIntent = null) {
// A guard bail-out, not a classification: there is nothing to classify. It is
// deliberately NOT 'history', which would claim the viewport was scrolled up.
if (!touch || !this.terminal) return null;
// touchstart already classified this exact point; reuse it rather than paying
// a second full-viewport scan for the same gesture.
const intent = cachedIntent ?? this._classifyMobileTerminalTap(touch.clientX, touch.clientY);
if (intent === 'history') {
this._blurMobileTerminalInput();
// Scrolled up: send NO mouse report — a tap on old output must not be
// delivered to the CLI as a click on whatever row now occupies that cell.
// Focus is a separate question, and the answer is yes: the user tapped the
// terminal, so let them type. Blurring here stranded activeElement on
// <body> with no way back to the keyboard.
this._focusMobileTerminalInput();
return intent;
}
@@ -3740,15 +3774,6 @@ Object.assign(CodemanApp.prototype, {
return true;
},
// Claude keeps most transcript history inside its own TUI rather than xterm
// scrollback. On verified versions, route a touch drag through the same SGR
// wheel path as desktop. Codex keeps the existing local touch behavior.
_shouldForwardTouchScrollToApp() {
const session = this.sessions?.get(this.activeSessionId);
if (session?.mode !== 'claude') return false;
return this._shouldForwardWheelToApp({ shiftKey: false });
},
// Encode wheel ticks as SGR reports (button 64 = up, 65 = down) at the pointer
// cell. Reports are coalesced into one fire-and-forget write per ~40ms: a
// trackpad emits dozens of wheel events per second and each send becomes a
+94 -54
View File
@@ -756,6 +756,94 @@ describe('Virtual Keyboard', () => {
it('focuses the terminal helper textarea when the terminal is tapped', async () => {
await page.evaluate(() => {
app.activeSessionId = 'mobile-focus-visible-input-test';
app.sessions.set('mobile-focus-visible-input-test', {
id: 'mobile-focus-visible-input-test',
mode: 'codex',
status: 'running',
});
app.hideWelcome();
const settings = app.loadAppSettingsFromStorage();
settings.cjkInputEnabled = false;
app.saveAppSettingsToStorage(settings);
app._updateCjkInputState();
});
await page.locator('#terminalContainer').tap({ position: { x: 40, y: 40 } });
const activeClass = await page.evaluate(() => document.activeElement?.className);
expect(activeClass).toContain('xterm-helper-textarea');
});
// Regression guard for the phone-keyboard blocker reduced in #173 and re-hit
// by #244. selectSession() ends with scrollToLastNonEmptyLine(), which parks
// the viewport ABOVE the bottom for any session whose buffer is taller than
// the screen and ends in blank rows, i.e. every real session after a tab
// switch. A tap-routing scheme that treats "viewport is scrolled up" as a
// reason to blur strands document.activeElement on <body> with no way to
// raise the keyboard, and the prompt row is no exception. Suppressing the
// MOUSE REPORT while scrolled up is correct and pinned below; suppressing
// FOCUS is not. Measured against PR #244 on 2026-08-09: body vs textarea.
//
// Must be a dispatched gesture: calling the touchend handler directly
// bypasses touchstart's preventDefault, which is half of what closes the
// focus path, so a direct call reports the right intent and still misses.
it('keeps the terminal input focusable after a tab switch parks the viewport off-bottom', async () => {
const probe = await page.evaluate(async () => {
window.__sentInputs = [];
app.activeSessionId = 'mobile-offbottom-tap-test';
app.sessions.set('mobile-offbottom-tap-test', {
id: 'mobile-offbottom-tap-test',
mode: 'claude',
cliVersion: '2.1.220',
status: 'running',
});
app._sendInputAsync = (_sessionId: string, input: string) => {
window.__sentInputs.push(input);
};
app.hideWelcome();
const settings = app.loadAppSettingsFromStorage();
settings.cjkInputEnabled = false;
app.saveAppSettingsToStorage(settings);
app._updateCjkInputState();
app.terminal.reset();
// Taller than the viewport, ending in the trailing blank rows that make
// scrollToLastNonEmptyLine() stop short of the bottom.
const lines: string[] = [];
for (let i = 1; i <= app.terminal.rows * 3; i++) lines.push(`Transcript row ${i}`);
lines.push('', '❯ ', '', '');
await new Promise<void>((resolve) => app.terminal.write(lines.join('\r\n'), resolve));
app.scrollToLastNonEmptyLine(); // what selectSession() does on every tab switch
(document.activeElement as HTMLElement | null)?.blur?.();
const screen = app.terminal.element?.querySelector('.xterm-screen');
const cell = app.terminal._core?._renderService?.dimensions?.css?.cell;
const rect = screen?.getBoundingClientRect();
if (!rect || !cell?.width || !cell?.height) return null;
const buffer = app.terminal.buffer.active;
return {
x: rect.left + cell.width * 2,
y: rect.top + cell.height * 5.5,
atBottom: buffer.viewportY >= buffer.baseY,
};
});
expect(probe).not.toBeNull();
// The guard only means anything if the viewport really did park off-bottom.
expect(probe!.atBottom).toBe(false);
await page.touchscreen.tap(probe!.x, probe!.y);
const state = await page.evaluate(() => ({
activeClass: document.activeElement?.className,
sentInputs: window.__sentInputs,
}));
expect(state.activeClass).toContain('xterm-helper-textarea');
// SGR coordinates are meaningless off-bottom, so the tap must stay silent.
expect(state.sentInputs).toEqual([]);
});
it('collapses a terminal readback without focusing the hidden textarea', async () => {
const point = await page.evaluate(async () => {
@@ -961,42 +1049,6 @@ describe('Virtual Keyboard', () => {
mode: 'codex',
status: 'running',
});
app.hideWelcome();
const settings = app.loadAppSettingsFromStorage();
settings.cjkInputEnabled = false;
app.saveAppSettingsToStorage(settings);
app._updateCjkInputState();
});
await page.locator('#terminalContainer').tap({ position: { x: 40, y: 40 } });
const activeClass = await page.evaluate(() => document.activeElement?.className);
expect(activeClass).toContain('xterm-helper-textarea');
});
// Regression guard for the phone-keyboard blocker reduced in #173 and re-hit
// by #244. selectSession() ends with scrollToLastNonEmptyLine(), which parks
// the viewport ABOVE the bottom for any session whose buffer is taller than
// the screen and ends in blank rows, i.e. every real session after a tab
// switch. A tap-routing scheme that treats "viewport is scrolled up" as a
// reason to blur strands document.activeElement on <body> with no way to
// raise the keyboard, and the prompt row is no exception. Suppressing the
// MOUSE REPORT while scrolled up is correct and pinned below; suppressing
// FOCUS is not. Measured against PR #244 on 2026-08-09: body vs textarea.
//
// Must be a dispatched gesture: calling the touchend handler directly
// bypasses touchstart's preventDefault, which is half of what closes the
// focus path, so a direct call reports the right intent and still misses.
it('keeps the terminal input focusable after a tab switch parks the viewport off-bottom', async () => {
const probe = await page.evaluate(async () => {
window.__sentInputs = [];
app.activeSessionId = 'mobile-offbottom-tap-test';
app.sessions.set('mobile-offbottom-tap-test', {
id: 'mobile-offbottom-tap-test',
mode: 'claude',
cliVersion: '2.1.220',
status: 'running',
});
app._sendInputAsync = (_sessionId: string, input: string) => {
window.__sentInputs.push(input);
};
@@ -1006,41 +1058,29 @@ describe('Virtual Keyboard', () => {
app.saveAppSettingsToStorage(settings);
app._updateCjkInputState();
app.terminal.reset();
// Taller than the viewport, ending in the trailing blank rows that make
// scrollToLastNonEmptyLine() stop short of the bottom.
const lines: string[] = [];
for (let i = 1; i <= app.terminal.rows * 3; i++) lines.push(`Transcript row ${i}`);
lines.push('', '❯ ', '', '');
await new Promise<void>((resolve) => app.terminal.write(lines.join('\r\n'), resolve));
app.scrollToLastNonEmptyLine(); // what selectSession() does on every tab switch
await new Promise<void>((resolve) =>
app.terminal.write('Agent readback\r\n tap to collapse\r\n\r\n› ask', resolve)
);
(document.activeElement as HTMLElement | null)?.blur?.();
const screen = app.terminal.element?.querySelector('.xterm-screen');
const cell = app.terminal._core?._renderService?.dimensions?.css?.cell;
const rect = screen?.getBoundingClientRect();
if (!rect || !cell?.width || !cell?.height) return null;
const buffer = app.terminal.buffer.active;
return {
x: rect.left + cell.width * 2,
y: rect.top + cell.height * 5.5,
atBottom: buffer.viewportY >= buffer.baseY,
y: rect.top + cell.height * (app.terminal.buffer.active.cursorY + 0.5),
};
});
expect(point).not.toBeNull();
expect(probe).not.toBeNull();
// The guard only means anything if the viewport really did park off-bottom.
expect(probe!.atBottom).toBe(false);
await page.touchscreen.tap(probe!.x, probe!.y);
await page.touchscreen.tap(point!.x, point!.y);
const state = await page.evaluate(() => ({
activeClass: document.activeElement?.className,
sentInputs: window.__sentInputs,
}));
expect(state.activeClass).toContain('xterm-helper-textarea');
// SGR coordinates are meaningless off-bottom, so the tap must stay silent.
expect(state.sentInputs).toEqual([]);
});
-19
View File
@@ -543,25 +543,6 @@ describe('terminal touch tap mouse guard', () => {
expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false);
});
it('touch: forwards verified Claude transcript scrolling but keeps Codex touch in local history', () => {
const { app } = loadTerminalUiHarness();
app.activeSessionId = 'sess-1';
app.terminal = {
modes: { mouseTrackingMode: 'none' },
buffer: { active: { viewportY: 50, baseY: 50 } },
};
app.sessions = new Map([['sess-1', { mode: 'claude', cliVersion: '2.1.220' }]]);
expect(app._shouldForwardTouchScrollToApp()).toBe(true);
app.loadAppSettingsFromStorage = () => ({ terminalWheelLocalScrollback: true });
expect(app._shouldForwardTouchScrollToApp()).toBe(false);
app.loadAppSettingsFromStorage = () => ({ terminalWheelLocalScrollback: false });
app.sessions = new Map([['sess-1', { mode: 'codex' }]]);
expect(app._shouldForwardTouchScrollToApp()).toBe(false);
});
it('wheel: the local-scrollback opt-out pins the plain wheel to local scrollback (issue #154)', () => {
const { app } = loadTerminalUiHarness();
app.activeSessionId = 'sess-1';