Skip to content

fix(mobile): close the on-screen keyboard when tapping outside the terminal - #279

Merged
Ark0N merged 3 commits into
Ark0N:masterfrom
Lint111:feat/mobile-keyboard-dismiss
Aug 11, 2026
Merged

fix(mobile): close the on-screen keyboard when tapping outside the terminal#279
Ark0N merged 3 commits into
Ark0N:masterfrom
Lint111:feat/mobile-keyboard-dismiss

Conversation

@Lint111

Copy link
Copy Markdown
Contributor

On a phone, once the on-screen keyboard is up there is no way to close it from inside the app.

The terminal keeps focus on a hidden textarea and nothing ever releases it, so tapping the header, the tab strip or any empty page chrome leaves the keyboard covering roughly half the screen. The only way out is the OS back gesture.

Repro

iPhone-class viewport (390×844), claude-mode session. Focus the terminal, then tap the header logo:

document.activeElement after the tap
mastertextarea.xterm-helper-textarea — keyboard stays up
this branchbody — keyboard closes

I found this on my own fork and checked it against pristine master before assuming it was mine: the behaviour is identical on both, so it is upstream, not something my branch introduced.

The change

A document-level touchend handler blurs the terminal input. It is deliberately narrow, because focus is not ours to take:

  • only when the terminal input actually holds focus;
  • never inside #terminalContainer_handleMobileTerminalTap already classifies and routes those taps, and it owns that decision;
  • never on a control. Anything focusable or clickable is about to take focus itself, and the keyboard accessory bar exists to be used while the keyboard is open, so dismissing there would fight the user. closest() covers taps that land on a child element, like an icon inside a button.

touchend rather than click: a tap meant to dismiss usually is not meant to activate whatever sits underneath, and touchend fires before the synthesized click, so the blur lands first. The listener is passive and never calls preventDefault.

Test

dismisses the on-screen keyboard when a tap lands outside the terminal, in test/mobile/keyboard.test.ts.

It fails on master with a behavioural assertion, not a TypeError:

AssertionError: expected 'xterm-helper-textarea' not to contain 'xterm-helper-textarea'

and passes here. It asserts all three cases in one gesture sequence — outside chrome dismisses, a visible button does not, inside the terminal does not — so it also guards against over-reach, not just the missing dismiss.

It drives real dispatched touch events rather than calling the helper directly. The handler is bound on document, so a direct call would bypass the routing the test exists to check. (That distinction cost me two wrong diagnoses on #244; it seemed worth encoding.)

Verification

Against master 26416f98:

npx tsc --noEmit clean
npm run check:frontend-syntax 32 files parse cleanly
npm run lint clean
npm run format:check clean
npm run test:ci 4944 passed | 12 skipped, 0 failed
test/mobile/keyboard.test.ts branch 5 failed | 47 passed (52)
test/mobile/keyboard.test.ts master 5 failed | 46 passed (51)

Identical five failures on both sides — stale layout and accessory-bar expectations plus a CJK timeout, all pre-existing and untouched here. The branch runs one more test and it passes: zero regressions.

Scope

2 files, +120. One behaviour change, no existing test weakened, and nothing outside the mobile keyboard path.

🤖 Generated with Claude Code

…rminal
On a phone the terminal holds focus on a hidden textarea, and nothing ever
released it. Once the keyboard was up, tapping the header, the tab strip or any
empty page chrome left it up — covering roughly half the screen with no in-app
way to dismiss it.
Repro, iPhone-class viewport (390x844), claude-mode session, focus the terminal
then tap the header logo:
| | document.activeElement after the tap |
| --- | --- |
| master | textarea.xterm-helper-textarea (keyboard stays up) |
| this branch | body (keyboard closes) |
A document-level touchend handler blurs the terminal input, deliberately scoped
so focus is never stolen from something that wants it:
- only when the terminal input actually holds focus;
- never inside #terminalContainer — _handleMobileTerminalTap already classifies
and routes those taps and owns that decision;
- never on a control. Anything focusable or clickable is about to take focus
itself, and the keyboard accessory bar exists to be used WHILE the keyboard is
open, so dismissing there would fight the user.
Bound to touchend rather than click: a tap meant to dismiss usually is not meant
to activate what sits underneath, and touchend fires before the synthesized
click so the blur lands first. The listener is passive — it never calls
preventDefault.
Test: `dismisses the on-screen keyboard when a tap lands outside the terminal`
in test/mobile/keyboard.test.ts. It fails on master with a BEHAVIOURAL assertion
(`expected 'xterm-helper-textarea' not to contain 'xterm-helper-textarea'`),
not a TypeError, and passes here. It drives real dispatched touch events rather
than calling the helper, because the handler is bound on document and a direct
call would bypass the routing under test.
test/mobile/keyboard.test.ts: 52 tests, 5 failed | 47 passed. Master is 51 tests,
5 failed | 46 passed — the same five pre-existing failures (stale layout and
accessory-bar expectations, a CJK timeout), untouched here.
Full suite: 4944 passed | 12 skipped, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CopilotAI lite review requested due to automatic review settings August 10, 2026 14:38

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Lint111and others added 2 commits August 10, 2026 18:21
Regression from the dismiss handler in Ark0N#279: it fired on any touchend,
and a scroll ends in touchend too. Scrolling to read something while composing
closed the keyboard and dropped the composer — worse than the bug it fixed.
Track finger travel from touchstart and only treat a near-stationary gesture as
a tap, using the same 8px TAP_THRESHOLD the terminal's own touch handling uses
so both agree on tap-vs-scroll. Multi-touch is never a dismissing tap.
All three listeners stay passive; nothing calls preventDefault.
Measured on a Pixel-class viewport with a Firefox UA:
tap -> dismissed
scroll (120px) -> keyboard kept
micro-drift (4px) -> dismissed, so an imprecise tap still works
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dismiss handler fired on any touchend, so a scroll closed the keyboard too —
a regression the original test could not see, because it only ever dispatched a
stationary tap.
The helper now takes an optional travel distance and emits touchmove steps, and
the test asserts a 120px scroll leaves the terminal input focused. Removing the
`if (moved) return` guard fails this assertion, so it genuinely pins the fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Lint111

Copy link
Copy Markdown
ContributorAuthor

Pushed two follow-up commits after testing on a real device.

The dismiss handler fired on any touchend, and a scroll ends in touchend too. Scrolling to read something while composing closed the keyboard and dropped the composer — worse than the bug the PR fixes. My original test only ever dispatched a stationary tap, so it could not see this.

Fixed by tracking finger travel from touchstart and only treating a near-stationary gesture as a tap, using the same 8px TAP_THRESHOLD the terminal's own touch handling already uses so both agree on tap-vs-scroll. Multi-touch is never a dismissing tap. All three listeners stay passive.

Measured on a Pixel-class viewport with a Firefox UA:

gestureresult
tapdismissed
scroll (120px)keyboard kept
micro-drift (4px)dismissed — an imprecise tap still works

The test now takes an optional travel distance and emits touchmove steps, and asserts the 120px scroll leaves the input focused. Removing the if (moved) return guard fails that assertion, so it pins the fix rather than just passing alongside it.

Re-verified: test:ci 4944 passed | 12 skipped, 0 failed; keyboard.test.ts 52 tests, same 5 pre-existing failures as master.

@Ark0N
Ark0N merged commit 2d4616f into Ark0N:masterAug 11, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Aug 11, 2026
#279 and #280 auto-merge cleanly, but the merged result was red: neither
branch could see the other, and CI cannot see either, because the only test
covering #279 lives in test/mobile/** which test:ci excludes.
Two problems, both in #279's test:
1. The in-terminal case tapped the terminal's top-left corner, i.e. an inert
transcript row, and asserted focus was retained. That is precisely the
gesture #280 redefines, so #280 turned it red. Aim it at the PROMPT row
instead: the one in-terminal tap whose outcome neither PR claims, so it
still proves the #terminalContainer exemption without asserting the
toggle's behaviour.
2. The "a real control is exempt" case was VACUOUS. It picked the first
button measuring >8px, which is .welcome-ralph-link inside the welcome
overlay hideWelcome() had already hidden: the rect still measures, but
elementFromPoint at that point returns .xterm-screen, so the case tapped
the TERMINAL and passed for the wrong reason. It only surfaced because
#280 changed what a terminal tap does. Require the sampled point to
actually resolve to the button, and fail loudly when no control is
usable rather than silently asserting nothing.
Mutation-checked: removing the install, the #terminalContainer exemption,
the control exemption or the `if (moved) return` scroll guard each turns
the test red on its own. The control exemption had no coverage before.
Also fold the duplicated tap slop into one constant: initTerminal's
TAP_THRESHOLD now reads MOBILE_KEYBOARD_DISMISS_TAP_SLOP instead of
re-declaring 8, since a drift between them is exactly the bug the second
#279 commit fixed. And restore the comment the slop constant was inserted
into the middle of, which left "Regions where a tap must NOT dismiss"
sitting above the slop rather than the selector it documents.
test/mobile/keyboard.test.ts: 5 failed | 47 passed (52). Master is
5 failed | 46 passed (51) — the same five pre-existing failures.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
prmiguel pushed a commit to incursion-space/codeman that referenced this pull request Aug 21, 2026
… a runner
`npm test` ran config/vitest.config.ts, which includes the browser, visual and
perf suites. On any machine without chromium, a free port and per-machine PNG
baselines that fails ~87 tests on a clean master, so the repo's most obvious
command could not be used as a pass/fail signal. The workaround had spread into
four docs as "never run bare `npm test`" warnings.
`npm test` now runs config/vitest.ci.config.ts — byte-for-byte what CI runs — so
local green means CI green. Verified: 264 files, 5248 tests, exit 0.
The suites it leaves out are not abandoned; each has a command:
test:browser 5 Playwright files (chromium + a live server; codex-predictive-echo
also needs a real codex binary)
test:mobile unchanged — the above plus per-machine PNG baselines
test:perf 2 wall-clock benchmarks; need an otherwise idle machine
test:all the old everything-behaviour, kept reachable
test:ci is untouched (CI still calls it). test:watch and test:coverage follow
test onto the gate's config.
The more important half is the hole this closes. The exclusion list lived as
literals in one config and pointed one way only: a file excluded from CI and
added to no runner would be tested by NOTHING, silently, with every command
still green — vitest counts "no files matched a filter" as success. That is the
same shape as the Ark0N#279/Ark0N#280 blind spot already documented in CLAUDE.md.
So the globs moved to config/test-suites.ts, one array per REASON a suite cannot
run in CI, and all three configs derive from it. test/test-suite-partition.test.ts
then checks the arithmetic against the files on disk: it fails if any test file
is reachable by no runner, or by two. Confirmed it fires by orphaning a file and
watching it name it. The partition is exact today:
gate 264 + browser 5 + perf 2 + mobile 9 = 280 = every *.test.ts in the repo
⚠️ One sharp edge, deliberate and documented: a file filter must match its
runner. `npm test -- test/mobile/keyboard.test.ts` now matches nothing and exits
GREEN having run zero tests, because the gate's config excludes that path.
CLAUDE.md recommended exactly that command in the on-screen-keyboard note; that
line now says `npm run test:mobile -- <file>`, and the Testing section calls out
the trap, since a green run of zero tests is worse than a red one.
Docs synced: CLAUDE.md, AGENTS.md, .github/CONTRIBUTING.md, README.md,
README.zh-CN.md, and two ci.yml comments that claimed only test/mobile/** was
excluded — it is three suites, and 5 Playwright files rather than 3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Lint111@Ark0N