fix(mobile): terminal and layout fixes for touch devices - #111
Conversation
Ark0N
left a comment
There was a problem hiding this comment.
Thanks @aakhter — the mobile fixes are well-aimed and the 24 new behavioral tests are genuinely high quality (they assert real DOM geometry, computed CSS, focus state, and the resize state machine rather than just snapshots; all pass locally). A few things block landing:
Blockers
Hard dependency on #112.
selectSessionhere callsscrollToLastNonEmptyLine()and_clearTerminalLoadState(), reads_xtermSnapshots, and the tab template references a per-sessionloadState— none of these exist on master; they're all introduced by #112. On bare master this is aTypeError/ReferenceErroron every tab switch. So this must land after#112. It also looks branched off an early version of #112 (it independently re-adds an identical_resetTerminalForReplay()and carries the pre-#112selectSessionbody), so a wholesale merge would silently revert #112's terminal rewrite. Please rebase onto the final reworked #112.Resize-arbitration guard freezes mobile width. The
cols < _ptyColsguard insession.ts resize()means a mobile-only session's PTY never narrows below the 120-col default → narrow phones render clipped. → allow the shrink for genuinely smaller viewports (and ideally cover rows-only shrink + re-grow in the unit test).Solo/detached header broken.
index.htmlremovessoloSessionTitleandsoloRedockBtn, butapp.js_applySoloMode(around 1025) still references them.
Should-fix
- The new mobile-header utility toggle button has no desktop hide rule → it shows at ≥1024px.
- The 18 binary visual-regression PNGs are environment-specific; freezing contributor-machine baselines into the repo will be flaky across machines/CI. → generate them in a canonical CI job, or gate the visual suite as allow-fail.
AGENTS.mdlargely duplicatesCLAUDE.md— consider pointing to it instead to avoid drift.
Once #112 is reworked and this is rebased on top with the resize guard fixed, the test coverage here is a real asset. Thanks again.
Mobile-focused fixes for the web UI: keyboard-accessory layout and overlap, native input visibility above the keyboard, CJK input handling, terminal touch scrolling, tab-menu tap targets, mic-recording glow containment, and mobile resize/keyboard-state handling on tab switch, plus mobile visual-regression test coverage and snapshots. Co-Authored-By: Saqeb Akhter <saqeb.akhter@gmail.com>
Review follow-ups on PR Ark0N#111 (rebased onto master post-Ark0N#112/Ark0N#113): Resize arbitration redesigned (review blocker 2): the previous 'cols < _ptyCols' guard froze a mobile-only session's PTY at the spawn default — narrow phones rendered clipped and could never re-fit. The guard now uses connection-scoped desktop sizing claims instead: ws-routes registers a claim on a desktop-typed resize and releases it on socket close (or when the same connection later reports a small viewport), and Session.resize() ignores mobile/tablet resizes only while at least one desktop connection holds a claim. A phone alone fully controls its size (shrink, rows-only shrink, re-grow); a phone glancing at a desktop-driven session can no longer reflow it. mobile-handlers' keyboard open/close resize now declares its viewport type so it participates in arbitration. Tests rewritten to cover mobile-only shrink/rows-only/re-grow, claim/release lifecycle, multi- claim behavior, and untyped legacy resizes; ws-routes test covers the claim lifecycle over a real socket. Solo/detached header restored (review blocker 3): index.html had removed #soloSessionTitle and #soloRedockBtn, which _applySoloMode still references — every detached window hit a null deref. Both are back alongside the new mobile utility toggle. Desktop leak fixed (review should-fix): .mobile-header-utility-toggle had no rule outside the <=768px media queries, so the raw button rendered on desktop. styles.css now hides it by default; the mobile/ tablet queries re-enable it. Visual-regression baselines reverted to master (review should-fix): the 18 contributor-machine PNGs are environment-specific (8 of the behavioral tests already report environment-sensitive failures across machines); re-baseline deliberately on the canonical machine instead. The 24 behavioral keyboard/layout/tabs tests are kept as-is. AGENTS.md trimmed to a pointer at CLAUDE.md (review should-fix) to avoid drift between duplicated guidance. Also dropped a dead getAttachmentHistoryForPersist stub (codex-branch residue — no such method exists in src/). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…setting, ESC passthrough Adversarial post-rebase audit (11 agents) confirmed four real issues; all fixed: - Desktop tab clicks stopped focusing the terminal: handleSessionTabClick passed preserveKeyboard:false on desktop (KeyboardHandler.keyboardVisible is mobile-only state) and selectSession's ternary mapped explicit false to 'never focus', skipping the gesture-stack focus master relies on. Focus policy now lives solely in _shouldFocusTerminalForTabSwitch() (desktop: always; touch: only while the keyboard is open). - Desktop sizing claims were almost never registered: selectSession's resizes run before _connectWs, so they went over HTTP (which never claims), leaving the arbitration inert in the canonical desktop+phone scenario. ws.onopen now sends a typed resize over the fresh socket — registering the claim and syncing PTY dims after (re)connects. - throttledResize (the main window-resize path) sent untyped HTTP resizes: a rotating phone bypassed a desktop claim, and a desktop narrowing past the tablet breakpoint never released its stale claim. It now sends typed resizes, WS-first, like sendResize. - The cjkInputEnabled App Settings toggle was silently ignored on touch phones/tablets (composer only reachable via the server inputCjkForm override, while the checkbox stayed visible and saveable). The user setting is honored everywhere again; mobile keeps native-input-by- default via the cjkInputEnabled:false mobile default. - _handleCjkInput appended multi-byte ESC sequences (hardware-keyboard arrows/Home/End on the composer) to local-echo pending text, typing raw ESC bytes into the prompt on Enter; they are now forwarded to the PTY like the onData path. Its backspace path also syncs the per-session flushed Maps the way onData does, so tab-switch restore no longer resurrects deleted characters. Defensive: Session.stop() clears desktop sizing claims (a hung client's socket close can lag teardown by a ping cycle), and the claims docblock documents the WS-only tradeoff explicitly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ark0N
left a comment
There was a problem hiding this comment.
Re-reviewed after rebasing this onto current master (post-#112-final and post-#113) and pushing fixes directly to the branch (maintainer edits). All three blockers and the should-fixes are addressed; summary of what changed:
Blockers
- #112 entanglement — rebased onto master
c29475e. The early-#112 artifacts are gone (_xtermSnapshots,codeman-xs-*localStorage, the pre-reworkselectSessionbody, the duplicate_resetTerminalForReplay, a deadgetAttachmentHistoryForPersisttest stub). An 11-agent diff audit verified the final #112 pane-buffer machinery and the #113 envelope migration survive byte-identical — the mobile changes are now purely additive. - Resize arbitration redesigned — the
cols < _ptyColsguard (which froze a mobile-only session at the 120-col spawn default) is replaced with connection-scoped desktop sizing claims:ws-routesregisters a claim per socket on a desktop-typed resize and releases it on close (or when that connection later reports a small viewport);Session.resize()ignores mobile/tablet resizes only while a desktop claim is live. A phone alone fully controls its size (shrink, rows-only, re-grow — all unit-tested); a phone glancing at a desktop-driven session can't reflow it. Wiring fixes that came out of the audit:ws.onopennow sends a typed resize (otherwise claims were never registered in the canonical flow), andthrottledResizesends typed WS-first resizes (so phone rotation arbitrates and a desktop narrowing past 768px releases its stale claim).mobile-handlers' keyboard resize declares its viewport type too. - Solo header restored —
#soloSessionTitle+#soloRedockBtnare back inindex.htmlalongside the new utility toggle;_applySoloModeresolves all elements (chromium-verified).
Should-fixes
- The utility toggle now has a base
display:none(it leaked onto desktop ≥769px); mobile/tablet media queries re-enable it. Verified hidden at 1440px, visible+functional at 390px. - The 18 visual-regression PNGs are reverted to master's baselines — 8 of the behavioral tests already fail purely from environment rendering differences on a second machine, so contributor-machine baselines would be flaky as predicted. The 24 behavioral tests are kept (84/92 pass here; the 8 failures reproduce identically on the unmodified branch, i.e. environment-sensitivity, not regressions).
AGENTS.mdis now a pointer toCLAUDE.md.
Additional fixes found by the post-rebase audit
- Desktop tab clicks stopped focusing the terminal (the
preserveKeyboardternary mapped desktop to never-focus, defeating master's gesture-stack focus). Focus policy now lives in_shouldFocusTerminalForTabSwitch(): desktop always focuses, touch only while the keyboard is open — your intended mobile behavior, minus the desktop regression. E2E-verified with a real session. cjkInputEnabledwas silently ignored on touch phones/tablets while its App Settings checkbox stayed toggleable. The explicit user setting is honored everywhere again; mobile keeps native-input-by-default via thecjkInputEnabled: falsemobile default._handleCjkInputswallowed multi-byte ESC sequences (hardware-keyboard arrows on the composer) into local-echo pending text — now forwarded to the PTY like theonDatapath; its backspace path also syncs the per-session flushed maps.
Known accepted tradeoffs (documented in the session.ts claims docblock): claims are WS-only (the stateless HTTP fallback can't claim), viewport typing is width-based (a landscape iPad classifies as desktop), and closeMobileHeaderUtilities is currently unused — the tray only closes via the toggle. Fine to iterate on later.
The keyboard/layout/tabs behavioral tests are genuinely good coverage — thanks for those. Merging once CI is green.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Mobile-focused fixes for the web terminal UI on touch devices, extracted as a focused change set:
/compactbutton.Verification
tsc --noEmitpasseseslintpasses(Mobile visual-regression snapshots are included; CI/maintainer can re-baseline if device rendering differs.)